React: [bug][16.5.0] option returns [object Object] instead of string

Created on 7 Sep 2018  Â·  27Comments  Â·  Source: facebook/react

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?

Discussion

Most helpful comment

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

All 27 comments

FormattedMessage doesn't render a string, it renders a span which is not valid HTML for the inside of an option tag. You should use intl.formatMessage() directly or try setting the https://github.com/yahoo/react-intl/wiki/Components#intlprovider textcomponent to React.Fragment to avoid the extra span

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

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.

If you remove option tag and add tagName property to FormattedMessage demo works correctly.

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

It doesn't work with React.Fragment too.

we should make this work probably as well.

IDK how to categorize this breakage, it worked solely because of how forgiving browsers can be (which is not unlimited causing the original bug), but the markup is definitely incorrect. It also definitely broke something that "worked" before

Does not work with a context consumer inside a option either.
https://codesandbox.io/s/pk574rm207

I'm going to tag this as a regression for now, at least until we're sure one way or another.

@quazzie You can fix that by wrapping value in option tag inside context Consumer render function.

Fork: https://codesandbox.io/s/oom712ov79

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

@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

@quazzie Sure thing, it seems like it's a regression.

For now if you want you can implement tagName property and pass option as value to component in your app. If property has been provided you can wrap String inside component otherwise you can just return String.

To be clear I'm just providing quick solutions until React team decided how to solve that issue.

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.

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

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.

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

@nicolas-cherel You're welcome to file a proposal for how it should work (as long as it doesn't include crashes).

To be clear — if you figure out how to make it work in all cases, I think we'd be happy supporting using custom components returning strings for this. I couldn't figure it out yet.

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.

As a feature request, sure. Please file a new issue or search if one exists?

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

React 15 also warned about this but we accidentally lost the warning in 16 for cases when components return a string.

I am sorry to go a bit off track here, but what is the status for this lost warning? Is it something which won't be re-introduced?

We had the exact case where we were using a translation component returning a string. The end result is that [object Object] gets rendered which is quite unexpected in the context of React.

Just opened a feature request here https://github.com/facebook/react/issues/15513

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

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

So any valid workaround that doesn't involves changing your entire component tree? Many people is using context inside options to provide localisation etc.

Is there any solution to this till date?

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

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jvorcak picture jvorcak  Â·  3Comments

varghesep picture varghesep  Â·  3Comments

huxiaoqi567 picture huxiaoqi567  Â·  3Comments

Prinzhorn picture Prinzhorn  Â·  3Comments

UnbearableBear picture UnbearableBear  Â·  3Comments