hanzoai/react
2
0
Fork 0
mirror of https://github.com/hanzoai/react.git synced 2026-09-24 17:02:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

Allow complex objects as children of option only if value is provided (#21431)

Browse Source
This commit is contained in:
Sebastian Markbåge
2021-05-05 08:40:54 -07:00
committed by GitHub
parent 4bfee83725
commit b14e3fef19
7 changed files with 168 additions and 105 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/react-dom/src/__tests__/ReactDOMOption-test.js
Vendored
+100 -23
View File
@@ -12,12 +12,14 @@
describe('ReactDOMOption', () => {
let React;
let ReactDOM;
let ReactDOMServer;
let ReactTestUtils;
beforeEach(() => {
jest.resetModules();
React = require('react');
ReactDOM = require('react-dom');
ReactDOMServer = require('react-dom/server');
ReactTestUtils = require('react-dom/test-utils');
});
@@ -32,9 +34,9 @@ describe('ReactDOMOption', () => {
expect(node.innerHTML).toBe('1 foo');
});
it('should ignore and warn invalid children types', () => {
it('should warn for invalid child tags', () => {
const el = (
<option>
<option value="12">
{1} <div /> {2}
</option>
);
@@ -42,10 +44,45 @@ describe('ReactDOMOption', () => {
expect(() => {
node = ReactTestUtils.renderIntoDocument(el);
}).toErrorDev(
'Only strings and numbers are supported as <option> children.\n' +
'validateDOMNesting(...): <div> cannot appear as a child of <option>.\n' +
' in div (at **)\n' +
' in option (at **)',
);
expect(node.innerHTML).toBe('1 [object Object] 2');
expect(node.innerHTML).toBe('1 <div></div> 2');
ReactTestUtils.renderIntoDocument(el);
});
it('should warn for component child if no value prop is provided', () => {
function Foo() {
return '2';
}
const el = (
<option>
{1} <Foo /> {3}
</option>
);
let node;
expect(() => {
node = ReactTestUtils.renderIntoDocument(el);
}).toErrorDev(
'Cannot infer the option value of complex children. ' +
'Pass a `value` prop or use a plain string as children to <option>.',
);
expect(node.innerHTML).toBe('1 2 3');
ReactTestUtils.renderIntoDocument(el);
});
it('should not warn for component child if value prop is provided', () => {
function Foo() {
return '2';
}
const el = (
<option value="123">
{1} <Foo /> {3}
</option>
);
const node = ReactTestUtils.renderIntoDocument(el);
expect(node.innerHTML).toBe('1 2 3');
ReactTestUtils.renderIntoDocument(el);
});
@@ -91,7 +128,7 @@ describe('ReactDOMOption', () => {
it('should support element-ish child', () => {
// This is similar to <fbt>.
// It's important that we toString it.
// We don't toString it because you must instead provide a value prop.
const obj = {
$$typeof: Symbol.for('react.element'),
type: props => props.content,
@@ -105,37 +142,42 @@ describe('ReactDOMOption', () => {
},
};
let node = ReactTestUtils.renderIntoDocument(<option>{obj}</option>);
expect(node.innerHTML).toBe('hello');
node = ReactTestUtils.renderIntoDocument(<option>{[obj]}</option>);
expect(node.innerHTML).toBe('hello');
expect(() => {
node = ReactTestUtils.renderIntoDocument(
<option>
{obj}
<span />
</option>,
);
}).toErrorDev(
'Only strings and numbers are supported as <option> children.',
let node = ReactTestUtils.renderIntoDocument(
<option value="a">{obj}</option>,
);
expect(node.innerHTML).toBe('hello[object Object]');
expect(node.innerHTML).toBe('hello');
node = ReactTestUtils.renderIntoDocument(
<option>
<option value="b">{[obj]}</option>,
);
expect(node.innerHTML).toBe('hello');
node = ReactTestUtils.renderIntoDocument(
<option value={obj}>{obj}</option>,
);
expect(node.innerHTML).toBe('hello');
expect(node.value).toBe('hello');
node = ReactTestUtils.renderIntoDocument(
<option value={obj}>
{'1'}
{obj}
{2}
</option>,
);
expect(node.innerHTML).toBe('1hello2');
expect(node.value).toBe('hello');
});
it('should be able to use dangerouslySetInnerHTML on option', () => {
const stub = <option dangerouslySetInnerHTML={{__html: 'foobar'}} />;
const node = ReactTestUtils.renderIntoDocument(stub);
let node;
expect(() => {
node = ReactTestUtils.renderIntoDocument(stub);
}).toErrorDev(
'Pass a `value` prop if you set dangerouslyInnerHTML so React knows which value should be selected.\n' +
' in option (at **)',
);
expect(node.innerHTML).toBe('foobar');
});
@@ -169,4 +211,39 @@ describe('ReactDOMOption', () => {
ReactDOM.render(<select value="gorilla">{options}</select>, container);
expect(node.selectedIndex).toEqual(2);
});
it('generates a warning and hydration error when an invalid nested tag is used as a child', () => {
const ref = React.createRef();
const children = (
<select readOnly={true} value="bar">
<option value="bar">
{['Bar', false, 'Foo', <div key="1" ref={ref} />, 'Baz']}
</option>
</select>
);
const container = document.createElement('div');
container.innerHTML = ReactDOMServer.renderToString(children);
expect(container.firstChild.getAttribute('value')).toBe(null);
expect(container.firstChild.getAttribute('defaultValue')).toBe(null);
const option = container.firstChild.firstChild;
expect(option.nodeName).toBe('OPTION');
expect(option.textContent).toBe('BarFooBaz');
expect(option.selected).toBe(true);
expect(() => ReactDOM.hydrate(children, container)).toErrorDev([
'Text content did not match. Server: "FooBaz" Client: "Foo"',
'validateDOMNesting(...): <div> cannot appear as a child of <option>.',
]);
expect(option.textContent).toBe('BarFooBaz');
expect(option.selected).toBe(true);
expect(ref.current.nodeName).toBe('DIV');
expect(ref.current.parentNode).toBe(option);
});
});
packages/react-dom/src/__tests__/ReactDOMServerIntegrationSelect-test.js
Vendored
+12 -16
View File
@@ -156,13 +156,12 @@ describe('ReactDOMServerIntegrationSelect', () => {
</option>
<option
id="baz"
value="baz"
dangerouslySetInnerHTML={{
__html: 'Baz',
__html: 'Baz', // This warns because no value prop is passed.
}}
/>
</select>,
1,
2,
);
expectSelectValue(e, 'bar');
},
@@ -228,26 +227,23 @@ describe('ReactDOMServerIntegrationSelect', () => {
</select>,
);
const option = e.options[0];
expect(option.childNodes.length).toBe(1);
expect(option.childNodes[0].nodeType).toBe(3);
expect(option.childNodes[0].nodeValue).toBe('A B');
expect(option.textContent).toBe('A B');
expect(option.value).toBe('bar');
expect(option.selected).toBe(true);
});
itRenders(
'a select option with flattened children and a warning',
'a select option with flattened children no value',
async render => {
const e = await render(
<select readOnly={true} value="bar">
<option value="bar">
{['Bar', false, 'Foo', <div key="1" />, 'Baz']}
</option>
<select value="A B" readOnly={true}>
<option>A {'B'}</option>
</select>,
1,
);
expect(e.getAttribute('value')).toBe(null);
expect(e.getAttribute('defaultValue')).toBe(null);
expect(e.firstChild.innerHTML).toBe('BarFoo[object Object]Baz');
expect(e.firstChild.selected).toBe(true);
const option = e.options[0];
expect(option.textContent).toBe('A B');
expect(option.value).toBe('A B');
expect(option.selected).toBe(true);
},
);
});
packages/react-dom/src/client/ReactDOMComponent.js
Vendored
+1 -7
View File
@@ -29,7 +29,6 @@ import {
restoreControlledState as ReactDOMInputRestoreControlledState,
} from './ReactDOMInput';
import {
getHostProps as ReactDOMOptionGetHostProps,
postMountWrapper as ReactDOMOptionPostMountWrapper,
validateProps as ReactDOMOptionValidateProps,
} from './ReactDOMOption';
@@ -535,7 +534,7 @@ export function setInitialProperties(
break;
case 'option':
ReactDOMOptionValidateProps(domElement, rawProps);
props = ReactDOMOptionGetHostProps(domElement, rawProps);
props = rawProps;
break;
case 'select':
ReactDOMSelectInitWrapperState(domElement, rawProps);
@@ -615,11 +614,6 @@ export function diffProperties(
nextProps = ReactDOMInputGetHostProps(domElement, nextRawProps);
updatePayload = [];
break;
case 'option':
lastProps = ReactDOMOptionGetHostProps(domElement, lastRawProps);
nextProps = ReactDOMOptionGetHostProps(domElement, nextRawProps);
updatePayload = [];
break;
case 'select':
lastProps = ReactDOMSelectGetHostProps(domElement, lastRawProps);
nextProps = ReactDOMSelectGetHostProps(domElement, nextRawProps);
packages/react-dom/src/client/ReactDOMHostConfig.js
Vendored
-1
View File
@@ -351,7 +351,6 @@ export function prepareUpdate(
export function shouldSetTextContent(type: string, props: Props): boolean {
return (
type === 'textarea' ||
type === 'option' ||
type === 'noscript' ||
typeof props.children === 'string' ||
typeof props.children === 'number' ||
packages/react-dom/src/client/ReactDOMOption.js
Vendored
+25 -51
View File
@@ -12,27 +12,7 @@ import {getToStringValue, toString} from './ToStringValue';
let didWarnSelectedSetOnOption = false;
let didWarnInvalidChild = false;
function flattenChildren(children) {
let content = '';
// Flatten children. We'll warn if they are invalid
// during validateProps() which runs for hydration too.
// Note that this would throw on non-element objects.
// Elements are stringified (which is normally irrelevant
// but matters for <fbt>).
Children.forEach(children, function(child) {
if (child == null) {
return;
}
content += (child: any);
// Note: we don't warn about invalid children here.
// Instead, this is done separately below so that
// it happens during the hydration code path too.
});
return content;
}
let didWarnInvalidInnerHTML = false;
/**
* Implements an <option> host component that warns when `selected` is set.
@@ -40,28 +20,33 @@ function flattenChildren(children) {
export function validateProps(element: Element, props: Object) {
if (__DEV__) {
// This mirrors the code path above, but runs for hydration too.
// Warn about invalid children here so that client and hydration are consistent.
// TODO: this seems like it could cause a DEV-only throw for hydration
// if children contains a non-element object. We should try to avoid that.
if (typeof props.children === 'object' && props.children !== null) {
Children.forEach(props.children, function(child) {
if (child == null) {
return;
}
if (typeof child === 'string' || typeof child === 'number') {
return;
}
if (typeof (child: any).type !== 'string') {
return;
}
if (!didWarnInvalidChild) {
didWarnInvalidChild = true;
// If a value is not provided, then the children must be simple.
if (props.value == null) {
if (typeof props.children === 'object' && props.children !== null) {
Children.forEach(props.children, function(child) {
if (child == null) {
return;
}
if (typeof child === 'string' || typeof child === 'number') {
return;
}
if (!didWarnInvalidChild) {
didWarnInvalidChild = true;
console.error(
'Cannot infer the option value of complex children. ' +
'Pass a `value` prop or use a plain string as children to <option>.',
);
}
});
} else if (props.dangerouslySetInnerHTML != null) {
if (!didWarnInvalidInnerHTML) {
didWarnInvalidInnerHTML = true;
console.error(
'Only strings and numbers are supported as <option> children.',
'Pass a `value` prop if you set dangerouslyInnerHTML so React knows ' +
'which value should be selected.',
);
}
});
}
}
// TODO: Remove support for `selected` in <option>.
@@ -81,14 +66,3 @@ export function postMountWrapper(element: Element, props: Object) {
element.setAttribute('value', toString(getToStringValue(props.value)));
}
}
export function getHostProps(element: Element, props: Object) {
const hostProps = {children: undefined, ...props};
const content = flattenChildren(props.children);
if (content) {
hostProps.children = content;
}
return hostProps;
}
packages/react-dom/src/server/ReactDOMServerFormatConfig.js
Vendored
+15 -2
View File
@@ -563,6 +563,7 @@ let didWarnDefaultChecked = false;
let didWarnDefaultSelectValue = false;
let didWarnDefaultTextareaValue = false;
let didWarnInvalidOptionChildren = false;
let didWarnInvalidOptionInnerHTML = false;
let didWarnSelectedSetOnOption = false;
function checkSelectProp(props, propName) {
@@ -667,7 +668,8 @@ function flattenOptionChildren(children: mixed): string {
) {
didWarnInvalidOptionChildren = true;
console.error(
'Only strings and numbers are supported as <option> children.',
'Cannot infer the option value of complex children. ' +
'Pass a `value` prop or use a plain string as children to <option>.',
);
}
}
@@ -736,7 +738,18 @@ function pushStartOption(
if (value !== null) {
stringValue = '' + value;
} else {
stringValue = children = flattenOptionChildren(children);
if (__DEV__) {
if (innerHTML !== null) {
if (!didWarnInvalidOptionInnerHTML) {
didWarnInvalidOptionInnerHTML = true;
console.error(
'Pass a `value` prop if you set dangerouslyInnerHTML so React knows ' +
'which value should be selected.',
);
}
}
}
stringValue = flattenOptionChildren(children);
}
if (isArray(selectedValue)) {
// multiple
packages/react-dom/src/server/ReactPartialRenderer.js
Vendored
+15 -5
View File
@@ -188,6 +188,7 @@ let didWarnDefaultChecked = false;
let didWarnDefaultSelectValue = false;
let didWarnDefaultTextareaValue = false;
let didWarnInvalidOptionChildren = false;
let didWarnInvalidOptionInnerHTML = false;
const didWarnAboutNoopUpdateForComponent = {};
const didWarnAboutBadClass = {};
const didWarnAboutModulePatternComponent = {};
@@ -334,7 +335,8 @@ function flattenOptionChildren(children: mixed): ?string {
) {
didWarnInvalidOptionChildren = true;
console.error(
'Only strings and numbers are supported as <option> children.',
'Cannot infer the option value of complex children. ' +
'Pass a `value` prop or use a plain string as children to <option>.',
);
}
}
@@ -1507,13 +1509,23 @@ class ReactDOMServerRenderer {
} else if (tag === 'option') {
let selected = null;
const selectValue = this.currentSelectValue;
const optionChildren = flattenOptionChildren(props.children);
if (selectValue != null) {
let value;
if (props.value != null) {
value = props.value + '';
} else {
value = optionChildren;
if (__DEV__) {
if (props.dangerouslySetInnerHTML != null) {
if (!didWarnInvalidOptionInnerHTML) {
didWarnInvalidOptionInnerHTML = true;
console.error(
'Pass a `value` prop if you set dangerouslyInnerHTML so React knows ' +
'which value should be selected.',
);
}
}
}
value = flattenOptionChildren(props.children);
}
selected = false;
if (isArray(selectValue)) {
@@ -1531,12 +1543,10 @@ class ReactDOMServerRenderer {
props = Object.assign(
{
selected: undefined,
children: undefined,
},
props,
{
selected: selected,
children: optionChildren,
},
);
}