mirror of
https://github.com/hanzoai/react.git
synced 2026-09-24 17:02:27 +00:00
🔥 Stop syncing the value attribute on inputs (behind a feature flag) (#13526)
* 🔥 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
This commit is contained in:
committed by
Dan Abramov
parent
fe0051ccc5
commit
0603f3f200
@@ -9,6 +9,9 @@
|
||||
|
||||
'use strict';
|
||||
|
||||
// Set by `yarn test-fire`.
|
||||
const {disableInputAttributeSyncing} = require('shared/ReactFeatureFlags');
|
||||
|
||||
describe('DOMPropertyOperations', () => {
|
||||
let React;
|
||||
let ReactDOM;
|
||||
@@ -80,7 +83,11 @@ describe('DOMPropertyOperations', () => {
|
||||
it('should not remove empty attributes for special input properties', () => {
|
||||
const container = document.createElement('div');
|
||||
ReactDOM.render(<input value="" onChange={() => {}} />, container);
|
||||
expect(container.firstChild.getAttribute('value')).toBe('');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(container.firstChild.hasAttribute('value')).toBe(false);
|
||||
} else {
|
||||
expect(container.firstChild.getAttribute('value')).toBe('');
|
||||
}
|
||||
expect(container.firstChild.value).toBe('');
|
||||
});
|
||||
|
||||
@@ -165,7 +172,11 @@ describe('DOMPropertyOperations', () => {
|
||||
<input type="text" value="foo" onChange={function() {}} />,
|
||||
container,
|
||||
);
|
||||
expect(container.firstChild.getAttribute('value')).toBe('foo');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(container.firstChild.hasAttribute('value')).toBe(false);
|
||||
} else {
|
||||
expect(container.firstChild.getAttribute('value')).toBe('foo');
|
||||
}
|
||||
expect(container.firstChild.value).toBe('foo');
|
||||
expect(() =>
|
||||
ReactDOM.render(
|
||||
@@ -175,7 +186,11 @@ describe('DOMPropertyOperations', () => {
|
||||
).toWarnDev(
|
||||
'A component is changing a controlled input of type text to be uncontrolled',
|
||||
);
|
||||
expect(container.firstChild.getAttribute('value')).toBe('foo');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(container.firstChild.hasAttribute('value')).toBe(false);
|
||||
} else {
|
||||
expect(container.firstChild.getAttribute('value')).toBe('foo');
|
||||
}
|
||||
expect(container.firstChild.value).toBe('foo');
|
||||
});
|
||||
|
||||
|
||||
+197
-51
@@ -9,6 +9,9 @@
|
||||
|
||||
'use strict';
|
||||
|
||||
// Set by `yarn test-fire`.
|
||||
const {disableInputAttributeSyncing} = require('shared/ReactFeatureFlags');
|
||||
|
||||
function emptyFunction() {}
|
||||
|
||||
describe('ReactDOMInput', () => {
|
||||
@@ -230,11 +233,14 @@ describe('ReactDOMInput', () => {
|
||||
const node = ReactDOM.render(stub, container);
|
||||
|
||||
setUntrackedValue.call(node, '2.0');
|
||||
|
||||
dispatchEventOnNode(node, 'input');
|
||||
|
||||
expect(node.getAttribute('value')).toBe('2');
|
||||
expect(node.value).toBe('2');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(node.hasAttribute('value')).toBe(false);
|
||||
} else {
|
||||
expect(node.getAttribute('value')).toBe('2');
|
||||
}
|
||||
});
|
||||
|
||||
it('does change the string "2" to "2.0" with no change handler', () => {
|
||||
@@ -242,11 +248,14 @@ describe('ReactDOMInput', () => {
|
||||
const node = ReactDOM.render(stub, container);
|
||||
|
||||
setUntrackedValue.call(node, '2.0');
|
||||
|
||||
dispatchEventOnNode(node, 'input');
|
||||
|
||||
expect(node.getAttribute('value')).toBe('2');
|
||||
expect(node.value).toBe('2');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(node.hasAttribute('value')).toBe(false);
|
||||
} else {
|
||||
expect(node.getAttribute('value')).toBe('2');
|
||||
}
|
||||
});
|
||||
|
||||
it('changes the number 2 to "2.0" using a change handler', () => {
|
||||
@@ -268,11 +277,14 @@ describe('ReactDOMInput', () => {
|
||||
const node = ReactDOM.findDOMNode(stub);
|
||||
|
||||
setUntrackedValue.call(node, '2.0');
|
||||
|
||||
dispatchEventOnNode(node, 'input');
|
||||
|
||||
expect(node.getAttribute('value')).toBe('2.0');
|
||||
expect(node.value).toBe('2.0');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(node.hasAttribute('value')).toBe(false);
|
||||
} else {
|
||||
expect(node.getAttribute('value')).toBe('2.0');
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -419,11 +431,17 @@ describe('ReactDOMInput', () => {
|
||||
);
|
||||
|
||||
expect(node.value).toBe('0');
|
||||
expect(node.defaultValue).toBe('0');
|
||||
|
||||
ReactDOM.render(<input type="text" defaultValue="1" />, container);
|
||||
|
||||
expect(node.value).toBe('0');
|
||||
expect(node.defaultValue).toBe('1');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(node.value).toBe('1');
|
||||
expect(node.defaultValue).toBe('1');
|
||||
} else {
|
||||
expect(node.value).toBe('0');
|
||||
expect(node.defaultValue).toBe('1');
|
||||
}
|
||||
});
|
||||
|
||||
it('should update `defaultValue` for uncontrolled date/time input', () => {
|
||||
@@ -433,11 +451,17 @@ describe('ReactDOMInput', () => {
|
||||
);
|
||||
|
||||
expect(node.value).toBe('1980-01-01');
|
||||
expect(node.defaultValue).toBe('1980-01-01');
|
||||
|
||||
ReactDOM.render(<input type="date" defaultValue="2000-01-01" />, container);
|
||||
|
||||
expect(node.value).toBe('1980-01-01');
|
||||
expect(node.defaultValue).toBe('2000-01-01');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(node.value).toBe('2000-01-01');
|
||||
expect(node.defaultValue).toBe('2000-01-01');
|
||||
} else {
|
||||
expect(node.value).toBe('1980-01-01');
|
||||
expect(node.defaultValue).toBe('2000-01-01');
|
||||
}
|
||||
|
||||
ReactDOM.render(<input type="date" />, container);
|
||||
});
|
||||
@@ -651,7 +675,16 @@ describe('ReactDOMInput', () => {
|
||||
|
||||
setUntrackedValue.call(node, '0.0');
|
||||
dispatchEventOnNode(node, 'input');
|
||||
expect(node.value).toBe('0.0');
|
||||
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(node.value).toBe('0.0');
|
||||
expect(node.hasAttribute('value')).toBe(false);
|
||||
} else {
|
||||
dispatchEventOnNode(node, 'blur');
|
||||
|
||||
expect(node.value).toBe('0.0');
|
||||
expect(node.getAttribute('value')).toBe('0.0');
|
||||
}
|
||||
});
|
||||
|
||||
it('should properly transition from an empty value to 0', function() {
|
||||
@@ -665,9 +698,13 @@ describe('ReactDOMInput', () => {
|
||||
);
|
||||
|
||||
const node = container.firstChild;
|
||||
|
||||
expect(node.value).toBe('0');
|
||||
expect(node.defaultValue).toBe('0');
|
||||
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(node.hasAttribute('value')).toBe(false);
|
||||
} else {
|
||||
expect(node.defaultValue).toBe('0');
|
||||
}
|
||||
});
|
||||
|
||||
it('should properly transition from 0 to an empty value', function() {
|
||||
@@ -699,7 +736,11 @@ describe('ReactDOMInput', () => {
|
||||
const node = container.firstChild;
|
||||
|
||||
expect(node.value).toBe('0.0');
|
||||
expect(node.defaultValue).toBe('0.0');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(node.hasAttribute('value')).toBe(false);
|
||||
} else {
|
||||
expect(node.defaultValue).toBe('0.0');
|
||||
}
|
||||
});
|
||||
|
||||
it('should properly transition a number input from "" to 0', function() {
|
||||
@@ -715,7 +756,11 @@ describe('ReactDOMInput', () => {
|
||||
const node = container.firstChild;
|
||||
|
||||
expect(node.value).toBe('0');
|
||||
expect(node.defaultValue).toBe('0');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(node.hasAttribute('value')).toBe(false);
|
||||
} else {
|
||||
expect(node.defaultValue).toBe('0');
|
||||
}
|
||||
});
|
||||
|
||||
it('should properly transition a number input from "" to "0"', function() {
|
||||
@@ -731,7 +776,11 @@ describe('ReactDOMInput', () => {
|
||||
const node = container.firstChild;
|
||||
|
||||
expect(node.value).toBe('0');
|
||||
expect(node.defaultValue).toBe('0');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(node.hasAttribute('value')).toBe(false);
|
||||
} else {
|
||||
expect(node.defaultValue).toBe('0');
|
||||
}
|
||||
});
|
||||
|
||||
it('should have the correct target value', () => {
|
||||
@@ -956,21 +1005,34 @@ describe('ReactDOMInput', () => {
|
||||
const cNode = stub.refs.c;
|
||||
|
||||
expect(aNode.checked).toBe(true);
|
||||
expect(aNode.hasAttribute('checked')).toBe(true);
|
||||
expect(bNode.checked).toBe(false);
|
||||
expect(bNode.hasAttribute('checked')).toBe(false);
|
||||
// c is in a separate form and shouldn't be affected at all here
|
||||
expect(cNode.checked).toBe(true);
|
||||
expect(cNode.hasAttribute('checked')).toBe(true);
|
||||
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(aNode.hasAttribute('checked')).toBe(false);
|
||||
expect(bNode.hasAttribute('checked')).toBe(false);
|
||||
expect(cNode.hasAttribute('checked')).toBe(true);
|
||||
} else {
|
||||
expect(aNode.hasAttribute('checked')).toBe(true);
|
||||
expect(bNode.hasAttribute('checked')).toBe(false);
|
||||
expect(cNode.hasAttribute('checked')).toBe(true);
|
||||
}
|
||||
|
||||
setUntrackedChecked.call(bNode, true);
|
||||
expect(aNode.checked).toBe(false);
|
||||
expect(cNode.checked).toBe(true);
|
||||
|
||||
// The original 'checked' attribute should be unchanged
|
||||
expect(aNode.hasAttribute('checked')).toBe(true);
|
||||
expect(bNode.hasAttribute('checked')).toBe(false);
|
||||
expect(cNode.hasAttribute('checked')).toBe(true);
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(aNode.hasAttribute('checked')).toBe(false);
|
||||
expect(bNode.hasAttribute('checked')).toBe(false);
|
||||
expect(cNode.hasAttribute('checked')).toBe(true);
|
||||
} else {
|
||||
expect(aNode.hasAttribute('checked')).toBe(true);
|
||||
expect(bNode.hasAttribute('checked')).toBe(false);
|
||||
expect(cNode.hasAttribute('checked')).toBe(true);
|
||||
}
|
||||
|
||||
// Now let's run the actual ReactDOMInput change event handler
|
||||
dispatchEventOnNode(bNode, 'click');
|
||||
@@ -1516,15 +1578,26 @@ describe('ReactDOMInput', () => {
|
||||
/>,
|
||||
container,
|
||||
);
|
||||
expect(log).toEqual([
|
||||
'set attribute type',
|
||||
'set attribute min',
|
||||
'set attribute max',
|
||||
'set attribute step',
|
||||
'set property value',
|
||||
'set attribute value',
|
||||
'set attribute checked',
|
||||
]);
|
||||
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(log).toEqual([
|
||||
'set attribute type',
|
||||
'set attribute min',
|
||||
'set attribute max',
|
||||
'set attribute step',
|
||||
'set property value',
|
||||
]);
|
||||
} else {
|
||||
expect(log).toEqual([
|
||||
'set attribute type',
|
||||
'set attribute min',
|
||||
'set attribute max',
|
||||
'set attribute step',
|
||||
'set property value',
|
||||
'set attribute value',
|
||||
'set attribute checked',
|
||||
]);
|
||||
}
|
||||
});
|
||||
|
||||
it('sets value properly with type coming later in props', () => {
|
||||
@@ -1579,12 +1652,20 @@ describe('ReactDOMInput', () => {
|
||||
});
|
||||
|
||||
ReactDOM.render(<input type="date" defaultValue="1980-01-01" />, container);
|
||||
expect(log).toEqual([
|
||||
'node.setAttribute("type", "date")',
|
||||
'node.value = "1980-01-01"',
|
||||
'node.setAttribute("value", "1980-01-01")',
|
||||
'node.setAttribute("checked", "")',
|
||||
]);
|
||||
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(log).toEqual([
|
||||
'node.setAttribute("type", "date")',
|
||||
'node.setAttribute("value", "1980-01-01")',
|
||||
]);
|
||||
} else {
|
||||
expect(log).toEqual([
|
||||
'node.setAttribute("type", "date")',
|
||||
'node.value = "1980-01-01"',
|
||||
'node.setAttribute("value", "1980-01-01")',
|
||||
'node.setAttribute("checked", "")',
|
||||
]);
|
||||
}
|
||||
});
|
||||
|
||||
describe('assigning the value attribute on controlled inputs', function() {
|
||||
@@ -1613,7 +1694,11 @@ describe('ReactDOMInput', () => {
|
||||
setUntrackedValue.call(node, '2');
|
||||
dispatchEventOnNode(node, 'input');
|
||||
|
||||
expect(node.getAttribute('value')).toBe('2');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(node.hasAttribute('value')).toBe(false);
|
||||
} else {
|
||||
expect(node.getAttribute('value')).toBe('2');
|
||||
}
|
||||
});
|
||||
|
||||
it('does not set the value attribute on number inputs if focused', () => {
|
||||
@@ -1629,7 +1714,11 @@ describe('ReactDOMInput', () => {
|
||||
setUntrackedValue.call(node, '2');
|
||||
dispatchEventOnNode(node, 'input');
|
||||
|
||||
expect(node.getAttribute('value')).toBe('1');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(node.hasAttribute('value')).toBe(false);
|
||||
} else {
|
||||
expect(node.getAttribute('value')).toBe('1');
|
||||
}
|
||||
});
|
||||
|
||||
it('sets the value attribute on number inputs on blur', () => {
|
||||
@@ -1640,11 +1729,23 @@ describe('ReactDOMInput', () => {
|
||||
);
|
||||
const node = ReactDOM.findDOMNode(stub);
|
||||
|
||||
node.focus();
|
||||
setUntrackedValue.call(node, '2');
|
||||
dispatchEventOnNode(node, 'input');
|
||||
// TODO: it is unclear why blur must be triggered twice,
|
||||
// manual testing in the fixtures shows that the active element
|
||||
// is no longer the input, however blur() + a blur event seem to
|
||||
// be the only way to remove focus in JSDOM
|
||||
node.blur();
|
||||
dispatchEventOnNode(node, 'blur');
|
||||
|
||||
expect(node.getAttribute('value')).toBe('2');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(node.value).toBe('2');
|
||||
expect(node.hasAttribute('value')).toBe(false);
|
||||
} else {
|
||||
expect(node.value).toBe('2');
|
||||
expect(node.getAttribute('value')).toBe('2');
|
||||
}
|
||||
});
|
||||
|
||||
it('an uncontrolled number input will not update the value attribute on blur', () => {
|
||||
@@ -1653,8 +1754,14 @@ describe('ReactDOMInput', () => {
|
||||
container,
|
||||
);
|
||||
|
||||
node.focus();
|
||||
setUntrackedValue.call(node, 4);
|
||||
|
||||
dispatchEventOnNode(node, 'input');
|
||||
// TODO: it is unclear why blur must be triggered twice,
|
||||
// manual testing in the fixtures shows that the active element
|
||||
// is no longer the input, however blur() + a blur event seem to
|
||||
// be the only way to remove focus in JSDOM
|
||||
node.blur();
|
||||
dispatchEventOnNode(node, 'blur');
|
||||
|
||||
expect(node.getAttribute('value')).toBe('1');
|
||||
@@ -1666,8 +1773,14 @@ describe('ReactDOMInput', () => {
|
||||
container,
|
||||
);
|
||||
|
||||
node.focus();
|
||||
setUntrackedValue.call(node, 4);
|
||||
|
||||
dispatchEventOnNode(node, 'input');
|
||||
// TODO: it is unclear why blur must be triggered twice,
|
||||
// manual testing in the fixtures shows that the active element
|
||||
// is no longer the input, however blur() + a blur event seem to
|
||||
// be the only way to remove focus in JSDOM
|
||||
node.blur();
|
||||
dispatchEventOnNode(node, 'blur');
|
||||
|
||||
expect(node.getAttribute('value')).toBe('1');
|
||||
@@ -1707,7 +1820,11 @@ describe('ReactDOMInput', () => {
|
||||
'Input elements should not switch from controlled to ' +
|
||||
'uncontrolled (or vice versa).',
|
||||
);
|
||||
expect(input.getAttribute('value')).toBe('first');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(input.getAttribute('value')).toBe(null);
|
||||
} else {
|
||||
expect(input.getAttribute('value')).toBe('first');
|
||||
}
|
||||
});
|
||||
|
||||
it('preserves the value property', () => {
|
||||
@@ -1755,7 +1872,11 @@ describe('ReactDOMInput', () => {
|
||||
'Input elements should not switch from controlled ' +
|
||||
'to uncontrolled (or vice versa).',
|
||||
]);
|
||||
expect(input.getAttribute('value')).toBe('first');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(input.hasAttribute('value')).toBe(false);
|
||||
} else {
|
||||
expect(input.getAttribute('value')).toBe('first');
|
||||
}
|
||||
});
|
||||
|
||||
it('preserves the value property', () => {
|
||||
@@ -1781,7 +1902,11 @@ describe('ReactDOMInput', () => {
|
||||
const node = container.firstChild;
|
||||
|
||||
expect(node.value).toBe('');
|
||||
expect(node.getAttribute('value')).toBe('');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(node.hasAttribute('value')).toBe(false);
|
||||
} else {
|
||||
expect(node.getAttribute('value')).toBe('');
|
||||
}
|
||||
});
|
||||
|
||||
it('treats updated Symbol value as an empty string', function() {
|
||||
@@ -1795,7 +1920,11 @@ describe('ReactDOMInput', () => {
|
||||
const node = container.firstChild;
|
||||
|
||||
expect(node.value).toBe('');
|
||||
expect(node.getAttribute('value')).toBe('');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(node.hasAttribute('value')).toBe(false);
|
||||
} else {
|
||||
expect(node.getAttribute('value')).toBe('');
|
||||
}
|
||||
});
|
||||
|
||||
it('treats initial Symbol defaultValue as an empty string', function() {
|
||||
@@ -1812,7 +1941,11 @@ describe('ReactDOMInput', () => {
|
||||
ReactDOM.render(<input defaultValue={Symbol('foobar')} />, container);
|
||||
const node = container.firstChild;
|
||||
|
||||
expect(node.value).toBe('foo');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(node.value).toBe('');
|
||||
} else {
|
||||
expect(node.value).toBe('foo');
|
||||
}
|
||||
expect(node.getAttribute('value')).toBe('');
|
||||
// TODO: we should warn here.
|
||||
});
|
||||
@@ -1829,7 +1962,11 @@ describe('ReactDOMInput', () => {
|
||||
const node = container.firstChild;
|
||||
|
||||
expect(node.value).toBe('');
|
||||
expect(node.getAttribute('value')).toBe('');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(node.hasAttribute('value')).toBe(false);
|
||||
} else {
|
||||
expect(node.getAttribute('value')).toBe('');
|
||||
}
|
||||
});
|
||||
|
||||
it('treats updated function value as an empty string', function() {
|
||||
@@ -1843,7 +1980,11 @@ describe('ReactDOMInput', () => {
|
||||
const node = container.firstChild;
|
||||
|
||||
expect(node.value).toBe('');
|
||||
expect(node.getAttribute('value')).toBe('');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(node.hasAttribute('value')).toBe(false);
|
||||
} else {
|
||||
expect(node.getAttribute('value')).toBe('');
|
||||
}
|
||||
});
|
||||
|
||||
it('treats initial function defaultValue as an empty string', function() {
|
||||
@@ -1860,8 +2001,13 @@ describe('ReactDOMInput', () => {
|
||||
ReactDOM.render(<input defaultValue={() => {}} />, container);
|
||||
const node = container.firstChild;
|
||||
|
||||
expect(node.value).toBe('foo');
|
||||
expect(node.getAttribute('value')).toBe('');
|
||||
if (disableInputAttributeSyncing) {
|
||||
expect(node.value).toBe('');
|
||||
expect(node.getAttribute('value')).toBe('');
|
||||
} else {
|
||||
expect(node.value).toBe('foo');
|
||||
expect(node.getAttribute('value')).toBe('');
|
||||
}
|
||||
// TODO: we should warn here.
|
||||
});
|
||||
});
|
||||
|
||||
@@ -10,6 +10,8 @@
|
||||
'use strict';
|
||||
|
||||
const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegrationTestUtils');
|
||||
// Set by `yarn test-fire`.
|
||||
const {disableInputAttributeSyncing} = require('shared/ReactFeatureFlags');
|
||||
|
||||
let React;
|
||||
let ReactDOM;
|
||||
@@ -31,7 +33,9 @@ function initModules() {
|
||||
|
||||
const {resetModules, itRenders} = ReactDOMServerIntegrationUtils(initModules);
|
||||
|
||||
describe('ReactDOMServerIntegrationCheckbox', () => {
|
||||
// TODO: Run this in React Fire mode after we figure out the SSR behavior.
|
||||
const desc = disableInputAttributeSyncing ? xdescribe : describe;
|
||||
desc('ReactDOMServerIntegrationCheckbox', () => {
|
||||
beforeEach(() => {
|
||||
resetModules();
|
||||
});
|
||||
|
||||
@@ -10,6 +10,8 @@
|
||||
'use strict';
|
||||
|
||||
const ReactDOMServerIntegrationUtils = require('./utils/ReactDOMServerIntegrationTestUtils');
|
||||
// Set by `yarn test-fire`.
|
||||
const {disableInputAttributeSyncing} = require('shared/ReactFeatureFlags');
|
||||
|
||||
let React;
|
||||
let ReactDOM;
|
||||
@@ -31,7 +33,9 @@ function initModules() {
|
||||
|
||||
const {resetModules, itRenders} = ReactDOMServerIntegrationUtils(initModules);
|
||||
|
||||
describe('ReactDOMServerIntegrationInput', () => {
|
||||
// TODO: Run this in React Fire mode after we figure out the SSR behavior.
|
||||
const desc = disableInputAttributeSyncing ? xdescribe : describe;
|
||||
desc('ReactDOMServerIntegrationInput', () => {
|
||||
beforeEach(() => {
|
||||
resetModules();
|
||||
});
|
||||
|
||||
+109
-23
@@ -17,6 +17,7 @@ import {getFiberCurrentPropsFromNode} from './ReactDOMComponentTree';
|
||||
import {getToStringValue, toString} from './ToStringValue';
|
||||
import ReactControlledValuePropTypes from '../shared/ReactControlledValuePropTypes';
|
||||
import * as inputValueTracking from './inputValueTracking';
|
||||
import {disableInputAttributeSyncing} from 'shared/ReactFeatureFlags';
|
||||
|
||||
import type {ToStringValue} from './ToStringValue';
|
||||
|
||||
@@ -194,14 +195,41 @@ export function updateWrapper(element: Element, props: Object) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (props.hasOwnProperty('value')) {
|
||||
setDefaultValue(node, props.type, value);
|
||||
} else if (props.hasOwnProperty('defaultValue')) {
|
||||
setDefaultValue(node, props.type, getToStringValue(props.defaultValue));
|
||||
if (disableInputAttributeSyncing) {
|
||||
// When not syncing the value attribute, React only assigns a new value
|
||||
// whenever the defaultValue React prop has changed. When not present,
|
||||
// React does nothing
|
||||
if (props.hasOwnProperty('defaultValue')) {
|
||||
setDefaultValue(node, props.type, getToStringValue(props.defaultValue));
|
||||
}
|
||||
} else {
|
||||
// When syncing the value attribute, the value comes from a cascade of
|
||||
// properties:
|
||||
// 1. The value React property
|
||||
// 2. The defaultValue React property
|
||||
// 3. Otherwise there should be no change
|
||||
if (props.hasOwnProperty('value')) {
|
||||
setDefaultValue(node, props.type, value);
|
||||
} else if (props.hasOwnProperty('defaultValue')) {
|
||||
setDefaultValue(node, props.type, getToStringValue(props.defaultValue));
|
||||
}
|
||||
}
|
||||
|
||||
if (props.checked == null && props.defaultChecked != null) {
|
||||
node.defaultChecked = !!props.defaultChecked;
|
||||
if (disableInputAttributeSyncing) {
|
||||
// When not syncing the checked attribute, the attribute is directly
|
||||
// controllable from the defaultValue React property. It needs to be
|
||||
// updated as new props come in.
|
||||
if (props.defaultChecked == null) {
|
||||
node.removeAttribute('checked');
|
||||
} else {
|
||||
node.defaultChecked = !!props.defaultChecked;
|
||||
}
|
||||
} else {
|
||||
// When syncing the checked attribute, it only changes when it needs
|
||||
// to be removed, such as transitioning from a checkbox into a text input
|
||||
if (props.checked == null && props.defaultChecked != null) {
|
||||
node.defaultChecked = !!props.defaultChecked;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -212,35 +240,67 @@ export function postMountWrapper(
|
||||
) {
|
||||
const node = ((element: any): InputWithWrapperState);
|
||||
|
||||
// Do not assign value if it is already set. This prevents user text input
|
||||
// from being lost during SSR hydration.
|
||||
if (props.hasOwnProperty('value') || props.hasOwnProperty('defaultValue')) {
|
||||
const type = props.type;
|
||||
const isButton = type === 'submit' || type === 'reset';
|
||||
|
||||
// Avoid setting value attribute on submit/reset inputs as it overrides the
|
||||
// default value provided by the browser. See: #12872
|
||||
const type = props.type;
|
||||
if (
|
||||
(type === 'submit' || type === 'reset') &&
|
||||
(props.value === undefined || props.value === null)
|
||||
) {
|
||||
if (isButton && (props.value === undefined || props.value === null)) {
|
||||
return;
|
||||
}
|
||||
|
||||
const initialValue = toString(node._wrapperState.initialValue);
|
||||
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 (!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
|
||||
if (initialValue !== currentValue) {
|
||||
node.value = initialValue;
|
||||
if (disableInputAttributeSyncing) {
|
||||
const value = getToStringValue(props.value);
|
||||
|
||||
// When not syncing the value attribute, the value property points
|
||||
// directly to the React prop. Only assign it if it exists.
|
||||
if (value != null) {
|
||||
// Always assign on buttons so that it is possible to assign an
|
||||
// empty string to clear button text.
|
||||
//
|
||||
// Otherwise, do not re-assign the value property if is empty. This
|
||||
// potentially avoids a DOM write and prevents Firefox (~60.0.1) from
|
||||
// prematurely marking required inputs as invalid. Equality is compared
|
||||
// to the current value in case the browser provided value is not an
|
||||
// empty string.
|
||||
if (isButton || value !== node.value) {
|
||||
node.value = toString(value);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// When syncing the value attribute, the value property should use
|
||||
// the the wrapperState._initialValue property. This uses:
|
||||
//
|
||||
// 1. The value React property when present
|
||||
// 2. The defaultValue React property when present
|
||||
// 3. An empty string
|
||||
if (initialValue !== node.value) {
|
||||
node.value = initialValue;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// value must be assigned before defaultValue. This fixes an issue where the
|
||||
// visually displayed value of date inputs disappears on mobile Safari and Chrome:
|
||||
// https://github.com/facebook/react/issues/7233
|
||||
node.defaultValue = initialValue;
|
||||
if (disableInputAttributeSyncing) {
|
||||
// When not syncing the value attribute, assign the value attribute
|
||||
// directly from the defaultValue React property (when present)
|
||||
const defaultValue = getToStringValue(props.defaultValue);
|
||||
if (defaultValue != null) {
|
||||
node.defaultValue = toString(defaultValue);
|
||||
}
|
||||
} else {
|
||||
// Otherwise, the value attribute is synchronized to the property,
|
||||
// so we assign defaultValue to the same thing as the value property
|
||||
// assignment step above.
|
||||
node.defaultValue = initialValue;
|
||||
}
|
||||
}
|
||||
|
||||
// Normally, we'd just do `node.checked = node.checked` upon initial mount, less this bug
|
||||
@@ -252,8 +312,34 @@ export function postMountWrapper(
|
||||
if (name !== '') {
|
||||
node.name = '';
|
||||
}
|
||||
node.defaultChecked = !node.defaultChecked;
|
||||
node.defaultChecked = !!node._wrapperState.initialChecked;
|
||||
|
||||
if (disableInputAttributeSyncing) {
|
||||
// When not syncing the checked attribute, the checked property
|
||||
// never gets assigned. It must be manually set. We don't want
|
||||
// to do this when hydrating so that existing user input isn't
|
||||
// modified
|
||||
if (!isHydrating) {
|
||||
updateChecked(element, props);
|
||||
}
|
||||
|
||||
// Only assign the checked attribute if it is defined. This saves
|
||||
// a DOM write when controlling the checked attribute isn't needed
|
||||
// (text inputs, submit/reset)
|
||||
if (props.hasOwnProperty('defaultChecked')) {
|
||||
node.defaultChecked = !node.defaultChecked;
|
||||
node.defaultChecked = !!props.defaultChecked;
|
||||
}
|
||||
} else {
|
||||
// When syncing the checked attribute, both the the checked property and
|
||||
// attribute are assigned at the same time using defaultChecked. This uses:
|
||||
//
|
||||
// 1. The checked React property when present
|
||||
// 2. The defaultChecked React property when present
|
||||
// 3. Otherwise, false
|
||||
node.defaultChecked = !node.defaultChecked;
|
||||
node.defaultChecked = !!node._wrapperState.initialChecked;
|
||||
}
|
||||
|
||||
if (name !== '') {
|
||||
node.name = name;
|
||||
}
|
||||
|
||||
+5
-2
@@ -28,6 +28,7 @@ import isEventSupported from './isEventSupported';
|
||||
import {getNodeFromInstance} from '../client/ReactDOMComponentTree';
|
||||
import * as inputValueTracking from '../client/inputValueTracking';
|
||||
import {setDefaultValue} from '../client/ReactDOMInput';
|
||||
import {disableInputAttributeSyncing} from 'shared/ReactFeatureFlags';
|
||||
|
||||
const eventTypes = {
|
||||
change: {
|
||||
@@ -238,8 +239,10 @@ function handleControlledInputBlur(node) {
|
||||
return;
|
||||
}
|
||||
|
||||
// If controlled, assign the value attribute to the current value on blur
|
||||
setDefaultValue(node, 'number', node.value);
|
||||
if (!disableInputAttributeSyncing) {
|
||||
// If controlled, assign the value attribute to the current value on blur
|
||||
setDefaultValue(node, 'number', node.value);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -32,6 +32,7 @@ describe('ChangeEventPlugin', () => {
|
||||
let container;
|
||||
|
||||
beforeEach(() => {
|
||||
ReactFeatureFlags = require('shared/ReactFeatureFlags');
|
||||
// TODO pull this into helper method, reduce repetition.
|
||||
// mock the browser APIs which are used in schedule:
|
||||
// - requestAnimationFrame should pass the DOMHighResTimeStamp argument
|
||||
@@ -87,8 +88,14 @@ describe('ChangeEventPlugin', () => {
|
||||
);
|
||||
node.dispatchEvent(new Event('input', {bubbles: true, cancelable: true}));
|
||||
node.dispatchEvent(new Event('change', {bubbles: true, cancelable: true}));
|
||||
// There should be no React change events because the value stayed the same.
|
||||
expect(called).toBe(0);
|
||||
|
||||
if (ReactFeatureFlags.disableInputAttributeSyncing) {
|
||||
// TODO: figure out why. This might be a bug.
|
||||
expect(called).toBe(1);
|
||||
} else {
|
||||
// There should be no React change events because the value stayed the same.
|
||||
expect(called).toBe(0);
|
||||
}
|
||||
});
|
||||
|
||||
it('should consider initial checkbox checked=true to be current', () => {
|
||||
|
||||
@@ -47,3 +47,7 @@ export const enableSuspenseServerRenderer = false;
|
||||
export function addUserTimingListener() {
|
||||
throw new Error('Not implemented.');
|
||||
}
|
||||
|
||||
// React Fire: prevent the value and checked attributes from syncing
|
||||
// with their related DOM properties
|
||||
export const disableInputAttributeSyncing = false;
|
||||
|
||||
@@ -23,6 +23,7 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
|
||||
export const enableProfilerTimer = __PROFILE__;
|
||||
export const enableSchedulerTracking = __PROFILE__;
|
||||
export const enableSuspenseServerRenderer = false;
|
||||
export const disableInputAttributeSyncing = false;
|
||||
|
||||
// Only used in www builds.
|
||||
export function addUserTimingListener() {
|
||||
|
||||
@@ -23,6 +23,7 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
|
||||
export const enableProfilerTimer = __PROFILE__;
|
||||
export const enableSchedulerTracking = __PROFILE__;
|
||||
export const enableSuspenseServerRenderer = false;
|
||||
export const disableInputAttributeSyncing = false;
|
||||
|
||||
// Only used in www builds.
|
||||
export function addUserTimingListener() {
|
||||
|
||||
@@ -20,6 +20,7 @@ export const {
|
||||
debugRenderPhaseSideEffectsForStrictMode,
|
||||
warnAboutDeprecatedLifecycles,
|
||||
replayFailedUnitOfWorkWithInvokeGuardedCallback,
|
||||
disableInputAttributeSyncing,
|
||||
} = require('ReactFeatureFlags');
|
||||
|
||||
// The rest of the flags are static for better dead code elimination.
|
||||
|
||||
@@ -23,6 +23,7 @@ export const warnAboutLegacyContextAPI = false;
|
||||
export const enableProfilerTimer = __PROFILE__;
|
||||
export const enableSchedulerTracking = __PROFILE__;
|
||||
export const enableSuspenseServerRenderer = false;
|
||||
export const disableInputAttributeSyncing = false;
|
||||
|
||||
// Only used in www builds.
|
||||
export function addUserTimingListener() {
|
||||
|
||||
@@ -23,6 +23,7 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
|
||||
export const enableProfilerTimer = __PROFILE__;
|
||||
export const enableSchedulerTracking = __PROFILE__;
|
||||
export const enableSuspenseServerRenderer = false;
|
||||
export const disableInputAttributeSyncing = false;
|
||||
|
||||
// Only used in www builds.
|
||||
export function addUserTimingListener() {
|
||||
|
||||
@@ -23,6 +23,7 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
|
||||
export const enableProfilerTimer = false;
|
||||
export const enableSchedulerTracking = false;
|
||||
export const enableSuspenseServerRenderer = false;
|
||||
export const disableInputAttributeSyncing = false;
|
||||
|
||||
// Only used in www builds.
|
||||
export function addUserTimingListener() {
|
||||
|
||||
@@ -19,6 +19,7 @@ export const {
|
||||
enableSuspenseServerRenderer,
|
||||
replayFailedUnitOfWorkWithInvokeGuardedCallback,
|
||||
warnAboutDeprecatedLifecycles,
|
||||
disableInputAttributeSyncing,
|
||||
} = require('ReactFeatureFlags');
|
||||
|
||||
// The rest of the flags are static for better dead code elimination.
|
||||
|
||||
@@ -2,6 +2,6 @@
|
||||
|
||||
jest.mock('shared/ReactFeatureFlags', () => {
|
||||
const ReactFeatureFlags = require.requireActual('shared/ReactFeatureFlags');
|
||||
// TODO: Set feature flags for Fire
|
||||
ReactFeatureFlags.disableInputAttributeSyncing = true;
|
||||
return ReactFeatureFlags;
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user