Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 24 commits into from
Sep 12, 2018
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
46e6bc8
:fire: Stop syncing the value attribute on inputs
nhunzaker Aug 31, 2018
7ce3884
Eliminate some additional checks
nhunzaker Sep 3, 2018
9097f42
Remove initialValue and initialWrapper from wrapperState flow type
nhunzaker Sep 3, 2018
c520ff9
Update tests with new sync logic, reduce some operations
nhunzaker Sep 4, 2018
ff926df
Update tests, add some caveats for SSR mismatches
nhunzaker Sep 4, 2018
d4eba96
Revert newline change
nhunzaker Sep 4, 2018
154c846
Remove unused type
nhunzaker Sep 4, 2018
fe220ce
Call toString to safely type string values
nhunzaker Sep 4, 2018
4221c4e
Add disableInputAttributeSyncing feature flag
nhunzaker Sep 12, 2018
c6e5890
Revert position of types in toStringValues
nhunzaker Sep 12, 2018
ad3befc
Invert flag on number input blur
nhunzaker Sep 12, 2018
69304e4
Add clarification why double blur is necessary
nhunzaker Sep 12, 2018
d79aca2
Update ReactFire number cases to be more explicite about blur
nhunzaker Sep 12, 2018
04f85c1
Move comments to reduce diff size
nhunzaker Sep 12, 2018
44d2204
Add comments to clarify behavior in each branch
nhunzaker Sep 12, 2018
19cc9e7
There is no need to assign a different checked behavior in Fire
nhunzaker Sep 12, 2018
2e4868a
Use checked reference
nhunzaker Sep 12, 2018
2727f1e
Format
nhunzaker Sep 12, 2018
31fa167
Avoid precomputing stringable values
nhunzaker Sep 12, 2018
1a37a43
Revert getToStringValue comment
nhunzaker Sep 12, 2018
30752e1
Revert placement of undefined in getToStringValue
nhunzaker Sep 12, 2018
7bc5f44
Do not eagerly stringify value
nhunzaker Sep 12, 2018
fa10f5f
Unify Fire test cases with normal ones
gaearon Sep 12, 2018
e4f68fe
Revert toString change. Only assign unsynced values when not nully
nhunzaker Sep 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions packages/react-dom/src/__tests__/DOMPropertyOperations-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@

'use strict';

// Set by `yarn test-fire`.
const {disableInputAttributeSyncing} = require('shared/ReactFeatureFlags');

describe('DOMPropertyOperations', () => {
let React;
let ReactDOM;
Expand Down Expand Up @@ -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('');
});

Expand Down Expand Up @@ -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(
Expand All @@ -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');
});

Expand Down
Loading