From bb8610f35e76765c64fe53e5bb8402ee321f480b Mon Sep 17 00:00:00 2001 From: Biki-das Date: Wed, 22 Nov 2023 21:47:50 +0530 Subject: [PATCH 1/4] fix: select console error to not suggest to set readonly to true --- .../shared/ReactControlledValuePropTypes.js | 4 +- .../src/__tests__/ReactDOMSelect-test.js | 110 ++++++++++++++++++ 2 files changed, 112 insertions(+), 2 deletions(-) diff --git a/packages/react-dom-bindings/src/shared/ReactControlledValuePropTypes.js b/packages/react-dom-bindings/src/shared/ReactControlledValuePropTypes.js index ac1b16010647b..4627c3b2d19e1 100644 --- a/packages/react-dom-bindings/src/shared/ReactControlledValuePropTypes.js +++ b/packages/react-dom-bindings/src/shared/ReactControlledValuePropTypes.js @@ -35,8 +35,8 @@ export function checkControlledValueProps( console.error( 'You provided a `value` prop to a form field without an ' + '`onChange` handler. This will render a read-only field. If ' + - 'the field should be mutable use `defaultValue`. Otherwise, ' + - 'set either `onChange` or `readOnly`.', + 'the field should be mutable use `defaultValue`. Otherwise, set %s.', + tagName === 'select' ? '`onChange`' : 'either `onChange` or `readOnly`', ); } diff --git a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js index 4332c52d124ec..57c5e7108d24d 100644 --- a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js @@ -1289,5 +1289,115 @@ describe('ReactDOMSelect', () => { ' This value must be coerced to a string before using it here.', ); }); + + it('should not warn about missing onChange if value is not set', () => { + ReactTestUtils.renderIntoDocument( + , + ); + }); + + it('should not warn about missing onChange if value is undefined', () => { + ReactTestUtils.renderIntoDocument( + , + ); + }); + + it('should not warn about missing onChange if onChange is set', () => { + const change = jest.fn(); + + ReactTestUtils.renderIntoDocument( + , + ); + }); + + it('should not warn about missing onChange if disabled is true', () => { + ReactTestUtils.renderIntoDocument( + , + ); + }); + + it('should warn about missing onChange if value is false', () => { + expect(() => + ReactTestUtils.renderIntoDocument( + , + ), + ).toErrorDev( + 'Warning: You provided a `value` prop to a form ' + + 'field without an `onChange` handler. This will render a read-only ' + + 'field. If the field should be mutable use `defaultValue`. ' + + 'Otherwise, set `onChange`.', + ); + }); + + it('should warn about missing onChange if value is 0', () => { + expect(() => + ReactTestUtils.renderIntoDocument( + , + ), + ).toErrorDev( + 'Warning: You provided a `value` prop to a form ' + + 'field without an `onChange` handler. This will render a read-only ' + + 'field. If the field should be mutable use `defaultValue`. ' + + 'Otherwise, set `onChange`.', + ); + }); + + it('should warn about missing onChange if value is "0"', () => { + expect(() => + ReactTestUtils.renderIntoDocument( + , + ), + ).toErrorDev( + 'Warning: You provided a `value` prop to a form ' + + 'field without an `onChange` handler. This will render a read-only ' + + 'field. If the field should be mutable use `defaultValue`. ' + + 'Otherwise, set `onChange`.', + ); + }); + + it('should warn about missing onChange if value is ""', () => { + expect(() => + ReactTestUtils.renderIntoDocument( + , + ), + ).toErrorDev( + 'Warning: You provided a `value` prop to a form ' + + 'field without an `onChange` handler. This will render a read-only ' + + 'field. If the field should be mutable use `defaultValue`. ' + + 'Otherwise, set `onChange`.', + ); + }); }); }); From 458cdd49c52cada3fdf94d71b1e4b20ab07025cf Mon Sep 17 00:00:00 2001 From: Biki-das Date: Fri, 1 Dec 2023 11:52:11 +0530 Subject: [PATCH 2/4] added expect statements to assert for not throw() --- .../src/__tests__/ReactDOMSelect-test.js | 51 ++++++++++--------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js index 57c5e7108d24d..5aed2c4f84306 100644 --- a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js @@ -1300,36 +1300,41 @@ describe('ReactDOMSelect', () => { ); }); - it('should not warn about missing onChange if value is undefined', () => { - ReactTestUtils.renderIntoDocument( - , - ); + it('should not throw an error about missing onChange if value is undefined', () => { + expect(() => { + ReactTestUtils.renderIntoDocument( + , + ); + }).not.toThrow(); }); it('should not warn about missing onChange if onChange is set', () => { const change = jest.fn(); - - ReactTestUtils.renderIntoDocument( - , - ); + expect(() => { + ReactTestUtils.renderIntoDocument( + , + ); + }).not.toThrow(); }); it('should not warn about missing onChange if disabled is true', () => { - ReactTestUtils.renderIntoDocument( - , - ); + expect(() => { + ReactTestUtils.renderIntoDocument( + , + ); + }).not.toThrow(); }); it('should warn about missing onChange if value is false', () => { From 60f7f1041bf88e92b20579d0bd8fc8f4137cdde8 Mon Sep 17 00:00:00 2001 From: Biki-das Date: Fri, 1 Dec 2023 17:48:03 +0530 Subject: [PATCH 3/4] Added missing expect statement to value not set assertion --- .../src/__tests__/ReactDOMSelect-test.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js index 5aed2c4f84306..9df6b07e23f65 100644 --- a/packages/react-dom/src/__tests__/ReactDOMSelect-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMSelect-test.js @@ -1291,13 +1291,15 @@ describe('ReactDOMSelect', () => { }); it('should not warn about missing onChange if value is not set', () => { - ReactTestUtils.renderIntoDocument( - , - ); + expect(() => { + ReactTestUtils.renderIntoDocument( + , + ); + }).not.toThrow(); }); it('should not throw an error about missing onChange if value is undefined', () => { From 1a6b6b5d6d780a6c2ba6dbbf53a5873f6f239407 Mon Sep 17 00:00:00 2001 From: Biki-das Date: Fri, 1 Dec 2023 21:16:51 +0530 Subject: [PATCH 4/4] replaced ternary with if/else for the error condition --- .../shared/ReactControlledValuePropTypes.js | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/react-dom-bindings/src/shared/ReactControlledValuePropTypes.js b/packages/react-dom-bindings/src/shared/ReactControlledValuePropTypes.js index 4627c3b2d19e1..3d96eb1e8339a 100644 --- a/packages/react-dom-bindings/src/shared/ReactControlledValuePropTypes.js +++ b/packages/react-dom-bindings/src/shared/ReactControlledValuePropTypes.js @@ -32,12 +32,19 @@ export function checkControlledValueProps( props.value == null ) ) { - console.error( - 'You provided a `value` prop to a form field without an ' + - '`onChange` handler. This will render a read-only field. If ' + - 'the field should be mutable use `defaultValue`. Otherwise, set %s.', - tagName === 'select' ? '`onChange`' : 'either `onChange` or `readOnly`', - ); + if (tagName === 'select') { + console.error( + 'You provided a `value` prop to a form field without an ' + + '`onChange` handler. This will render a read-only field. If ' + + 'the field should be mutable use `defaultValue`. Otherwise, set `onChange`.', + ); + } else { + console.error( + 'You provided a `value` prop to a form field without an ' + + '`onChange` handler. This will render a read-only field. If ' + + 'the field should be mutable use `defaultValue`. Otherwise, set either `onChange` or `readOnly`.', + ); + } } if (