Skip to content

Commit

Permalink
Revert "Don't update textarea defaultValue and input checked unnecess…
Browse files Browse the repository at this point in the history
…arily (#26580)"

This reverts commit 9a9da77.
  • Loading branch information
kassens committed Apr 20, 2023
1 parent 87fb8ea commit ad3318e
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 78 deletions.
2 changes: 1 addition & 1 deletion packages/react-dom-bindings/src/client/ReactDOMInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export function validateInputProps(element: Element, props: Object) {
export function updateInputChecked(element: Element, props: Object) {
const node: HTMLInputElement = (element: any);
const checked = props.checked;
if (checked != null && node.checked !== !!checked) {
if (checked != null) {
node.checked = checked;
}
}
Expand Down
19 changes: 8 additions & 11 deletions packages/react-dom-bindings/src/client/ReactDOMTextarea.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ export function validateTextareaProps(element: Element, props: Object) {
export function updateTextarea(element: Element, props: Object) {
const node: HTMLTextAreaElement = (element: any);
const value = getToStringValue(props.value);
const defaultValue = getToStringValue(props.defaultValue);
if (defaultValue != null) {
node.defaultValue = toString(defaultValue);
} else {
node.defaultValue = '';
}
if (value != null) {
// Cast `value` to a string to ensure the value is set correctly. While
// browsers typically do this as necessary, jsdom doesn't.
Expand All @@ -70,19 +76,10 @@ export function updateTextarea(element: Element, props: Object) {
node.value = newValue;
}
// TOOO: This should respect disableInputAttributeSyncing flag.
if (props.defaultValue == null) {
if (node.defaultValue !== newValue) {
node.defaultValue = newValue;
}
return;
if (props.defaultValue == null && node.defaultValue !== newValue) {
node.defaultValue = newValue;
}
}
const defaultValue = getToStringValue(props.defaultValue);
if (defaultValue != null) {
node.defaultValue = toString(defaultValue);
} else {
node.defaultValue = '';
}
}

export function initTextarea(element: Element, props: Object) {
Expand Down
70 changes: 5 additions & 65 deletions packages/react-dom/src/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1043,63 +1043,6 @@ describe('ReactDOMComponent', () => {
expect(nodeValueSetter).toHaveBeenCalledTimes(2);
});

it('should not incur unnecessary DOM mutations for controlled string properties', () => {
function onChange() {}
const container = document.createElement('div');
ReactDOM.render(<input value="" onChange={onChange} />, container);

const node = container.firstChild;

let nodeValue = '';
const nodeValueSetter = jest.fn();
Object.defineProperty(node, 'value', {
get: function () {
return nodeValue;
},
set: nodeValueSetter.mockImplementation(function (newValue) {
nodeValue = newValue;
}),
});

ReactDOM.render(<input value="foo" onChange={onChange} />, container);
expect(nodeValueSetter).toHaveBeenCalledTimes(1);

ReactDOM.render(
<input value="foo" data-unrelated={true} onChange={onChange} />,
container,
);
expect(nodeValueSetter).toHaveBeenCalledTimes(1);

expect(() => {
ReactDOM.render(<input onChange={onChange} />, container);
}).toErrorDev(
'A component is changing a controlled input to be uncontrolled. This is likely caused by ' +
'the value changing from a defined to undefined, which should not happen. Decide between ' +
'using a controlled or uncontrolled input element for the lifetime of the component.',
);
expect(nodeValueSetter).toHaveBeenCalledTimes(1);

expect(() => {
ReactDOM.render(<input value={null} onChange={onChange} />, container);
}).toErrorDev(
'value` prop on `input` should not be null. Consider using an empty string to clear the ' +
'component or `undefined` for uncontrolled components.',
);
expect(nodeValueSetter).toHaveBeenCalledTimes(1);

expect(() => {
ReactDOM.render(<input value="" onChange={onChange} />, container);
}).toErrorDev(
' A component is changing an uncontrolled input to be controlled. This is likely caused by ' +
'the value changing from undefined to a defined value, which should not happen. Decide between ' +
'using a controlled or uncontrolled input element for the lifetime of the component.',
);
expect(nodeValueSetter).toHaveBeenCalledTimes(2);

ReactDOM.render(<input onChange={onChange} />, container);
expect(nodeValueSetter).toHaveBeenCalledTimes(2);
});

it('should not incur unnecessary DOM mutations for boolean properties', () => {
const container = document.createElement('div');
function onChange() {
Expand All @@ -1123,12 +1066,7 @@ describe('ReactDOMComponent', () => {
});

ReactDOM.render(
<input
type="checkbox"
onChange={onChange}
checked={true}
data-unrelated={true}
/>,
<input type="checkbox" onChange={onChange} checked={true} />,
container,
);
expect(nodeValueSetter).toHaveBeenCalledTimes(0);
Expand Down Expand Up @@ -1156,13 +1094,15 @@ describe('ReactDOMComponent', () => {
'using a controlled or uncontrolled input element for the lifetime of the component.',
);

expect(nodeValueSetter).toHaveBeenCalledTimes(2);
// TODO: Non-null values are updated twice on inputs. This is should ideally be fixed.
expect(nodeValueSetter).toHaveBeenCalledTimes(3);

ReactDOM.render(
<input type="checkbox" onChange={onChange} checked={true} />,
container,
);
expect(nodeValueSetter).toHaveBeenCalledTimes(3);
// TODO: Non-null values are updated twice on inputs. This is should ideally be fixed.
expect(nodeValueSetter).toHaveBeenCalledTimes(5);
});

it('should ignore attribute list for elements with the "is" attribute', () => {
Expand Down
1 change: 0 additions & 1 deletion packages/react-dom/src/__tests__/ReactDOMTextarea-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,6 @@ describe('ReactDOMTextarea', () => {
ref={n => (node = n)}
value="foo"
onChange={emptyFunction}
data-count={this.state.count}
/>
</div>
);
Expand Down

0 comments on commit ad3318e

Please sign in to comment.