Skip to content

Commit

Permalink
Delete value attribute when defaultValue sets back to null/missing
Browse files Browse the repository at this point in the history
For some types, this already resets the value, so if we also have a value
we want to set that after. So this moves to updating defaultValue before
value.
  • Loading branch information
sebmarkbage committed Apr 9, 2023
1 parent 18a5a9f commit 2019ddc
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 38 deletions.
50 changes: 27 additions & 23 deletions packages/react-dom-bindings/src/client/ReactDOMInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,38 +92,17 @@ export function updateInputChecked(element: Element, props: Object) {
export function updateInput(element: Element, props: Object) {
const node: HTMLInputElement = (element: any);

updateInputChecked(element, props);

const value = getToStringValue(props.value);
const type = props.type;

if (value != null) {
if (type === 'number') {
if (
// $FlowFixMe[incompatible-type]
(value === 0 && node.value === '') ||
// We explicitly want to coerce to number here if possible.
// eslint-disable-next-line
node.value != (value: any)
) {
node.value = toString((value: any));
}
} else if (node.value !== toString((value: any))) {
node.value = toString((value: any));
}
} else if (type === 'submit' || type === 'reset') {
// Submit/reset inputs need the attribute removed completely to avoid
// blank-text buttons.
node.removeAttribute('value');
return;
}

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.defaultValue != null) {
setDefaultValue(node, props.type, getToStringValue(props.defaultValue));
} else {
node.removeAttribute('value');
}
} else {
// When syncing the value attribute, the value comes from a cascade of
Expand All @@ -135,6 +114,8 @@ export function updateInput(element: Element, props: Object) {
setDefaultValue(node, props.type, value);
} else if (props.defaultValue != null) {
setDefaultValue(node, props.type, getToStringValue(props.defaultValue));
} else {
node.removeAttribute('value');
}
}

Expand All @@ -154,6 +135,29 @@ export function updateInput(element: Element, props: Object) {
node.defaultChecked = !!props.defaultChecked;
}
}

updateInputChecked(element, props);

if (value != null) {
if (type === 'number') {
if (
// $FlowFixMe[incompatible-type]
(value === 0 && node.value === '') ||
// We explicitly want to coerce to number here if possible.
// eslint-disable-next-line
node.value != (value: any)
) {
node.value = toString((value: any));
}
} else if (node.value !== toString((value: any))) {
node.value = toString((value: any));
}
} else if (type === 'submit' || type === 'reset') {
// Submit/reset inputs need the attribute removed completely to avoid
// blank-text buttons.
node.removeAttribute('value');
return;
}
}

export function postInitInput(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1166,11 +1166,7 @@ describe('DOMPropertyOperations', () => {
).toErrorDev(
'A component is changing a controlled input to be uncontrolled',
);
if (disableInputAttributeSyncing) {
expect(container.firstChild.hasAttribute('value')).toBe(false);
} else {
expect(container.firstChild.getAttribute('value')).toBe('foo');
}
expect(container.firstChild.hasAttribute('value')).toBe(false);
expect(container.firstChild.value).toBe('foo');
});

Expand Down
38 changes: 28 additions & 10 deletions packages/react-dom/src/__tests__/ReactDOMInput-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1952,11 +1952,7 @@ describe('ReactDOMInput', () => {
expect(renderInputWithStringThenWithUndefined).toErrorDev(
'A component is changing a controlled input to be uncontrolled.',
);
if (disableInputAttributeSyncing) {
expect(input.getAttribute('value')).toBe(null);
} else {
expect(input.getAttribute('value')).toBe('latest');
}
expect(input.getAttribute('value')).toBe(null);
});

it('preserves the value property', () => {
Expand Down Expand Up @@ -2002,11 +1998,7 @@ describe('ReactDOMInput', () => {
'or `undefined` for uncontrolled components.',
'A component is changing a controlled input to be uncontrolled.',
]);
if (disableInputAttributeSyncing) {
expect(input.hasAttribute('value')).toBe(false);
} else {
expect(input.getAttribute('value')).toBe('latest');
}
expect(input.hasAttribute('value')).toBe(false);
});

it('preserves the value property', () => {
Expand Down Expand Up @@ -2165,4 +2157,30 @@ describe('ReactDOMInput', () => {
expect(node.hasAttribute('value')).toBe(false);
});
});

it('should remove previous `defaultValue`', () => {
const node = ReactDOM.render(
<input type="text" defaultValue="0" />,
container,
);

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

ReactDOM.render(<input type="text" />, container);
expect(node.defaultValue).toBe('');
});

it('should treat `defaultValue={null}` as missing', () => {
const node = ReactDOM.render(
<input type="text" defaultValue="0" />,
container,
);

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

ReactDOM.render(<input type="text" defaultValue={null} />, container);
expect(node.defaultValue).toBe('');
});
});

0 comments on commit 2019ddc

Please sign in to comment.