react: [bug][16.5.0] option returns [object Object] instead of string
What is the current behavior?
<option> returns [object Object] instead string
Demo: https://codesandbox.io/s/ww5mv2w957
What is the expected behavior? Expected string
Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
About this issue
- Original URL
- State: closed
- Created 6 years ago
- Reactions: 10
- Comments: 29 (7 by maintainers)
Links to this issue
Commits related to this issue
- Restore DOMOption warnings for Object and Fragment children (#13586) — committed to Simek/react by Simek 6 years ago
- Move PaymentMethodLabel to a lib function and add it to OrderForm (i18n) The fact that we use these labels inside `<select/>` options prevent us from implementing this as a React component as for now... — committed to opencollective/opencollective-frontend by Betree 6 years ago
- Move PaymentMethodLabel to a lib function and add it to OrderForm (i18n) The fact that we use these labels inside `<select/>` options prevent us from implementing this as a React component as for now... — committed to opencollective/opencollective-frontend by Betree 6 years ago
Since we warned about this before (in 15), and it crashes in unexpected ways, I think the new behavior is actually a bugfix — both for the missing warning, and for inconsistent behavior. It’s very unfortunate that the warnings didn’t get emitted in some cases (to be clear, it did for components returning DOM nodes — but we missed the case for strings). But I think allowing crashes in this case is worse than addressing the problem head on.
The intended workaround if you’re using react-intl is indeed this: https://codesandbox.io/s/6yr4xy5ll3
If you use something else, just make what you’re passing to
<option>as a child is actually a string: https://codesandbox.io/s/oom712ov79Both of these cases don’t crash on updates, and work correctly. Again, very sorry we only discovered this now.
If anyone still needs an answer to the original question, as mentioned in earlier comments and in the documentation here, you cannot use a formatted message in the format of
instead I’ve found formating it as such does work:
where
txtwill return the id’s default string for whichever Message Descriptor file is selectedI don’t think it was strictly a regression. This is kind of a thorny area. It was never intentionally supported. It accidentally worked on initial mount but then crashed on updates (https://github.com/facebook/react/pull/13261). Fixing the crash was more important, so we fixed it to be treated as text content (which it should be). Unfortunately this means putting custom components in the middle is not supported. That’s consistent with how
textareaand similar elements work.I think it’s better to show invalid output and warn about something that breaks on updates, than to let people use it only to discover it crashes in production. But I can see arguments for why this should be supported when the custom component returns a string. Unfortunately I don’t know how to fix it in a way that would both solve the update crashes and support text-only content. I think for now it’s reasonable to say putting custom components into
<option>doesn’t really work (and never quite worked correctly), and ask you to manually provide a string to it.This is fixed in 18 Alpha. https://codesandbox.io/s/thirsty-austin-yzgyg?file=/src/index.js
@Simek thanks but in my real app i use a component that uses context internally so i can’t really use that.
<option><MyComponent value="value" /></option>and MyComponent uses a context.Consumer, processes value a bit and returns a string.Shouldn’t this be kept open (or is there another issue tracking this?) even if nobody has any implementation ideas yet? It’s totally reasonable to expect
<option><React.Fragment>Foo</React.Fragment></option>to work, considering the developer knows that the output HTML would in fact be valid.React 15 also warned about this but we accidentally lost the warning in 16 for cases when components return a string.
@bvaughn yeah it seems to be connected to #13465 It seems you can’t use any component as child to option. Test with simple functional component yields same problem: https://codesandbox.io/s/7y5km7r7k6
If you remove
optiontag and addtagNameproperty toFormattedMessagedemo works correctly.Fork: https://codesandbox.io/s/6yr4xy5ll3
@jquense Sorry, forgot add default tag for
FormattedMessageIt doesn’t work withReact.Fragmenttoo.@hrupesh There’s still no solution that isn’t hacky. What I did and what my personal recommendation would be is to use a replacement over native select element, carefully crafted to keep accessibility levels high, like Reach UI Listbox.
@gaearon Maybe I’m missing something, but 16.4 did not warn about this code:
This code worked just fine in React 16.4, but later broke in 16.5. In some of the code examples I’ve seen from
react-intlpackage, it would render as<option><span>message</span></option>which would cause the warning. But in the example above no such warning occurs because the component returns a string.16.4.2: https://codesandbox.io/s/gifted-beaver-xjv1j 16.8.6: https://codesandbox.io/s/angry-carson-8rnd3
Could we perhaps reevaluate this as a regression that needs to be fixed rather than an enhancement?
I would expect that
<option><span>message</span></option>should be an error while<option>foo</option>(from the example above) should not.Just opened a feature request here https://github.com/facebook/react/issues/15513
@gaearon I see how this would be difficult to fix, but I’d argue that it still counts as a bug. Isn’t it reasonable for a user of React to expect
optionelements to support nesting of components the same way that every other React component does? If it says anywhere in the React docs and tutorial that only certain kinds of components can be nested, I must have missed it.@gaearon a single
childrenthat renders a single string wouldn’t trigger the crash. I understand that it’s easier to validate and enforce<option>children input, but the truth is, anyone that want to use context to setup an<option>has to leak the context into the parent. That, and having a corner case where you can’t use react to setup an element children feels extremely weird.@quazzie You can fix that by wrapping
valueinoptiontag inside contextConsumerrender function.Fork: https://codesandbox.io/s/oom712ov79
For what it’s worth, the same Code Sandbox works with React 16.4 but not 16.5.
Maybe related to #13465? cc @gaearon
I don’t have context on that change, so I’m not sure if this outcome is expected/okay or not.