Skip to content

Commit

Permalink
Make uncontrolled -> controlled warning clearer (#17070)
Browse files Browse the repository at this point in the history 
* Make uncontrolled -> controlled warning clearer

* Update phrasing, mirror for opposite direction

* Remove unused substitution

* Update warning tests

* Literally got these backwards, womp womp

* Rerere-fix tests
  • Loading branch information
@vcarl
vcarl committed Apr 7, 2020
1 parent ddc4b65 commit 03de849
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 56 deletions.
2 changes: 1 addition & 1 deletion packages/react-dom/src/__tests__/DOMPropertyOperations-test.js
View file
Expand Up @@ -184,7 +184,7 @@ describe('DOMPropertyOperations', () => {
container,
),
).toErrorDev(
'A component is changing a controlled input of type text to be uncontrolled',
'A component is changing a controlled input to be uncontrolled',
);
if (disableInputAttributeSyncing) {
expect(container.firstChild.hasAttribute('value')).toBe(false);
Expand Down
6 changes: 3 additions & 3 deletions packages/react-dom/src/__tests__/ReactDOMComponentTree-test.js
View file
Expand Up @@ -178,9 +178,9 @@ describe('ReactDOMComponentTree', () => {
const component = <Controlled />;
const instance = ReactDOM.render(component, container);
expect(() => simulateInput(instance.a, finishValue)).toErrorDev(
'Warning: A component is changing an uncontrolled input of ' +
'type text to be controlled. Input elements should not ' +
'switch from uncontrolled to controlled (or vice versa). ' +
'Warning: 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. More info: ' +
'https://fb.me/react-controlled-components',
Expand Down
101 changes: 55 additions & 46 deletions packages/react-dom/src/__tests__/ReactDOMInput-test.js
View file
Expand Up @@ -479,8 +479,7 @@ describe('ReactDOMInput', () => {
expect(() =>
ReactDOM.render(<input type="text" defaultValue="1" />, container),
).toErrorDev(
'A component is changing a controlled input of type ' +
'text to be uncontrolled.',
'A component is changing a controlled input to be uncontrolled.',
);
expect(node.value).toBe('0');
});
Expand Down Expand Up @@ -858,8 +857,7 @@ describe('ReactDOMInput', () => {
container,
),
).toErrorDev(
'A component is changing a controlled input of type ' +
'submit to be uncontrolled.',
'A component is changing a controlled input to be uncontrolled.',
);

const node = container.firstChild;
Expand All @@ -878,8 +876,7 @@ describe('ReactDOMInput', () => {
container,
),
).toErrorDev(
'A component is changing a controlled input of type ' +
'reset to be uncontrolled.',
'A component is changing a controlled input to be uncontrolled.',
);

const node = container.firstChild;
Expand Down Expand Up @@ -1272,8 +1269,9 @@ describe('ReactDOMInput', () => {
);
ReactDOM.render(stub, container);
expect(() => ReactDOM.render(<input type="text" />, container)).toErrorDev(
'Warning: A component is changing a controlled input of type text to be uncontrolled. ' +
'Input elements should not switch from controlled to uncontrolled (or vice versa). ' +
'Warning: 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. More info: https://fb.me/react-controlled-components\n' +
' in input (at **)',
Expand All @@ -1290,8 +1288,9 @@ describe('ReactDOMInput', () => {
).toErrorDev([
'`value` prop on `input` should not be null. ' +
'Consider using an empty string to clear the component or `undefined` for uncontrolled components',
'Warning: A component is changing a controlled input of type text to be uncontrolled. ' +
'Input elements should not switch from controlled to uncontrolled (or vice versa). ' +
'Warning: 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. More info: https://fb.me/react-controlled-components\n' +
' in input (at **)',
Expand All @@ -1309,8 +1308,9 @@ describe('ReactDOMInput', () => {
container,
),
).toErrorDev(
'Warning: A component is changing a controlled input of type text to be uncontrolled. ' +
'Input elements should not switch from controlled to uncontrolled (or vice versa). ' +
'Warning: 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. More info: https://fb.me/react-controlled-components\n' +
' in input (at **)',
Expand All @@ -1323,8 +1323,9 @@ describe('ReactDOMInput', () => {
expect(() =>
ReactDOM.render(<input type="text" value="controlled" />, container),
).toErrorDev(
'Warning: A component is changing an uncontrolled input of type text to be controlled. ' +
'Input elements should not switch from uncontrolled to controlled (or vice versa). ' +
'Warning: 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. More info: https://fb.me/react-controlled-components\n' +
' in input (at **)',
Expand All @@ -1340,8 +1341,9 @@ describe('ReactDOMInput', () => {
expect(() =>
ReactDOM.render(<input type="text" value="controlled" />, container),
).toErrorDev(
'Warning: A component is changing an uncontrolled input of type text to be controlled. ' +
'Input elements should not switch from uncontrolled to controlled (or vice versa). ' +
'Warning: 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. More info: https://fb.me/react-controlled-components\n' +
' in input (at **)',
Expand All @@ -1356,8 +1358,9 @@ describe('ReactDOMInput', () => {
expect(() =>
ReactDOM.render(<input type="checkbox" />, container),
).toErrorDev(
'Warning: A component is changing a controlled input of type checkbox to be uncontrolled. ' +
'Input elements should not switch from controlled to uncontrolled (or vice versa). ' +
'Warning: 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. More info: https://fb.me/react-controlled-components\n' +
' in input (at **)',
Expand All @@ -1372,8 +1375,9 @@ describe('ReactDOMInput', () => {
expect(() =>
ReactDOM.render(<input type="checkbox" checked={null} />, container),
).toErrorDev(
'Warning: A component is changing a controlled input of type checkbox to be uncontrolled. ' +
'Input elements should not switch from controlled to uncontrolled (or vice versa). ' +
'Warning: 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. More info: https://fb.me/react-controlled-components\n' +
' in input (at **)',
Expand All @@ -1391,8 +1395,9 @@ describe('ReactDOMInput', () => {
container,
),
).toErrorDev(
'Warning: A component is changing a controlled input of type checkbox to be uncontrolled. ' +
'Input elements should not switch from controlled to uncontrolled (or vice versa). ' +
'Warning: 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. More info: https://fb.me/react-controlled-components\n' +
' in input (at **)',
Expand All @@ -1405,8 +1410,9 @@ describe('ReactDOMInput', () => {
expect(() =>
ReactDOM.render(<input type="checkbox" checked={true} />, container),
).toErrorDev(
'Warning: A component is changing an uncontrolled input of type checkbox to be controlled. ' +
'Input elements should not switch from uncontrolled to controlled (or vice versa). ' +
'Warning: 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. More info: https://fb.me/react-controlled-components\n' +
' in input (at **)',
Expand All @@ -1419,8 +1425,9 @@ describe('ReactDOMInput', () => {
expect(() =>
ReactDOM.render(<input type="checkbox" checked={true} />, container),
).toErrorDev(
'Warning: A component is changing an uncontrolled input of type checkbox to be controlled. ' +
'Input elements should not switch from uncontrolled to controlled (or vice versa). ' +
'Warning: 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. More info: https://fb.me/react-controlled-components\n' +
' in input (at **)',
Expand All @@ -1431,8 +1438,9 @@ describe('ReactDOMInput', () => {
const stub = <input type="radio" checked={true} onChange={emptyFunction} />;
ReactDOM.render(stub, container);
expect(() => ReactDOM.render(<input type="radio" />, container)).toErrorDev(
'Warning: A component is changing a controlled input of type radio to be uncontrolled. ' +
'Input elements should not switch from controlled to uncontrolled (or vice versa). ' +
'Warning: 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. More info: https://fb.me/react-controlled-components\n' +
' in input (at **)',
Expand All @@ -1445,8 +1453,9 @@ describe('ReactDOMInput', () => {
expect(() =>
ReactDOM.render(<input type="radio" checked={null} />, container),
).toErrorDev(
'Warning: A component is changing a controlled input of type radio to be uncontrolled. ' +
'Input elements should not switch from controlled to uncontrolled (or vice versa). ' +
'Warning: 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. More info: https://fb.me/react-controlled-components\n' +
' in input (at **)',
Expand All @@ -1459,8 +1468,9 @@ describe('ReactDOMInput', () => {
expect(() =>
ReactDOM.render(<input type="radio" defaultChecked={true} />, container),
).toErrorDev(
'Warning: A component is changing a controlled input of type radio to be uncontrolled. ' +
'Input elements should not switch from controlled to uncontrolled (or vice versa). ' +
'Warning: 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. More info: https://fb.me/react-controlled-components\n' +
' in input (at **)',
Expand All @@ -1473,8 +1483,9 @@ describe('ReactDOMInput', () => {
expect(() =>
ReactDOM.render(<input type="radio" checked={true} />, container),
).toErrorDev(
'Warning: A component is changing an uncontrolled input of type radio to be controlled. ' +
'Input elements should not switch from uncontrolled to controlled (or vice versa). ' +
'Warning: 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. More info: https://fb.me/react-controlled-components\n' +
' in input (at **)',
Expand All @@ -1487,8 +1498,9 @@ describe('ReactDOMInput', () => {
expect(() =>
ReactDOM.render(<input type="radio" checked={true} />, container),
).toErrorDev(
'Warning: A component is changing an uncontrolled input of type radio to be controlled. ' +
'Input elements should not switch from uncontrolled to controlled (or vice versa). ' +
'Warning: 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. More info: https://fb.me/react-controlled-components\n' +
' in input (at **)',
Expand Down Expand Up @@ -1539,8 +1551,9 @@ describe('ReactDOMInput', () => {
expect(() =>
ReactDOM.render(<input type="radio" value="value" />, container),
).toErrorDev(
'Warning: A component is changing a controlled input of type radio to be uncontrolled. ' +
'Input elements should not switch from controlled to uncontrolled (or vice versa). ' +
'Warning: 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. More info: https://fb.me/react-controlled-components\n' +
' in input (at **)',
Expand Down Expand Up @@ -1837,8 +1850,7 @@ describe('ReactDOMInput', () => {

it('reverts the value attribute to the initial value', () => {
expect(renderInputWithStringThenWithUndefined).toErrorDev(
'Input elements should not switch from controlled to ' +
'uncontrolled (or vice versa).',
'A component is changing a controlled input to be uncontrolled.',
);
if (disableInputAttributeSyncing) {
expect(input.getAttribute('value')).toBe(null);
Expand All @@ -1849,8 +1861,7 @@ describe('ReactDOMInput', () => {

it('preserves the value property', () => {
expect(renderInputWithStringThenWithUndefined).toErrorDev(
'Input elements should not switch from controlled to ' +
'uncontrolled (or vice versa).',
'A component is changing a controlled input to be uncontrolled.',
);
expect(input.value).toBe('latest');
});
Expand Down Expand Up @@ -1889,8 +1900,7 @@ describe('ReactDOMInput', () => {
'`value` prop on `input` should not be null. ' +
'Consider using an empty string to clear the component ' +
'or `undefined` for uncontrolled components.',
'Input elements should not switch from controlled ' +
'to uncontrolled (or vice versa).',
'A component is changing a controlled input to be uncontrolled.',
]);
if (disableInputAttributeSyncing) {
expect(input.hasAttribute('value')).toBe(false);
Expand All @@ -1904,8 +1914,7 @@ describe('ReactDOMInput', () => {
'`value` prop on `input` should not be null. ' +
'Consider using an empty string to clear the component ' +
'or `undefined` for uncontrolled components.',
'Input elements should not switch from controlled ' +
'to uncontrolled (or vice versa).',
'A component is changing a controlled input to be uncontrolled.',
]);
expect(input.value).toBe('latest');
});
Expand Down
12 changes: 6 additions & 6 deletions packages/react-dom/src/client/ReactDOMInput.js
View file
Expand Up @@ -143,11 +143,11 @@ export function updateWrapper(element: Element, props: Object) {
!didWarnUncontrolledToControlled
) {
console.error(
'A component is changing an uncontrolled input of type %s to be controlled. ' +
'Input elements should not switch from uncontrolled to controlled (or vice versa). ' +
'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. More info: https://fb.me/react-controlled-components',
props.type,
);
didWarnUncontrolledToControlled = true;
}
Expand All @@ -157,11 +157,11 @@ export function updateWrapper(element: Element, props: Object) {
!didWarnControlledToUncontrolled
) {
console.error(
'A component is changing a controlled input of type %s to be uncontrolled. ' +
'Input elements should not switch from controlled to uncontrolled (or vice versa). ' +
'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. More info: https://fb.me/react-controlled-components',
props.type,
);
didWarnControlledToUncontrolled = true;
}
Expand Down

0 comments on commit 03de849

Please sign in to comment.