From 3203e0eb1932d19d830de7514a294bcbe8f71d74 Mon Sep 17 00:00:00 2001 From: Maxime Nory Date: Wed, 30 May 2018 18:57:23 +0200 Subject: [PATCH 1/4] Set the correct initial value on input range --- packages/react-dom/src/client/ReactDOMFiberInput.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-dom/src/client/ReactDOMFiberInput.js b/packages/react-dom/src/client/ReactDOMFiberInput.js index 475b444b55dbf..fb57c0c7714f6 100644 --- a/packages/react-dom/src/client/ReactDOMFiberInput.js +++ b/packages/react-dom/src/client/ReactDOMFiberInput.js @@ -210,7 +210,7 @@ export function postMountWrapper(element: Element, props: Object) { if (props.hasOwnProperty('value') || props.hasOwnProperty('defaultValue')) { const initialValue = '' + node._wrapperState.initialValue; - const currentValue = node.value; + const currentValue = props.type === 'range' ? '' : node.value; // Do not assign value if it is already set. This prevents user text input // from being lost during SSR hydration. From 1684cfcc78f972c136ab9b799ff582a32c6c8db7 Mon Sep 17 00:00:00 2001 From: Maxime Nory Date: Wed, 30 May 2018 22:19:47 +0200 Subject: [PATCH 2/4] Add description and update value diff check for input range --- packages/react-dom/src/client/ReactDOMFiberInput.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/react-dom/src/client/ReactDOMFiberInput.js b/packages/react-dom/src/client/ReactDOMFiberInput.js index fb57c0c7714f6..f0bf363fc68d0 100644 --- a/packages/react-dom/src/client/ReactDOMFiberInput.js +++ b/packages/react-dom/src/client/ReactDOMFiberInput.js @@ -210,6 +210,10 @@ export function postMountWrapper(element: Element, props: Object) { if (props.hasOwnProperty('value') || props.hasOwnProperty('defaultValue')) { const initialValue = '' + node._wrapperState.initialValue; + + // With range inputs node.value may be a default value calculated from the + // min/max attributes. This ensures that node.value is set with the correct + // value coming from props. const currentValue = props.type === 'range' ? '' : node.value; // Do not assign value if it is already set. This prevents user text input @@ -220,6 +224,8 @@ export function postMountWrapper(element: Element, props: Object) { // prematurely marking required inputs as invalid if (initialValue !== currentValue) { node.value = initialValue; + } else if (props.type === 'range') { + node.value = initialValue; } } From cc2d60958367657cd2bd8c5a16877bb5e615b510 Mon Sep 17 00:00:00 2001 From: Maxime Nory Date: Thu, 31 May 2018 09:50:26 +0200 Subject: [PATCH 3/4] add isHydrating argument and tests --- .../ReactDOMServerIntegrationForms-test.js | 30 +++++++++++++++++-- .../src/client/ReactDOMFiberComponent.js | 4 +-- .../src/client/ReactDOMFiberInput.js | 24 +++++++++------ 3 files changed, 45 insertions(+), 13 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationForms-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationForms-test.js index fc4594261eba2..2d873d20c61ed 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationForms-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationForms-test.js @@ -348,9 +348,13 @@ describe('ReactDOMServerIntegration', () => { ControlledSelect; beforeEach(() => { ControlledInput = class extends React.Component { + static defaultProps = { + type: 'text', + initialValue: 'Hello', + }; constructor() { - super(); - this.state = {value: 'Hello'}; + super(...arguments); + this.state = {value: this.props.initialValue}; } handleChange(event) { if (this.props.onChange) { @@ -361,6 +365,7 @@ describe('ReactDOMServerIntegration', () => { render() { return ( @@ -551,6 +556,27 @@ describe('ReactDOMServerIntegration', () => { expect(changeCount).toBe(0); }); + it('should not blow away user-interaction on successful reconnect to an uncontrolled range input', () => + testUserInteractionBeforeClientRender( + , + '0.5', + '1', + )); + + it('should not blow away user-interaction on successful reconnect to a controlled range input', async () => { + let changeCount = 0; + await testUserInteractionBeforeClientRender( + changeCount++} + />, + '0.25', + '1', + ); + expect(changeCount).toBe(0); + }); + it('should not blow away user-entered text on successful reconnect to an uncontrolled checkbox', () => testUserInteractionBeforeClientRender( , diff --git a/packages/react-dom/src/client/ReactDOMFiberComponent.js b/packages/react-dom/src/client/ReactDOMFiberComponent.js index 3c9e802b6e34e..e089b33098520 100644 --- a/packages/react-dom/src/client/ReactDOMFiberComponent.js +++ b/packages/react-dom/src/client/ReactDOMFiberComponent.js @@ -530,7 +530,7 @@ export function setInitialProperties( // TODO: Make sure we check if this is still unmounted or do any clean // up necessary since we never stop tracking anymore. inputValueTracking.track((domElement: any)); - ReactDOMFiberInput.postMountWrapper(domElement, rawProps); + ReactDOMFiberInput.postMountWrapper(domElement, rawProps, false); break; case 'textarea': // TODO: Make sure we check if this is still unmounted or do any clean @@ -1077,7 +1077,7 @@ export function diffHydratedProperties( // TODO: Make sure we check if this is still unmounted or do any clean // up necessary since we never stop tracking anymore. inputValueTracking.track((domElement: any)); - ReactDOMFiberInput.postMountWrapper(domElement, rawProps); + ReactDOMFiberInput.postMountWrapper(domElement, rawProps, true); break; case 'textarea': // TODO: Make sure we check if this is still unmounted or do any clean diff --git a/packages/react-dom/src/client/ReactDOMFiberInput.js b/packages/react-dom/src/client/ReactDOMFiberInput.js index f0bf363fc68d0..65fec82b45e4c 100644 --- a/packages/react-dom/src/client/ReactDOMFiberInput.js +++ b/packages/react-dom/src/client/ReactDOMFiberInput.js @@ -205,27 +205,33 @@ export function updateWrapper(element: Element, props: Object) { } } -export function postMountWrapper(element: Element, props: Object) { +export function postMountWrapper( + element: Element, + props: Object, + isHydrating: boolean, +) { const node = ((element: any): InputWithWrapperState); if (props.hasOwnProperty('value') || props.hasOwnProperty('defaultValue')) { const initialValue = '' + node._wrapperState.initialValue; - - // With range inputs node.value may be a default value calculated from the - // min/max attributes. This ensures that node.value is set with the correct - // value coming from props. - const currentValue = props.type === 'range' ? '' : node.value; + let currentValue; + if (isHydrating) { + currentValue = node.value; + } else { + // With range inputs node.value may be a default value calculated from the + // min/max attributes. This ensures that node.value is set with the correct + // value coming from props. + currentValue = props.type === 'range' ? '' : node.value; + } // Do not assign value if it is already set. This prevents user text input // from being lost during SSR hydration. - if (currentValue === '') { + if (!node.hasAttribute('value')) { // Do not re-assign the value property if there is no change. This // potentially avoids a DOM write and prevents Firefox (~60.0.1) from // prematurely marking required inputs as invalid if (initialValue !== currentValue) { node.value = initialValue; - } else if (props.type === 'range') { - node.value = initialValue; } } From 745ac8654cf7cbff012d82f699e6c29717c6a478 Mon Sep 17 00:00:00 2001 From: Maxime Nory Date: Thu, 31 May 2018 15:30:21 +0200 Subject: [PATCH 4/4] update node value according to isHydrating --- packages/react-dom/src/client/ReactDOMFiberInput.js | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOMFiberInput.js b/packages/react-dom/src/client/ReactDOMFiberInput.js index 65fec82b45e4c..9483279516232 100644 --- a/packages/react-dom/src/client/ReactDOMFiberInput.js +++ b/packages/react-dom/src/client/ReactDOMFiberInput.js @@ -214,19 +214,11 @@ export function postMountWrapper( if (props.hasOwnProperty('value') || props.hasOwnProperty('defaultValue')) { const initialValue = '' + node._wrapperState.initialValue; - let currentValue; - if (isHydrating) { - currentValue = node.value; - } else { - // With range inputs node.value may be a default value calculated from the - // min/max attributes. This ensures that node.value is set with the correct - // value coming from props. - currentValue = props.type === 'range' ? '' : node.value; - } + const currentValue = node.value; // Do not assign value if it is already set. This prevents user text input // from being lost during SSR hydration. - if (!node.hasAttribute('value')) { + if (!isHydrating) { // Do not re-assign the value property if there is no change. This // potentially avoids a DOM write and prevents Firefox (~60.0.1) from // prematurely marking required inputs as invalid