Skip to content

Commit

Permalink
Fixes children when using dangerouslySetInnerHtml in a selected <opti…
Browse files Browse the repository at this point in the history
…on> (facebook#13078)

* Fixes children when using dangerouslySetInnerHtml in a selected <option>

This fixes an inadvertent cast of undefined children to an empty string when creating an option tag that will be selected:

```
  <select defaultValue="test">
    <option value='test' dangerouslySetInnerHTML={{ __html: '&rlm; test'}} />
  </select>
```

This causes an invariant error because both children and dangerouslySetInnerHTML are set.

* PR fix and new ReactDOMServerIntegrationForms test

* Account for null case

* Combine test cases into single test

* Add tests for failure cases

* Fix lint
  • Loading branch information
mridgway authored and gaearon committed Jun 21, 2018
1 parent a960d18 commit da5c87b
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 1 deletion.
31 changes: 31 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMSelect-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,37 @@ describe('ReactDOMSelect', () => {
expect(markup).not.toContain('<option selected="" value="gorilla"');
});

it('should support server-side rendering with dangerouslySetInnerHTML', () => {
const stub = (
<select defaultValue="giraffe">
<option
value="monkey"
dangerouslySetInnerHTML={{
__html: 'A monkey!',
}}>
{undefined}
</option>
<option
value="giraffe"
dangerouslySetInnerHTML={{
__html: 'A giraffe!',
}}>
{null}
</option>
<option
value="gorilla"
dangerouslySetInnerHTML={{
__html: 'A gorilla!',
}}
/>
</select>
);
const markup = ReactDOMServer.renderToString(stub);
expect(markup).toContain('<option selected="" value="giraffe"');
expect(markup).not.toContain('<option selected="" value="monkey"');
expect(markup).not.toContain('<option selected="" value="gorilla"');
});

it('should support server-side rendering with multiple', () => {
const stub = (
<select multiple={true} value={['giraffe', 'gorilla']} onChange={noop}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const {
resetModules,
itRenders,
itClientRenders,
itThrowsWhenRendering,
renderIntoDom,
serverRender,
} = ReactDOMServerIntegrationUtils(initModules);
Expand Down Expand Up @@ -327,6 +328,81 @@ describe('ReactDOMServerIntegration', () => {
expectSelectValue(e, 'bar');
});

itRenders(
'a select with options that use dangerouslySetInnerHTML',
async render => {
const e = await render(
<select defaultValue="baz" value="bar" readOnly={true}>
<option
id="foo"
value="foo"
dangerouslySetInnerHTML={{
__html: 'Foo',
}}>
{undefined}
</option>
<option
id="bar"
value="bar"
dangerouslySetInnerHTML={{
__html: 'Bar',
}}>
{null}
</option>
<option
id="baz"
value="baz"
dangerouslySetInnerHTML={{
__html: 'Baz',
}}
/>
</select>,
1,
);
expectSelectValue(e, 'bar');
},
);

itThrowsWhenRendering(
'a select with option that uses dangerouslySetInnerHTML and 0 as child',
async render => {
await render(
<select defaultValue="baz" value="foo" readOnly={true}>
<option
id="foo"
value="foo"
dangerouslySetInnerHTML={{
__html: 'Foo',
}}>
{0}
</option>
</select>,
1,
);
},
'Can only set one of `children` or `props.dangerouslySetInnerHTML`.',
);

itThrowsWhenRendering(
'a select with option that uses dangerouslySetInnerHTML and empty string as child',
async render => {
await render(
<select defaultValue="baz" value="foo" readOnly={true}>
<option
id="foo"
value="foo"
dangerouslySetInnerHTML={{
__html: 'Foo',
}}>
{''}
</option>
</select>,
1,
);
},
'Can only set one of `children` or `props.dangerouslySetInnerHTML`.',
);

itRenders(
'a select value overriding defaultValue no matter the prop order',
async render => {
Expand Down
5 changes: 4 additions & 1 deletion packages/react-dom/src/server/ReactPartialRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,10 @@ function flattenTopLevelChildren(children: mixed): FlatReactChildren {
return [fragmentChildElement];
}

function flattenOptionChildren(children: mixed): string {
function flattenOptionChildren(children: mixed): ?string {
if (children === undefined || children === null) {
return children;
}
let content = '';
// Flatten children and warn if they aren't strings or numbers;
// invalid types are ignored.
Expand Down

0 comments on commit da5c87b

Please sign in to comment.