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)

Commits related to this issue

Most upvoted comments

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/oom712ov79

Both 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

<select>
    <option value="foo">
        <FormattedMessage id="bar"/>
    </option>
</select>

instead I’ve found formating it as such does work:

<select>
    <FormattedMessage id="bar">
        {txt => <option value="foo">{txt}</option>}
    </FormattedMessage>
</select>

where txt will return the id’s default string for whichever Message Descriptor file is selected

I 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 textarea and 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.

@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.

screen shot 2018-09-07 at 5 20 32 pm

@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 option tag and add tagName property to FormattedMessage demo works correctly.

Fork: https://codesandbox.io/s/6yr4xy5ll3

@jquense Sorry, forgot add default tag for FormattedMessage It doesn’t work with React.Fragment too.

@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:

import React from "react";
import ReactDOM from "react-dom";

const App = () => {
  return (
    <select>
      <option>
        <Foo />
      </option>
    </select>
  );
};

function Foo() {
  return "foo";
}

ReactDOM.render(<App />, document.getElementById("root"));

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-intl package, 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 option elements 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 children that 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 value in option tag inside context Consumer render 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.