Skip to content

Commit c0c39a6

Browse files
authored
[Fiber] Update input.defaultValue for type=number too (react#36980)
We used to avoid updating .defaultValue until blur on number inputs because of a Chrome bug, but the relevant Chromium behavior was fixed in 2020 in M85 https://issues.chromium.org/issues/40124871 and likewise in WebKit https://bugs.webkit.org/show_bug.cgi?id=217156 so I think we can probably just remove this special-cased logic. We do still need some special logic for number inputs because it's the only type where we let users specify a non-string value that has multiple string representations and need to be careful not to clobber the .value if it has a slightly different string than what you'd get from casting a number to string. Fixes react#29862. Tested with the number inputs fixture in current Chrome, Safari, and Firefox.
1 parent 5123b06 commit c0c39a6

3 files changed

Lines changed: 25 additions & 56 deletions

File tree

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

Lines changed: 23 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import {getCurrentFiberOwnerNameInDevOrNull} from 'react-reconciler/src/ReactCur
1313
import {getFiberCurrentPropsFromNode} from './ReactDOMComponentTree';
1414
import {getToStringValue, toString} from './ToStringValue';
1515
import {track, trackHydrated, updateValueIfChanged} from './inputValueTracking';
16-
import getActiveElement from './getActiveElement';
1716
import {
1817
disableInputAttributeSyncing,
1918
enableHydrationChangeEvent,
@@ -121,10 +120,13 @@ export function updateInput(
121120
if (value != null) {
122121
if (type === 'number') {
123122
if (
123+
// "" == 0, so a cleared field wouldn't otherwise be restored to 0.
124124
// $FlowFixMe[incompatible-type]
125125
// $FlowFixMe[invalid-compare]
126126
(value === 0 && node.value === '') ||
127-
// We explicitly want to coerce to number here if possible.
127+
// We explicitly want to coerce to number here if possible, so that
128+
// other spellings of the same number (e.g. "0.0" mid-edit) aren't
129+
// clobbered while the user types.
128130
// eslint-disable-next-line
129131
node.value != (value as any)
130132
) {
@@ -144,7 +146,7 @@ export function updateInput(
144146
// whenever the defaultValue React prop has changed. When not present,
145147
// React does nothing
146148
if (defaultValue != null) {
147-
setDefaultValue(node, type, getToStringValue(defaultValue));
149+
setDefaultValue(node, getToStringValue(defaultValue));
148150
} else if (lastDefaultValue != null) {
149151
node.removeAttribute('value');
150152
}
@@ -155,9 +157,22 @@ export function updateInput(
155157
// 2. The defaultValue React property
156158
// 3. Otherwise there should be no change
157159
if (value != null) {
158-
setDefaultValue(node, type, getToStringValue(value));
160+
if (
161+
type === 'number' &&
162+
// We explicitly want to coerce to number here if possible.
163+
// eslint-disable-next-line
164+
node.value == (value as any)
165+
) {
166+
// node.value may be a different spelling of the same number (e.g.
167+
// "0.0" for 0). Mirror what's displayed, like the value setter does.
168+
// Not redundant with the assignment above: browsers sanitize invalid
169+
// assigned values to "", in which case we sync the React value below.
170+
setDefaultValue(node, getToStringValue(node.value));
171+
} else {
172+
setDefaultValue(node, getToStringValue(value));
173+
}
159174
} else if (defaultValue != null) {
160-
setDefaultValue(node, type, getToStringValue(defaultValue));
175+
setDefaultValue(node, getToStringValue(defaultValue));
161176
} else if (lastDefaultValue != null) {
162177
node.removeAttribute('value');
163178
}
@@ -462,26 +477,8 @@ export function restoreControlledInputState(element: Element, props: Object) {
462477
}
463478
}
464479

465-
// In Chrome, assigning defaultValue to certain input types triggers input validation.
466-
// For number inputs, the display value loses trailing decimal points. For email inputs,
467-
// Chrome raises "The specified value <x> is not a valid email address".
468-
//
469-
// Here we check to see if the defaultValue has actually changed, avoiding these problems
470-
// when the user is inputting text
471-
//
472-
// https://github.com/facebook/react/issues/7253
473-
export function setDefaultValue(
474-
node: HTMLInputElement,
475-
type: ?string,
476-
value: ToStringValue,
477-
) {
478-
if (
479-
// Focused number inputs synchronize on blur. See ChangeEventPlugin.js
480-
type !== 'number' ||
481-
getActiveElement(node.ownerDocument) !== node
482-
) {
483-
if (node.defaultValue !== toString(value)) {
484-
node.defaultValue = toString(value);
485-
}
480+
function setDefaultValue(node: HTMLInputElement, value: ToStringValue) {
481+
if (node.defaultValue !== toString(value)) {
482+
node.defaultValue = toString(value);
486483
}
487484
}

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

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,8 @@ import getEventTarget from '../getEventTarget';
2323
import isEventSupported from '../isEventSupported';
2424
import {getNodeFromInstance} from '../../client/ReactDOMComponentTree';
2525
import {updateValueIfChanged} from '../../client/inputValueTracking';
26-
import {setDefaultValue} from '../../client/ReactDOMInput';
2726
import {enqueueStateRestore} from '../ReactDOMControlledComponent';
2827

29-
import {disableInputAttributeSyncing} from 'shared/ReactFeatureFlags';
3028
import {batchedUpdates} from '../ReactDOMUpdateBatching';
3129
import {
3230
processDispatchQueue,
@@ -260,20 +258,6 @@ function getTargetInstForInputOrChangeEvent(
260258
}
261259
}
262260

263-
function handleControlledInputBlur(node: HTMLInputElement, props: any) {
264-
if (node.type !== 'number') {
265-
return;
266-
}
267-
268-
if (!disableInputAttributeSyncing) {
269-
const isControlled = props.value != null;
270-
if (isControlled) {
271-
// If controlled, assign the value attribute to the current value on blur
272-
setDefaultValue(node as any, 'number', (node as any).value);
273-
}
274-
}
275-
}
276-
277261
/**
278262
* This plugin creates an `onChange` event that normalizes change events
279263
* across form elements. This event fires at a time when it's possible to
@@ -330,15 +314,6 @@ function extractEvents(
330314
if (handleEventFunc) {
331315
handleEventFunc(domEventName, targetNode, targetInst);
332316
}
333-
334-
// When blurring, set the value attribute for number inputs
335-
if (domEventName === 'focusout' && targetInst) {
336-
// These props aren't necessarily the most current but we warn for changing
337-
// between controlled and uncontrolled, so it doesn't matter and the previous
338-
// code was also broken for changes.
339-
const props = targetInst.memoizedProps;
340-
handleControlledInputBlur(targetNode as any as HTMLInputElement, props);
341-
}
342317
}
343318

344319
export {registerEvents, extractEvents};

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,9 +1035,6 @@ describe('ReactDOMInput', () => {
10351035
expect(node.value).toBe('0.0');
10361036
expect(node.hasAttribute('value')).toBe(false);
10371037
} else {
1038-
dispatchEventOnNode(node, 'blur');
1039-
dispatchEventOnNode(node, 'focusout');
1040-
10411038
expect(node.value).toBe('0.0');
10421039
expect(node.getAttribute('value')).toBe('0.0');
10431040
}
@@ -2664,7 +2661,7 @@ describe('ReactDOMInput', () => {
26642661
}
26652662
});
26662663

2667-
it('does not set the value attribute on number inputs if focused', async () => {
2664+
it('sets the value attribute on number inputs even when focused', async () => {
26682665
const Input = getTestInput();
26692666
await act(() => {
26702667
root.render(<Input type="number" value="1" />);
@@ -2681,7 +2678,7 @@ describe('ReactDOMInput', () => {
26812678
if (disableInputAttributeSyncing) {
26822679
expect(node.hasAttribute('value')).toBe(false);
26832680
} else {
2684-
expect(node.getAttribute('value')).toBe('1');
2681+
expect(node.getAttribute('value')).toBe('2');
26852682
}
26862683
});
26872684

0 commit comments

Comments
 (0)