@samitouri / QOS-React / commits / a079011f95

🔥 Stop syncing the value attribute on inputs (behind a feature flag) (#13526)

* :fire: Stop syncing the value attribute on inputs * Eliminate some additional checks * Remove initialValue and initialWrapper from wrapperState flow type * Update tests with new sync logic, reduce some operations * Update tests, add some caveats for SSR mismatches * Revert newline change * Remove unused type * Call toString to safely type string values * Add disableInputAttributeSyncing feature flag Reverts tests to original state, adds attribute sync feature flag, then moves all affected tests to ReactFire-test.js. * Revert position of types in toStringValues * Invert flag on number input blur * Add clarification why double blur is necessary * Update ReactFire number cases to be more explicite about blur * Move comments to reduce diff size * Add comments to clarify behavior in each branch * There is no need to assign a different checked behavior in Fire * Use checked reference * Format * Avoid precomputing stringable values * Revert getToStringValue comment * Revert placement of undefined in getToStringValue * Do not eagerly stringify value * Unify Fire test cases with normal ones * Revert toString change. Only assign unsynced values when not nully

Nathan Hunzaker committed Sep 12, 2018 at 11:29 UTC a079011f95033758c17f1d7c2ee44d9cc80623f2

16 files changed +360 -84

packages/react-dom/src/__tests__/DOMPropertyOperations-test.js

+18 -3

@@ -9,6 +9,9 @@

10 'use strict';

12 +// Set by `yarn test-fire`.

13 +const {disableInputAttributeSyncing} = require('shared/ReactFeatureFlags');

14 +

15 describe('DOMPropertyOperations', () => {

16 let React;

17 let ReactDOM;

@@ -80,7 +83,11 @@ describe('DOMPropertyOperations', () => {

83 it('should not remove empty attributes for special input properties', () => {

84 const container = document.createElement('div');

85 ReactDOM.render(<input value="" onChange={() => {}} />, container);

83 - expect(container.firstChild.getAttribute('value')).toBe('');

86 + if (disableInputAttributeSyncing) {

87 + expect(container.firstChild.hasAttribute('value')).toBe(false);

88 + } else {

89 + expect(container.firstChild.getAttribute('value')).toBe('');

90 + }

91 expect(container.firstChild.value).toBe('');

92 });

@@ -165,7 +172,11 @@ describe('DOMPropertyOperations', () => {

172 <input type="text" value="foo" onChange={function() {}} />,

173 container,

174 );

168 - expect(container.firstChild.getAttribute('value')).toBe('foo');

175 + if (disableInputAttributeSyncing) {

176 + expect(container.firstChild.hasAttribute('value')).toBe(false);

177 + } else {

178 + expect(container.firstChild.getAttribute('value')).toBe('foo');

179 + }

180 expect(container.firstChild.value).toBe('foo');

181 expect(() =>

182 ReactDOM.render(

@@ -175,7 +186,11 @@ describe('DOMPropertyOperations', () => {

186 ).toWarnDev(

187 'A component is changing a controlled input of type text to be uncontrolled',

188 );

178 - expect(container.firstChild.getAttribute('value')).toBe('foo');

189 + if (disableInputAttributeSyncing) {

190 + expect(container.firstChild.hasAttribute('value')).toBe(false);

191 + } else {

192 + expect(container.firstChild.getAttribute('value')).toBe('foo');

193 + }

194 expect(container.firstChild.value).toBe('foo');

195 });

196

packages/react-dom/src/__tests__/ReactDOMInput-test.js

+197 -51

@@ -9,6 +9,9 @@

10 'use strict';

12 +// Set by `yarn test-fire`.

13 +const {disableInputAttributeSyncing} = require('shared/ReactFeatureFlags');

14 +

15 function emptyFunction() {}

17 describe('ReactDOMInput', () => {

@@ -230,11 +233,14 @@ describe('ReactDOMInput', () => {

233 const node = ReactDOM.render(stub, container);

234

235 setUntrackedValue.call(node, '2.0');

233 -

236 dispatchEventOnNode(node, 'input');

237

236 - expect(node.getAttribute('value')).toBe('2');

238 expect(node.value).toBe('2');

239 + if (disableInputAttributeSyncing) {

240 + expect(node.hasAttribute('value')).toBe(false);

241 + } else {

242 + expect(node.getAttribute('value')).toBe('2');

243 + }

244 });

245

246 it('does change the string "2" to "2.0" with no change handler', () => {

@@ -242,11 +248,14 @@ describe('ReactDOMInput', () => {

248 const node = ReactDOM.render(stub, container);

249

250 setUntrackedValue.call(node, '2.0');

245 -

251 dispatchEventOnNode(node, 'input');

252

248 - expect(node.getAttribute('value')).toBe('2');

253 expect(node.value).toBe('2');

254 + if (disableInputAttributeSyncing) {

255 + expect(node.hasAttribute('value')).toBe(false);

256 + } else {

257 + expect(node.getAttribute('value')).toBe('2');

258 + }

259 });

260

261 it('changes the number 2 to "2.0" using a change handler', () => {

@@ -268,11 +277,14 @@ describe('ReactDOMInput', () => {

277 const node = ReactDOM.findDOMNode(stub);

278

279 setUntrackedValue.call(node, '2.0');

271 -

280 dispatchEventOnNode(node, 'input');

281

274 - expect(node.getAttribute('value')).toBe('2.0');

282 expect(node.value).toBe('2.0');

283 + if (disableInputAttributeSyncing) {

284 + expect(node.hasAttribute('value')).toBe(false);

285 + } else {

286 + expect(node.getAttribute('value')).toBe('2.0');

287 + }

288 });

289 });

290

@@ -419,11 +431,17 @@ describe('ReactDOMInput', () => {

431 );

432

433 expect(node.value).toBe('0');

434 + expect(node.defaultValue).toBe('0');

435

436 ReactDOM.render(<input type="text" defaultValue="1" />, container);

437

425 - expect(node.value).toBe('0');

426 - expect(node.defaultValue).toBe('1');

438 + if (disableInputAttributeSyncing) {

439 + expect(node.value).toBe('1');

440 + expect(node.defaultValue).toBe('1');

441 + } else {

442 + expect(node.value).toBe('0');

443 + expect(node.defaultValue).toBe('1');

444 + }

445 });

446

447 it('should update `defaultValue` for uncontrolled date/time input', () => {

@@ -433,11 +451,17 @@ describe('ReactDOMInput', () => {

451 );

452

453 expect(node.value).toBe('1980-01-01');

454 + expect(node.defaultValue).toBe('1980-01-01');

455

456 ReactDOM.render(<input type="date" defaultValue="2000-01-01" />, container);

457

439 - expect(node.value).toBe('1980-01-01');

440 - expect(node.defaultValue).toBe('2000-01-01');

458 + if (disableInputAttributeSyncing) {

459 + expect(node.value).toBe('2000-01-01');

460 + expect(node.defaultValue).toBe('2000-01-01');

461 + } else {

462 + expect(node.value).toBe('1980-01-01');

463 + expect(node.defaultValue).toBe('2000-01-01');

464 + }

465

466 ReactDOM.render(<input type="date" />, container);

467 });

@@ -651,7 +675,16 @@ describe('ReactDOMInput', () => {

675

676 setUntrackedValue.call(node, '0.0');

677 dispatchEventOnNode(node, 'input');

654 - expect(node.value).toBe('0.0');

678 +

679 + if (disableInputAttributeSyncing) {

680 + expect(node.value).toBe('0.0');

681 + expect(node.hasAttribute('value')).toBe(false);

682 + } else {

683 + dispatchEventOnNode(node, 'blur');

684 +

685 + expect(node.value).toBe('0.0');

686 + expect(node.getAttribute('value')).toBe('0.0');

687 + }

688 });

689

690 it('should properly transition from an empty value to 0', function() {

@@ -665,9 +698,13 @@ describe('ReactDOMInput', () => {

698 );

699

700 const node = container.firstChild;

668 -

701 expect(node.value).toBe('0');

670 - expect(node.defaultValue).toBe('0');

702 +

703 + if (disableInputAttributeSyncing) {

704 + expect(node.hasAttribute('value')).toBe(false);

705 + } else {

706 + expect(node.defaultValue).toBe('0');

707 + }

708 });

709

710 it('should properly transition from 0 to an empty value', function() {

@@ -699,7 +736,11 @@ describe('ReactDOMInput', () => {

736 const node = container.firstChild;

737

738 expect(node.value).toBe('0.0');

702 - expect(node.defaultValue).toBe('0.0');

739 + if (disableInputAttributeSyncing) {

740 + expect(node.hasAttribute('value')).toBe(false);

741 + } else {

742 + expect(node.defaultValue).toBe('0.0');

743 + }

744 });

745

746 it('should properly transition a number input from "" to 0', function() {

@@ -715,7 +756,11 @@ describe('ReactDOMInput', () => {

756 const node = container.firstChild;

757

758 expect(node.value).toBe('0');

718 - expect(node.defaultValue).toBe('0');

759 + if (disableInputAttributeSyncing) {

760 + expect(node.hasAttribute('value')).toBe(false);

761 + } else {

762 + expect(node.defaultValue).toBe('0');

763 + }

764 });

765

766 it('should properly transition a number input from "" to "0"', function() {

@@ -731,7 +776,11 @@ describe('ReactDOMInput', () => {

776 const node = container.firstChild;

777

778 expect(node.value).toBe('0');

734 - expect(node.defaultValue).toBe('0');

779 + if (disableInputAttributeSyncing) {

780 + expect(node.hasAttribute('value')).toBe(false);

781 + } else {

782 + expect(node.defaultValue).toBe('0');

783 + }

784 });

785

786 it('should have the correct target value', () => {

@@ -956,21 +1005,34 @@ describe('ReactDOMInput', () => {

1005 const cNode = stub.refs.c;

1006

1007 expect(aNode.checked).toBe(true);

959 - expect(aNode.hasAttribute('checked')).toBe(true);

1008 expect(bNode.checked).toBe(false);

961 - expect(bNode.hasAttribute('checked')).toBe(false);

1009 // c is in a separate form and shouldn't be affected at all here

1010 expect(cNode.checked).toBe(true);

964 - expect(cNode.hasAttribute('checked')).toBe(true);

1011 +

1012 + if (disableInputAttributeSyncing) {

1013 + expect(aNode.hasAttribute('checked')).toBe(false);

1014 + expect(bNode.hasAttribute('checked')).toBe(false);

1015 + expect(cNode.hasAttribute('checked')).toBe(true);

1016 + } else {

1017 + expect(aNode.hasAttribute('checked')).toBe(true);

1018 + expect(bNode.hasAttribute('checked')).toBe(false);

1019 + expect(cNode.hasAttribute('checked')).toBe(true);

1020 + }

1021

1022 setUntrackedChecked.call(bNode, true);

1023 expect(aNode.checked).toBe(false);

1024 expect(cNode.checked).toBe(true);

1025

1026 // The original 'checked' attribute should be unchanged

971 - expect(aNode.hasAttribute('checked')).toBe(true);

972 - expect(bNode.hasAttribute('checked')).toBe(false);

973 - expect(cNode.hasAttribute('checked')).toBe(true);

1027 + if (disableInputAttributeSyncing) {

1028 + expect(aNode.hasAttribute('checked')).toBe(false);

1029 + expect(bNode.hasAttribute('checked')).toBe(false);

1030 + expect(cNode.hasAttribute('checked')).toBe(true);

1031 + } else {

1032 + expect(aNode.hasAttribute('checked')).toBe(true);

1033 + expect(bNode.hasAttribute('checked')).toBe(false);

1034 + expect(cNode.hasAttribute('checked')).toBe(true);

1035 + }

1036

1037 // Now let's run the actual ReactDOMInput change event handler

1038 dispatchEventOnNode(bNode, 'click');

@@ -1516,15 +1578,26 @@ describe('ReactDOMInput', () => {

1578 />,

1579 container,

1580 );

1519 - expect(log).toEqual([

1520 - 'set attribute type',

1521 - 'set attribute min',

1522 - 'set attribute max',

1523 - 'set attribute step',

1524 - 'set property value',

1525 - 'set attribute value',

1526 - 'set attribute checked',

1527 - ]);

1581 +

1582 + if (disableInputAttributeSyncing) {

1583 + expect(log).toEqual([

1584 + 'set attribute type',

1585 + 'set attribute min',

1586 + 'set attribute max',

1587 + 'set attribute step',

1588 + 'set property value',

1589 + ]);

1590 + } else {

1591 + expect(log).toEqual([

1592 + 'set attribute type',

1593 + 'set attribute min',

1594 + 'set attribute max',

1595 + 'set attribute step',

1596 + 'set property value',

1597 + 'set attribute value',

1598 + 'set attribute checked',

1599 + ]);

1600 + }

1601 });

1602

1603 it('sets value properly with type coming later in props', () => {

@@ -1579,12 +1652,20 @@ describe('ReactDOMInput', () => {

1652 });

1653

1654 ReactDOM.render(<input type="date" defaultValue="1980-01-01" />, container);

1582 - expect(log).toEqual([

1583 - 'node.setAttribute("type", "date")',

1584 - 'node.value = "1980-01-01"',

1585 - 'node.setAttribute("value", "1980-01-01")',

1586 - 'node.setAttribute("checked", "")',

1587 - ]);

1655 +

1656 + if (disableInputAttributeSyncing) {

1657 + expect(log).toEqual([

1658 + 'node.setAttribute("type", "date")',

1659 + 'node.setAttribute("value", "1980-01-01")',

1660 + ]);

1661 + } else {

1662 + expect(log).toEqual([

1663 + 'node.setAttribute("type", "date")',

1664 + 'node.value = "1980-01-01"',

1665 + 'node.setAttribute("value", "1980-01-01")',

1666 + 'node.setAttribute("checked", "")',

1667 + ]);

1668 + }

1669 });

1670

1671 describe('assigning the value attribute on controlled inputs', function() {

@@ -1613,7 +1694,11 @@ describe('ReactDOMInput', () => {

1694 setUntrackedValue.call(node, '2');

1695 dispatchEventOnNode(node, 'input');

1696

1616 - expect(node.getAttribute('value')).toBe('2');

1697 + if (disableInputAttributeSyncing) {

1698 + expect(node.hasAttribute('value')).toBe(false);

1699 + } else {

1700 + expect(node.getAttribute('value')).toBe('2');

1701 + }

1702 });

1703

1704 it('does not set the value attribute on number inputs if focused', () => {

@@ -1629,7 +1714,11 @@ describe('ReactDOMInput', () => {

1714 setUntrackedValue.call(node, '2');

1715 dispatchEventOnNode(node, 'input');

1716

1632 - expect(node.getAttribute('value')).toBe('1');

1717 + if (disableInputAttributeSyncing) {

1718 + expect(node.hasAttribute('value')).toBe(false);

1719 + } else {

1720 + expect(node.getAttribute('value')).toBe('1');

1721 + }

1722 });

1723

1724 it('sets the value attribute on number inputs on blur', () => {

@@ -1640,11 +1729,23 @@ describe('ReactDOMInput', () => {

1729 );

1730 const node = ReactDOM.findDOMNode(stub);

1731

1732 + node.focus();

1733 setUntrackedValue.call(node, '2');

1734 dispatchEventOnNode(node, 'input');

1735 + // TODO: it is unclear why blur must be triggered twice,

1736 + // manual testing in the fixtures shows that the active element

1737 + // is no longer the input, however blur() + a blur event seem to

1738 + // be the only way to remove focus in JSDOM

1739 + node.blur();

1740 dispatchEventOnNode(node, 'blur');

1741

1647 - expect(node.getAttribute('value')).toBe('2');

1742 + if (disableInputAttributeSyncing) {

1743 + expect(node.value).toBe('2');

1744 + expect(node.hasAttribute('value')).toBe(false);

1745 + } else {

1746 + expect(node.value).toBe('2');

1747 + expect(node.getAttribute('value')).toBe('2');

1748 + }

1749 });

1750

1751 it('an uncontrolled number input will not update the value attribute on blur', () => {

@@ -1653,8 +1754,14 @@ describe('ReactDOMInput', () => {

1754 container,

1755 );

1756

1757 + node.focus();

1758 setUntrackedValue.call(node, 4);

1657 -

1759 + dispatchEventOnNode(node, 'input');

1760 + // TODO: it is unclear why blur must be triggered twice,

1761 + // manual testing in the fixtures shows that the active element

1762 + // is no longer the input, however blur() + a blur event seem to

1763 + // be the only way to remove focus in JSDOM

1764 + node.blur();

1765 dispatchEventOnNode(node, 'blur');

1766

1767 expect(node.getAttribute('value')).toBe('1');

@@ -1666,8 +1773,14 @@ describe('ReactDOMInput', () => {

1773 container,

1774 );

1775

1776 + node.focus();

1777 setUntrackedValue.call(node, 4);

1670 -

1778 + dispatchEventOnNode(node, 'input');

1779 + // TODO: it is unclear why blur must be triggered twice,

1780 + // manual testing in the fixtures shows that the active element

1781 + // is no longer the input, however blur() + a blur event seem to

1782 + // be the only way to remove focus in JSDOM

1783 + node.blur();

1784 dispatchEventOnNode(node, 'blur');

1785

1786 expect(node.getAttribute('value')).toBe('1');

@@ -1707,7 +1820,11 @@ describe('ReactDOMInput', () => {

1820 'Input elements should not switch from controlled to ' +

1821 'uncontrolled (or vice versa).',

1822 );

1710 - expect(input.getAttribute('value')).toBe('first');

1823 + if (disableInputAttributeSyncing) {

1824 + expect(input.getAttribute('value')).toBe(null);

1825 + } else {

1826 + expect(input.getAttribute('value')).toBe('first');

1827 + }

1828 });

1829

1830 it('preserves the value property', () => {

@@ -1755,7 +1872,11 @@ describe('ReactDOMInput', () => {

1872 'Input elements should not switch from controlled ' +

1873 'to uncontrolled (or vice versa).',

1874 ]);

1758 - expect(input.getAttribute('value')).toBe('first');

1875 + if (disableInputAttributeSyncing) {

1876 + expect(input.hasAttribute('value')).toBe(false);

1877 + } else {

1878 + expect(input.getAttribute('value')).toBe('first');

1879 + }

1880 });

1881

1882 it('preserves the value property', () => {

@@ -1781,7 +1902,11 @@ describe('ReactDOMInput', () => {

1902 const node = container.firstChild;

1903

1904 expect(node.value).toBe('');

1784 - expect(node.getAttribute('value')).toBe('');

1905 + if (disableInputAttributeSyncing) {

1906 + expect(node.hasAttribute('value')).toBe(false);

1907 + } else {

1908 + expect(node.getAttribute('value')).toBe('');

1909 + }

1910 });

1911

1912 it('treats updated Symbol value as an empty string', function() {

@@ -1795,7 +1920,11 @@ describe('ReactDOMInput', () => {

1920 const node = container.firstChild;

1921

1922 expect(node.value).toBe('');

1798 - expect(node.getAttribute('value')).toBe('');

1923 + if (disableInputAttributeSyncing) {

1924 + expect(node.hasAttribute('value')).toBe(false);

1925 + } else {

1926 + expect(node.getAttribute('value')).toBe('');

1927 + }

1928 });

1929

1930 it('treats initial Symbol defaultValue as an empty string', function() {

@@ -1812,7 +1941,11 @@ describe('ReactDOMInput', () => {

1941 ReactDOM.render(<input defaultValue={Symbol('foobar')} />, container);

1942 const node = container.firstChild;

1943

1815 - expect(node.value).toBe('foo');

1944 + if (disableInputAttributeSyncing) {

1945 + expect(node.value).toBe('');

1946 + } else {

1947 + expect(node.value).toBe('foo');

1948 + }

1949 expect(node.getAttribute('value')).toBe('');

1950 // TODO: we should warn here.

1951 });

@@ -1829,7 +1962,11 @@ describe('ReactDOMInput', () => {

1962 const node = container.firstChild;

1963

1964 expect(node.value).toBe('');

1832 - expect(node.getAttribute('value')).toBe('');

1965 + if (disableInputAttributeSyncing) {

1966 + expect(node.hasAttribute('value')).toBe(false);

1967 + } else {

1968 + expect(node.getAttribute('value')).toBe('');

1969 + }

1970 });

1971

1972 it('treats updated function value as an empty string', function() {

@@ -1843,7 +1980,11 @@ describe('ReactDOMInput', () => {

1980 const node = container.firstChild;

1981

1982 expect(node.value).toBe('');

1846 - expect(node.getAttribute('value')).toBe('');

1983 + if (disableInputAttributeSyncing) {

1984 + expect(node.hasAttribute('value')).toBe(false);

1985 + } else {

1986 + expect(node.getAttribute('value')).toBe('');

1987 + }

1988 });

1989

1990 it('treats initial function defaultValue as an empty string', function() {

@@ -1860,8 +2001,13 @@ describe('ReactDOMInput', () => {

2001 ReactDOM.render(<input defaultValue={() => {}} />, container);

2002 const node = container.firstChild;

2003

1863 - expect(node.value).toBe('foo');

1864 - expect(node.getAttribute('value')).toBe('');

2004 + if (disableInputAttributeSyncing) {

2005 + expect(node.value).toBe('');

2006 + expect(node.getAttribute('value')).toBe('');

2007 + } else {

2008 + expect(node.value).toBe('foo');

2009 + expect(node.getAttribute('value')).toBe('');

2010 + }

2011 // TODO: we should warn here.

2012 });

2013 });

packages/react-dom/src/__tests__/ReactDOMServerIntegrationCheckbox-test.js

+5 -1

@@ -10,6 +10,8 @@

10 'use strict';

12 const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegrationTestUtils');

13 +// Set by `yarn test-fire`.

14 +const {disableInputAttributeSyncing} = require('shared/ReactFeatureFlags');

16 let React;

17 let ReactDOM;

@@ -31,7 +33,9 @@ function initModules() {

34 const {resetModules, itRenders} = ReactDOMServerIntegrationUtils(initModules);

34 -describe('ReactDOMServerIntegrationCheckbox', () => {

36 +// TODO: Run this in React Fire mode after we figure out the SSR behavior.

37 +const desc = disableInputAttributeSyncing ? xdescribe : describe;

38 +desc('ReactDOMServerIntegrationCheckbox', () => {

39 beforeEach(() => {

40 resetModules();

41 });

packages/react-dom/src/__tests__/ReactDOMServerIntegrationInput-test.js

+5 -1

@@ -10,6 +10,8 @@

10 'use strict';

12 const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegrationTestUtils');

13 +// Set by `yarn test-fire`.

14 +const {disableInputAttributeSyncing} = require('shared/ReactFeatureFlags');

16 let React;

17 let ReactDOM;

@@ -31,7 +33,9 @@ function initModules() {

34 const {resetModules, itRenders} = ReactDOMServerIntegrationUtils(initModules);

34 -describe('ReactDOMServerIntegrationInput', () => {

36 +// TODO: Run this in React Fire mode after we figure out the SSR behavior.

37 +const desc = disableInputAttributeSyncing ? xdescribe : describe;

38 +desc('ReactDOMServerIntegrationInput', () => {

39 beforeEach(() => {

40 resetModules();

41 });

packages/react-dom/src/client/ReactDOMInput.js

+109 -23

@@ -17,6 +17,7 @@ import {getFiberCurrentPropsFromNode} from './ReactDOMComponentTree';

17 import {getToStringValue, toString} from './ToStringValue';

18 import ReactControlledValuePropTypes from '../shared/ReactControlledValuePropTypes';

19 import * as inputValueTracking from './inputValueTracking';

20 +import {disableInputAttributeSyncing} from 'shared/ReactFeatureFlags';

22 import type {ToStringValue} from './ToStringValue';

@@ -194,14 +195,41 @@ export function updateWrapper(element: Element, props: Object) {

195 return;

196 }

197

197 - if (props.hasOwnProperty('value')) {

198 - setDefaultValue(node, props.type, value);

199 - } else if (props.hasOwnProperty('defaultValue')) {

200 - setDefaultValue(node, props.type, getToStringValue(props.defaultValue));

198 + if (disableInputAttributeSyncing) {

199 + // When not syncing the value attribute, React only assigns a new value

200 + // whenever the defaultValue React prop has changed. When not present,

201 + // React does nothing

202 + if (props.hasOwnProperty('defaultValue')) {

203 + setDefaultValue(node, props.type, getToStringValue(props.defaultValue));

204 + }

205 + } else {

206 + // When syncing the value attribute, the value comes from a cascade of

207 + // properties:

208 + // 1. The value React property

209 + // 2. The defaultValue React property

210 + // 3. Otherwise there should be no change

211 + if (props.hasOwnProperty('value')) {

212 + setDefaultValue(node, props.type, value);

213 + } else if (props.hasOwnProperty('defaultValue')) {

214 + setDefaultValue(node, props.type, getToStringValue(props.defaultValue));

215 + }

216 }

217

203 - if (props.checked == null && props.defaultChecked != null) {

204 - node.defaultChecked = !!props.defaultChecked;

218 + if (disableInputAttributeSyncing) {

219 + // When not syncing the checked attribute, the attribute is directly

220 + // controllable from the defaultValue React property. It needs to be

221 + // updated as new props come in.

222 + if (props.defaultChecked == null) {

223 + node.removeAttribute('checked');

224 + } else {

225 + node.defaultChecked = !!props.defaultChecked;

226 + }

227 + } else {

228 + // When syncing the checked attribute, it only changes when it needs

229 + // to be removed, such as transitioning from a checkbox into a text input

230 + if (props.checked == null && props.defaultChecked != null) {

231 + node.defaultChecked = !!props.defaultChecked;

232 + }

233 }

234 }

235

@@ -212,35 +240,67 @@ export function postMountWrapper(

240 ) {

241 const node = ((element: any): InputWithWrapperState);

242

243 + // Do not assign value if it is already set. This prevents user text input

244 + // from being lost during SSR hydration.

245 if (props.hasOwnProperty('value') || props.hasOwnProperty('defaultValue')) {

246 + const type = props.type;

247 + const isButton = type === 'submit' || type === 'reset';

248 +

249 // Avoid setting value attribute on submit/reset inputs as it overrides the

250 // default value provided by the browser. See: #12872

218 - const type = props.type;

219 - if (

220 - (type === 'submit' || type === 'reset') &&

221 - (props.value === undefined || props.value === null)

222 - ) {

251 + if (isButton && (props.value === undefined || props.value === null)) {

252 return;

253 }

254

255 const initialValue = toString(node._wrapperState.initialValue);

227 - const currentValue = node.value;

256

257 // Do not assign value if it is already set. This prevents user text input

258 // from being lost during SSR hydration.

259 if (!isHydrating) {

232 - // Do not re-assign the value property if there is no change. This

233 - // potentially avoids a DOM write and prevents Firefox (~60.0.1) from

234 - // prematurely marking required inputs as invalid

235 - if (initialValue !== currentValue) {

236 - node.value = initialValue;

260 + if (disableInputAttributeSyncing) {

261 + const value = getToStringValue(props.value);

262 +

263 + // When not syncing the value attribute, the value property points

264 + // directly to the React prop. Only assign it if it exists.

265 + if (value != null) {

266 + // Always assign on buttons so that it is possible to assign an

267 + // empty string to clear button text.

268 + //

269 + // Otherwise, do not re-assign the value property if is empty. This

270 + // potentially avoids a DOM write and prevents Firefox (~60.0.1) from

271 + // prematurely marking required inputs as invalid. Equality is compared

272 + // to the current value in case the browser provided value is not an

273 + // empty string.

274 + if (isButton || value !== node.value) {

275 + node.value = toString(value);

276 + }

277 + }

278 + } else {

279 + // When syncing the value attribute, the value property should use

280 + // the the wrapperState._initialValue property. This uses:

281 + //

282 + // 1. The value React property when present

283 + // 2. The defaultValue React property when present

284 + // 3. An empty string

285 + if (initialValue !== node.value) {

286 + node.value = initialValue;

287 + }

288 }

289 }

290

240 - // value must be assigned before defaultValue. This fixes an issue where the

241 - // visually displayed value of date inputs disappears on mobile Safari and Chrome:

242 - // https://github.com/facebook/react/issues/7233

243 - node.defaultValue = initialValue;

291 + if (disableInputAttributeSyncing) {

292 + // When not syncing the value attribute, assign the value attribute

293 + // directly from the defaultValue React property (when present)

294 + const defaultValue = getToStringValue(props.defaultValue);

295 + if (defaultValue != null) {

296 + node.defaultValue = toString(defaultValue);

297 + }

298 + } else {

299 + // Otherwise, the value attribute is synchronized to the property,

300 + // so we assign defaultValue to the same thing as the value property

301 + // assignment step above.

302 + node.defaultValue = initialValue;

303 + }

304 }

305

306 // Normally, we'd just do `node.checked = node.checked` upon initial mount, less this bug

@@ -252,8 +312,34 @@ export function postMountWrapper(

312 if (name !== '') {

313 node.name = '';

314 }

255 - node.defaultChecked = !node.defaultChecked;

256 - node.defaultChecked = !!node._wrapperState.initialChecked;

315 +

316 + if (disableInputAttributeSyncing) {

317 + // When not syncing the checked attribute, the checked property

318 + // never gets assigned. It must be manually set. We don't want

319 + // to do this when hydrating so that existing user input isn't

320 + // modified

321 + if (!isHydrating) {

322 + updateChecked(element, props);

323 + }

324 +

325 + // Only assign the checked attribute if it is defined. This saves

326 + // a DOM write when controlling the checked attribute isn't needed

327 + // (text inputs, submit/reset)

328 + if (props.hasOwnProperty('defaultChecked')) {

329 + node.defaultChecked = !node.defaultChecked;

330 + node.defaultChecked = !!props.defaultChecked;

331 + }

332 + } else {

333 + // When syncing the checked attribute, both the the checked property and

334 + // attribute are assigned at the same time using defaultChecked. This uses:

335 + //

336 + // 1. The checked React property when present

337 + // 2. The defaultChecked React property when present

338 + // 3. Otherwise, false

339 + node.defaultChecked = !node.defaultChecked;

340 + node.defaultChecked = !!node._wrapperState.initialChecked;

341 + }

342 +

343 if (name !== '') {

344 node.name = name;

345 }

packages/react-dom/src/events/ChangeEventPlugin.js

+5 -2

@@ -28,6 +28,7 @@ import isEventSupported from './isEventSupported';

28 import {getNodeFromInstance} from '../client/ReactDOMComponentTree';

29 import * as inputValueTracking from '../client/inputValueTracking';

30 import {setDefaultValue} from '../client/ReactDOMInput';

31 +import {disableInputAttributeSyncing} from 'shared/ReactFeatureFlags';

33 const eventTypes = {

34 change: {

@@ -238,8 +239,10 @@ function handleControlledInputBlur(node) {

239 return;

240 }

241

241 - // If controlled, assign the value attribute to the current value on blur

242 - setDefaultValue(node, 'number', node.value);

242 + if (!disableInputAttributeSyncing) {

243 + // If controlled, assign the value attribute to the current value on blur

244 + setDefaultValue(node, 'number', node.value);

245 + }

246 }

247

248 /**

packages/react-dom/src/events/__tests__/ChangeEventPlugin-test.internal.js

+9 -2

@@ -32,6 +32,7 @@ describe('ChangeEventPlugin', () => {

32 let container;

34 beforeEach(() => {

35 + ReactFeatureFlags = require('shared/ReactFeatureFlags');

36 // TODO pull this into helper method, reduce repetition.

37 // mock the browser APIs which are used in schedule:

38 // - requestAnimationFrame should pass the DOMHighResTimeStamp argument

@@ -87,8 +88,14 @@ describe('ChangeEventPlugin', () => {

88 );

89 node.dispatchEvent(new Event('input', {bubbles: true, cancelable: true}));

90 node.dispatchEvent(new Event('change', {bubbles: true, cancelable: true}));

90 - // There should be no React change events because the value stayed the same.

91 - expect(called).toBe(0);

91 +

92 + if (ReactFeatureFlags.disableInputAttributeSyncing) {

93 + // TODO: figure out why. This might be a bug.

94 + expect(called).toBe(1);

95 + } else {

96 + // There should be no React change events because the value stayed the same.

97 + expect(called).toBe(0);

98 + }

99 });

100

101 it('should consider initial checkbox checked=true to be current', () => {

packages/shared/ReactFeatureFlags.js

@@ -47,3 +47,7 @@ export const enableSuspenseServerRenderer = false;

47 export function addUserTimingListener() {

48 throw new Error('Not implemented.');

49 }

50 +

51 +// React Fire: prevent the value and checked attributes from syncing

52 +// with their related DOM properties

53 +export const disableInputAttributeSyncing = false;

packages/shared/forks/ReactFeatureFlags.native-fabric-fb.js

@@ -23,6 +23,7 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;

23 export const enableProfilerTimer = __PROFILE__;

24 export const enableSchedulerTracking = __PROFILE__;

25 export const enableSuspenseServerRenderer = false;

26 +export const disableInputAttributeSyncing = false;

28 // Only used in www builds.

29 export function addUserTimingListener() {

packages/shared/forks/ReactFeatureFlags.native-fabric-oss.js

@@ -23,6 +23,7 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;

23 export const enableProfilerTimer = __PROFILE__;

24 export const enableSchedulerTracking = __PROFILE__;

25 export const enableSuspenseServerRenderer = false;

26 +export const disableInputAttributeSyncing = false;

28 // Only used in www builds.

29 export function addUserTimingListener() {

packages/shared/forks/ReactFeatureFlags.native-fb.js

@@ -20,6 +20,7 @@ export const {

20 debugRenderPhaseSideEffectsForStrictMode,

21 warnAboutDeprecatedLifecycles,

22 replayFailedUnitOfWorkWithInvokeGuardedCallback,

23 + disableInputAttributeSyncing,

24 } = require('ReactFeatureFlags');

26 // The rest of the flags are static for better dead code elimination.

packages/shared/forks/ReactFeatureFlags.native-oss.js

@@ -23,6 +23,7 @@ export const warnAboutLegacyContextAPI = false;

23 export const enableProfilerTimer = __PROFILE__;

24 export const enableSchedulerTracking = __PROFILE__;

25 export const enableSuspenseServerRenderer = false;

26 +export const disableInputAttributeSyncing = false;

28 // Only used in www builds.

29 export function addUserTimingListener() {

packages/shared/forks/ReactFeatureFlags.persistent.js

@@ -23,6 +23,7 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;

23 export const enableProfilerTimer = __PROFILE__;

24 export const enableSchedulerTracking = __PROFILE__;

25 export const enableSuspenseServerRenderer = false;

26 +export const disableInputAttributeSyncing = false;

28 // Only used in www builds.

29 export function addUserTimingListener() {

packages/shared/forks/ReactFeatureFlags.test-renderer.js

@@ -23,6 +23,7 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = false;

23 export const enableProfilerTimer = false;

24 export const enableSchedulerTracking = false;

25 export const enableSuspenseServerRenderer = false;

26 +export const disableInputAttributeSyncing = false;

28 // Only used in www builds.

29 export function addUserTimingListener() {

packages/shared/forks/ReactFeatureFlags.www.js

@@ -19,6 +19,7 @@ export const {

19 enableSuspenseServerRenderer,

20 replayFailedUnitOfWorkWithInvokeGuardedCallback,

21 warnAboutDeprecatedLifecycles,

22 + disableInputAttributeSyncing,

23 } = require('ReactFeatureFlags');

25 // The rest of the flags are static for better dead code elimination.

scripts/jest/setupFire.js

+1 -1

@@ -2,6 +2,6 @@

3 jest.mock('shared/ReactFeatureFlags', () => {

4 const ReactFeatureFlags = require.requireActual('shared/ReactFeatureFlags');

5 - // TODO: Set feature flags for Fire

5 + ReactFeatureFlags.disableInputAttributeSyncing = true;

6 return ReactFeatureFlags;

7 });