Skip to content

Commit

Permalink
Make option children a text content by default
Browse files Browse the repository at this point in the history
fix #11911
  • Loading branch information
Slowyn committed Jul 24, 2018
1 parent ca0941f commit 388028e
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 25 deletions.
25 changes: 25 additions & 0 deletions fixtures/dom/src/components/fixtures/selects/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,31 @@ class SelectFixture extends React.Component {
</form>
</div>
</TestCase>

<TestCase
title="An option which contains conditional render fails"
relatedIssues="11911">
<TestCase.Steps>
<li>Select any option</li>
</TestCase.Steps>
<TestCase.ExpectedResult>
Option should be set
</TestCase.ExpectedResult>

<div className="test-fixture">
<select value={this.state.value} onChange={this.onChange}>
<option value="red">
red {this.state.value === 'red' && 'is chosen '} TextNode
</option>
<option value="blue">
blue {this.state.value === 'blue' && 'is chosen '} TextNode
</option>
<option value="green">
green {this.state.value === 'green' && 'is chosen '} TextNode
</option>
</select>
</div>
</TestCase>
</FixtureSet>
);
}
Expand Down
4 changes: 1 addition & 3 deletions packages/react-dom/src/__tests__/ReactDOMOption-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@ describe('ReactDOMOption', () => {
expect(() => {
node = ReactTestUtils.renderIntoDocument(el);
}).toWarnDev(
'<div> cannot appear as a child of <option>.\n' +
' in div (at **)\n' +
' in option (at **)',
'<div> cannot appear as a child of <option>.\n' + ' in option (at **)',
);
expect(node.innerHTML).toBe('1 2');
ReactTestUtils.renderIntoDocument(el);
Expand Down
14 changes: 12 additions & 2 deletions packages/react-dom/src/client/ReactDOMFiberOption.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import React from 'react';
import warning from 'shared/warning';
import validateDOMNesting from './validateDOMNesting';

let didWarnSelectedSetOnOption = false;

Expand All @@ -17,14 +18,23 @@ function flattenChildren(children) {

// Flatten children and warn if they aren't strings or numbers;
// invalid types are ignored.
// We can silently skip them because invalid DOM nesting warning
// catches these cases in Fiber.
React.Children.forEach(children, function(child) {
if (child == null) {
return;
}
if (typeof child === 'string' || typeof child === 'number') {
content += child;
return;
}
if (__DEV__) {
// We do not have HostContext here
// but we can put some parent information at least
// Also this cause a bit different message than it was previously
validateDOMNesting(child.type, null, {
current: {
tag: 'option',
},
});
}
});

Expand Down
1 change: 1 addition & 0 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ export function prepareUpdate(
export function shouldSetTextContent(type: string, props: Props): boolean {
return (
type === 'textarea' ||
type === 'option' ||
typeof props.children === 'string' ||
typeof props.children === 'number' ||
(typeof props.dangerouslySetInnerHTML === 'object' &&
Expand Down
40 changes: 20 additions & 20 deletions scripts/rollup/results.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@
"filename": "react.development.js",
"bundleType": "UMD_DEV",
"packageName": "react",
"size": 59086,
"gzip": 16296
"size": 59233,
"gzip": 16554
},
{
"filename": "react.production.min.js",
"bundleType": "UMD_PROD",
"packageName": "react",
"size": 7217,
"gzip": 3050
"size": 7332,
"gzip": 3111
},
{
"filename": "react.development.js",
Expand Down Expand Up @@ -46,15 +46,15 @@
"filename": "react-dom.development.js",
"bundleType": "UMD_DEV",
"packageName": "react-dom",
"size": 641505,
"gzip": 149286
"size": 656643,
"gzip": 153886
},
{
"filename": "react-dom.production.min.js",
"bundleType": "UMD_PROD",
"packageName": "react-dom",
"size": 96507,
"gzip": 31258
"size": 98962,
"gzip": 32102
},
{
"filename": "react-dom.development.js",
Expand Down Expand Up @@ -88,15 +88,15 @@
"filename": "react-dom-test-utils.development.js",
"bundleType": "UMD_DEV",
"packageName": "react-dom",
"size": 46355,
"gzip": 12768
"size": 44787,
"gzip": 12185
},
{
"filename": "react-dom-test-utils.production.min.js",
"bundleType": "UMD_PROD",
"packageName": "react-dom",
"size": 10653,
"gzip": 3918
"size": 10366,
"gzip": 3841
},
{
"filename": "react-dom-test-utils.development.js",
Expand All @@ -123,15 +123,15 @@
"filename": "react-dom-unstable-native-dependencies.development.js",
"bundleType": "UMD_DEV",
"packageName": "react-dom",
"size": 62921,
"gzip": 16559
"size": 60219,
"gzip": 15769
},
{
"filename": "react-dom-unstable-native-dependencies.production.min.js",
"bundleType": "UMD_PROD",
"packageName": "react-dom",
"size": 11622,
"gzip": 4007
"size": 11378,
"gzip": 3929
},
{
"filename": "react-dom-unstable-native-dependencies.development.js",
Expand Down Expand Up @@ -165,15 +165,15 @@
"filename": "react-dom-server.browser.development.js",
"bundleType": "UMD_DEV",
"packageName": "react-dom",
"size": 105006,
"gzip": 27524
"size": 104567,
"gzip": 27873
},
{
"filename": "react-dom-server.browser.production.min.js",
"bundleType": "UMD_PROD",
"packageName": "react-dom",
"size": 15463,
"gzip": 5917
"size": 15619,
"gzip": 5953
},
{
"filename": "react-dom-server.browser.development.js",
Expand Down

0 comments on commit 388028e

Please sign in to comment.