From 1d2755a71757adc6b84fa3c12cf30d637ce52c93 Mon Sep 17 00:00:00 2001 From: raunofreiberg Date: Tue, 14 Aug 2018 15:18:29 +0300 Subject: [PATCH 1/4] wip: ignore symbols and functions in select tag --- .../src/__tests__/ReactDOMSelect-test.js | 264 ++++++++++++++++++ .../src/client/ReactDOMFiberOption.js | 3 +- .../src/client/ReactDOMFiberSelect.js | 6 +- 3 files changed, 270 insertions(+), 3 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js index 4bb68cc92bec7..ea6b7faa0c426 100644 --- a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js @@ -808,4 +808,268 @@ describe('ReactDOMSelect', () => { expect(node.options[1].selected).toBe(false); // b expect(node.options[2].selected).toBe(false); // c }); + + describe('When given a Symbol value', () => { + it('treats initial Symbol value as an empty string', () => { + let node; + + expect( + () => + (node = ReactTestUtils.renderIntoDocument( + , + )), + ).toWarnDev('Invalid value for prop `value`'); + + expect(node.value).toBe(''); + }); + + it('treats updated Symbol value as an empty string', () => { + let node; + + expect( + () => + (node = ReactTestUtils.renderIntoDocument( + , + )), + ).toWarnDev('Invalid value for prop `value`'); + + expect(node.value).toBe('monkey'); + + node = ReactTestUtils.renderIntoDocument( + , + ); + + expect(node.value).toBe(''); + }); + + it('treats initial Symbol defaultValue as an empty string', () => { + let node; + + expect( + () => + (node = ReactTestUtils.renderIntoDocument( + , + )), + ).toWarnDev('Invalid value for prop `value`'); + + expect(node.value).toBe(''); + }); + + it('treats updated Symbol defaultValue as an empty string', () => { + let node; + + expect( + () => + (node = ReactTestUtils.renderIntoDocument( + , + )), + ).toWarnDev('Invalid value for prop `value`'); + + expect(node.value).toBe('monkey'); + + node = ReactTestUtils.renderIntoDocument( + , + ); + + expect(node.value).toBe(''); + }); + }); + + describe('When given a function value', () => { + let setUntrackedValue; + function dispatchEventOnNode(node, type) { + node.dispatchEvent(new Event(type, {bubbles: true, cancelable: true})); + } + + beforeEach(() => { + setUntrackedValue = Object.getOwnPropertyDescriptor( + HTMLSelectElement.prototype, + 'value', + ).set; + }); + + it('treats initial function value as an empty string', () => { + let node; + + expect( + () => + (node = ReactTestUtils.renderIntoDocument( + , + )), + ).toWarnDev('Invalid value for prop `value`'); + + expect(node.value).toBe(''); + }); + + it('treats initial function defaultValue as an empty string', () => { + let node; + + expect( + () => + (node = ReactTestUtils.renderIntoDocument( + , + )), + ).toWarnDev('Invalid value for prop `value`'); + + expect(node.value).toBe(''); + }); + + it('treats updated function value as an empty string', () => { + let node; + + expect( + () => + (node = ReactTestUtils.renderIntoDocument( + , + )), + ).toWarnDev('Invalid value for prop `value`'); + + expect(node.value).toBe('monkey'); + + node = ReactTestUtils.renderIntoDocument( + , + ); + + expect(node.value).toBe(''); + }); + + it('treats updated function defaultValue as an empty string', () => { + let node; + + expect( + () => + (node = ReactTestUtils.renderIntoDocument( + , + )), + ).toWarnDev('Invalid value for prop `value`'); + + expect(node.value).toBe('monkey'); + + node = ReactTestUtils.renderIntoDocument( + , + ); + + expect(node.value).toBe(''); + }); + + it('treats controlled function value as an empty string', () => { + let selectNode; + + class Form extends React.Component { + state = { + value: 'giraffe', + }; + + handleChange = e => this.setState({value: e.target.value}); + + render() { + return ( + + ); + } + } + + expect(() => ReactTestUtils.renderIntoDocument(
)).toWarnDev( + 'Invalid value for prop `value`', + ); + + expect(selectNode.value).toBe('giraffe'); + + setUntrackedValue.call(selectNode, () => {}); + dispatchEventOnNode(selectNode, 'change'); + + expect(selectNode.value).toBe(''); + }); + + it('treats controlled function value as an empty string', () => { + let selectNode; + + class Form extends React.Component { + state = { + value: 'giraffe', + }; + + handleChange = e => this.setState({value: e.target.value}); + + render() { + return ( + + ); + } + } + + expect(() => ReactTestUtils.renderIntoDocument()).toWarnDev( + 'Invalid value for prop `value`', + ); + + expect(selectNode.value).toBe('giraffe'); + + setUntrackedValue.call(selectNode, () => {}); + dispatchEventOnNode(selectNode, 'change'); + + expect(selectNode.value).toBe(''); + }); + }); }); diff --git a/packages/react-dom/src/client/ReactDOMFiberOption.js b/packages/react-dom/src/client/ReactDOMFiberOption.js index 5c2c561594ff9..9d905ceaaa018 100644 --- a/packages/react-dom/src/client/ReactDOMFiberOption.js +++ b/packages/react-dom/src/client/ReactDOMFiberOption.js @@ -10,6 +10,7 @@ import React from 'react'; import warning from 'shared/warning'; import {validateDOMNesting, updatedAncestorInfo} from './validateDOMNesting'; +import {getToStringValue, toString} from './ToStringValue'; let didWarnSelectedSetOnOption = false; @@ -73,7 +74,7 @@ export function validateProps(element: Element, props: Object) { export function postMountWrapper(element: Element, props: Object) { // value="" should make a value attribute (#6219) if (props.value != null) { - element.setAttribute('value', props.value); + element.setAttribute('value', toString(getToStringValue(props.value))); } } diff --git a/packages/react-dom/src/client/ReactDOMFiberSelect.js b/packages/react-dom/src/client/ReactDOMFiberSelect.js index dfe71a6ae999c..5918cca5975ec 100644 --- a/packages/react-dom/src/client/ReactDOMFiberSelect.js +++ b/packages/react-dom/src/client/ReactDOMFiberSelect.js @@ -12,6 +12,8 @@ import {getCurrentFiberOwnerNameInDevOrNull} from 'react-reconciler/src/ReactCur import warning from 'shared/warning'; import ReactControlledValuePropTypes from '../shared/ReactControlledValuePropTypes'; +import {getToStringValue, toString} from './ToStringValue'; +import type {ToStringValue} from './ToStringValue'; let didWarnValueDefaultValue; @@ -21,7 +23,7 @@ if (__DEV__) { type SelectWithWrapperState = HTMLSelectElement & { _wrapperState: { - initialValue: ?string, + initialValue: ToStringValue | void, wasMultiple: boolean, }, }; @@ -98,7 +100,7 @@ function updateOptions( } else { // Do not set `select.value` as exact behavior isn't consistent across all // browsers for all cases. - let selectedValue = '' + (propValue: string); + let selectedValue = toString(getToStringValue((propValue: any))); let defaultSelected = null; for (let i = 0; i < options.length; i++) { if (options[i].value === selectedValue) { From a6041ebbf4247b09beba368582349bdfe69c2f0c Mon Sep 17 00:00:00 2001 From: raunofreiberg Date: Tue, 14 Aug 2018 16:02:43 +0300 Subject: [PATCH 2/4] fix: Use ToStringValue as a maybe type --- packages/react-dom/src/client/ReactDOMFiberSelect.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-dom/src/client/ReactDOMFiberSelect.js b/packages/react-dom/src/client/ReactDOMFiberSelect.js index 5918cca5975ec..1c51a02376b18 100644 --- a/packages/react-dom/src/client/ReactDOMFiberSelect.js +++ b/packages/react-dom/src/client/ReactDOMFiberSelect.js @@ -23,7 +23,7 @@ if (__DEV__) { type SelectWithWrapperState = HTMLSelectElement & { _wrapperState: { - initialValue: ToStringValue | void, + initialValue: ?ToStringValue, wasMultiple: boolean, }, }; From 81ff1c9a12b8ac7c63226fc2e96e776af51d37cc Mon Sep 17 00:00:00 2001 From: raunofreiberg Date: Tue, 14 Aug 2018 16:30:37 +0300 Subject: [PATCH 3/4] refactor: remove unnecessary test --- .../src/__tests__/ReactDOMSelect-test.js | 84 ------------------- 1 file changed, 84 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js index ea6b7faa0c426..8c8173a6a2f76 100644 --- a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js @@ -900,18 +900,6 @@ describe('ReactDOMSelect', () => { }); describe('When given a function value', () => { - let setUntrackedValue; - function dispatchEventOnNode(node, type) { - node.dispatchEvent(new Event(type, {bubbles: true, cancelable: true})); - } - - beforeEach(() => { - setUntrackedValue = Object.getOwnPropertyDescriptor( - HTMLSelectElement.prototype, - 'value', - ).set; - }); - it('treats initial function value as an empty string', () => { let node; @@ -999,77 +987,5 @@ describe('ReactDOMSelect', () => { expect(node.value).toBe(''); }); - - it('treats controlled function value as an empty string', () => { - let selectNode; - - class Form extends React.Component { - state = { - value: 'giraffe', - }; - - handleChange = e => this.setState({value: e.target.value}); - - render() { - return ( - - ); - } - } - - expect(() => ReactTestUtils.renderIntoDocument()).toWarnDev( - 'Invalid value for prop `value`', - ); - - expect(selectNode.value).toBe('giraffe'); - - setUntrackedValue.call(selectNode, () => {}); - dispatchEventOnNode(selectNode, 'change'); - - expect(selectNode.value).toBe(''); - }); - - it('treats controlled function value as an empty string', () => { - let selectNode; - - class Form extends React.Component { - state = { - value: 'giraffe', - }; - - handleChange = e => this.setState({value: e.target.value}); - - render() { - return ( - - ); - } - } - - expect(() => ReactTestUtils.renderIntoDocument()).toWarnDev( - 'Invalid value for prop `value`', - ); - - expect(selectNode.value).toBe('giraffe'); - - setUntrackedValue.call(selectNode, () => {}); - dispatchEventOnNode(selectNode, 'change'); - - expect(selectNode.value).toBe(''); - }); }); }); From 48dcdefe78b00efe382209dde4f635acfef584cd Mon Sep 17 00:00:00 2001 From: raunofreiberg Date: Tue, 14 Aug 2018 18:12:51 +0300 Subject: [PATCH 4/4] refactor: remove implicit return from tests --- .../src/__tests__/ReactDOMSelect-test.js | 152 +++++++++--------- 1 file changed, 72 insertions(+), 80 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js index 8c8173a6a2f76..20dc76053880f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js @@ -813,16 +813,15 @@ describe('ReactDOMSelect', () => { it('treats initial Symbol value as an empty string', () => { let node; - expect( - () => - (node = ReactTestUtils.renderIntoDocument( - , - )), - ).toWarnDev('Invalid value for prop `value`'); + expect(() => { + node = ReactTestUtils.renderIntoDocument( + , + ); + }).toWarnDev('Invalid value for prop `value`'); expect(node.value).toBe(''); }); @@ -830,16 +829,15 @@ describe('ReactDOMSelect', () => { it('treats updated Symbol value as an empty string', () => { let node; - expect( - () => - (node = ReactTestUtils.renderIntoDocument( - , - )), - ).toWarnDev('Invalid value for prop `value`'); + expect(() => { + node = ReactTestUtils.renderIntoDocument( + , + ); + }).toWarnDev('Invalid value for prop `value`'); expect(node.value).toBe('monkey'); @@ -857,16 +855,15 @@ describe('ReactDOMSelect', () => { it('treats initial Symbol defaultValue as an empty string', () => { let node; - expect( - () => - (node = ReactTestUtils.renderIntoDocument( - , - )), - ).toWarnDev('Invalid value for prop `value`'); + expect(() => { + node = ReactTestUtils.renderIntoDocument( + , + ); + }).toWarnDev('Invalid value for prop `value`'); expect(node.value).toBe(''); }); @@ -874,16 +871,15 @@ describe('ReactDOMSelect', () => { it('treats updated Symbol defaultValue as an empty string', () => { let node; - expect( - () => - (node = ReactTestUtils.renderIntoDocument( - , - )), - ).toWarnDev('Invalid value for prop `value`'); + expect(() => { + node = ReactTestUtils.renderIntoDocument( + , + ); + }).toWarnDev('Invalid value for prop `value`'); expect(node.value).toBe('monkey'); @@ -903,16 +899,15 @@ describe('ReactDOMSelect', () => { it('treats initial function value as an empty string', () => { let node; - expect( - () => - (node = ReactTestUtils.renderIntoDocument( - , - )), - ).toWarnDev('Invalid value for prop `value`'); + expect(() => { + node = ReactTestUtils.renderIntoDocument( + , + ); + }).toWarnDev('Invalid value for prop `value`'); expect(node.value).toBe(''); }); @@ -920,16 +915,15 @@ describe('ReactDOMSelect', () => { it('treats initial function defaultValue as an empty string', () => { let node; - expect( - () => - (node = ReactTestUtils.renderIntoDocument( - , - )), - ).toWarnDev('Invalid value for prop `value`'); + expect(() => { + node = ReactTestUtils.renderIntoDocument( + , + ); + }).toWarnDev('Invalid value for prop `value`'); expect(node.value).toBe(''); }); @@ -937,16 +931,15 @@ describe('ReactDOMSelect', () => { it('treats updated function value as an empty string', () => { let node; - expect( - () => - (node = ReactTestUtils.renderIntoDocument( - , - )), - ).toWarnDev('Invalid value for prop `value`'); + expect(() => { + node = ReactTestUtils.renderIntoDocument( + , + ); + }).toWarnDev('Invalid value for prop `value`'); expect(node.value).toBe('monkey'); @@ -964,16 +957,15 @@ describe('ReactDOMSelect', () => { it('treats updated function defaultValue as an empty string', () => { let node; - expect( - () => - (node = ReactTestUtils.renderIntoDocument( - , - )), - ).toWarnDev('Invalid value for prop `value`'); + expect(() => { + node = ReactTestUtils.renderIntoDocument( + , + ); + }).toWarnDev('Invalid value for prop `value`'); expect(node.value).toBe('monkey');