From 8de05f8adf0e81fa329228b697aa53e47d957885 Mon Sep 17 00:00:00 2001 From: raunofreiberg Date: Fri, 10 Aug 2018 11:18:55 +0300 Subject: [PATCH 1/5] tests: remove TODOs about warnings in ReactDOMInput-test.js --- .../src/__tests__/ReactDOMInput-test.js | 26 ++++++++++++------- .../src/shared/ReactDOMUnknownPropertyHook.js | 10 +++++++ 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMInput-test.js b/packages/react-dom/src/__tests__/ReactDOMInput-test.js index 5afa9dcf7aa..5367e867cc3 100644 --- a/packages/react-dom/src/__tests__/ReactDOMInput-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMInput-test.js @@ -1640,23 +1640,29 @@ describe('ReactDOMInput', () => { expect(node.getAttribute('value')).toBe(''); }); - it('treats initial Symbol defaultValue as an empty string', function() { - ReactDOM.render(, container); + fit('treats initial Symbol defaultValue as an empty string', function() { + expect(() => + ReactDOM.render( + , + container + ), + ).toWarnDev('Invalid value for prop `defaultValue`'); + const node = container.firstChild; expect(node.value).toBe(''); expect(node.getAttribute('value')).toBe(''); - // TODO: we should warn here. }); it('treats updated Symbol defaultValue as an empty string', function() { ReactDOM.render(, container); - ReactDOM.render(, container); + expect(() => + ReactDOM.render(, container), + ).toWarnDev('Invalid value for prop `defaultValue`'); const node = container.firstChild; expect(node.value).toBe('foo'); expect(node.getAttribute('value')).toBe(''); - // TODO: we should warn here. }); }); @@ -1689,22 +1695,24 @@ describe('ReactDOMInput', () => { }); it('treats initial function defaultValue as an empty string', function() { - ReactDOM.render( {}} />, container); + expect(() => + ReactDOM.render( {}} />, container), + ).toWarnDev('Invalid value for prop `defaultValue`'); const node = container.firstChild; expect(node.value).toBe(''); expect(node.getAttribute('value')).toBe(''); - // TODO: we should warn here. }); it('treats updated function defaultValue as an empty string', function() { ReactDOM.render(, container); - ReactDOM.render( {}} />, container); + expect(() => + ReactDOM.render( {}} />, container), + ).toWarnDev('Invalid value for prop `defaultValue`'); const node = container.firstChild; expect(node.value).toBe('foo'); expect(node.getAttribute('value')).toBe(''); - // TODO: we should warn here. }); }); diff --git a/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js b/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js index 9e7fda4e6cd..125d62887e4 100644 --- a/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js +++ b/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js @@ -214,6 +214,16 @@ if (__DEV__) { return true; } + // Ignore the reservation of the `defaultValue` prop for warnings + if ( + typeof value === 'symbol' || typeof value === 'function' && + tagName === 'input' && + name === 'defaultValue' + ) { + warnedProperties[name] = true; + return false; + } + // Now that we've validated casing, do not validate // data types for reserved props if (isReserved) { From e1aeb5cc9318b5f672e7a234c721e7102c45e35f Mon Sep 17 00:00:00 2001 From: raunofreiberg Date: Fri, 10 Aug 2018 11:20:40 +0300 Subject: [PATCH 2/5] fix: remove fit --- packages/react-dom/src/__tests__/ReactDOMInput-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMInput-test.js b/packages/react-dom/src/__tests__/ReactDOMInput-test.js index 5367e867cc3..9b70967d28d 100644 --- a/packages/react-dom/src/__tests__/ReactDOMInput-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMInput-test.js @@ -1640,7 +1640,7 @@ describe('ReactDOMInput', () => { expect(node.getAttribute('value')).toBe(''); }); - fit('treats initial Symbol defaultValue as an empty string', function() { + it('treats initial Symbol defaultValue as an empty string', function() { expect(() => ReactDOM.render( , From 4d22d812c8eb47e029c6117c691b8b8e3fcd01dc Mon Sep 17 00:00:00 2001 From: raunofreiberg Date: Fri, 10 Aug 2018 11:23:31 +0300 Subject: [PATCH 3/5] Run prettier --- packages/react-dom/src/__tests__/ReactDOMInput-test.js | 5 +---- .../react-dom/src/shared/ReactDOMUnknownPropertyHook.js | 7 ++++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMInput-test.js b/packages/react-dom/src/__tests__/ReactDOMInput-test.js index 9b70967d28d..5f558895295 100644 --- a/packages/react-dom/src/__tests__/ReactDOMInput-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMInput-test.js @@ -1642,10 +1642,7 @@ describe('ReactDOMInput', () => { it('treats initial Symbol defaultValue as an empty string', function() { expect(() => - ReactDOM.render( - , - container - ), + ReactDOM.render(, container), ).toWarnDev('Invalid value for prop `defaultValue`'); const node = container.firstChild; diff --git a/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js b/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js index 125d62887e4..97391bfe663 100644 --- a/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js +++ b/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js @@ -216,9 +216,10 @@ if (__DEV__) { // Ignore the reservation of the `defaultValue` prop for warnings if ( - typeof value === 'symbol' || typeof value === 'function' && - tagName === 'input' && - name === 'defaultValue' + typeof value === 'symbol' || + (typeof value === 'function' && + tagName === 'input' && + name === 'defaultValue') ) { warnedProperties[name] = true; return false; From 5120ce3780e694cea926b3e2e5f6d47ba00311b2 Mon Sep 17 00:00:00 2001 From: raunofreiberg Date: Fri, 10 Aug 2018 11:58:01 +0300 Subject: [PATCH 4/5] refactor: Enable warnings for the defaultValue prop in shouldRemoveAttributeWithWarning --- .../src/__tests__/ReactDOMInput-test.js | 10 ++++++++-- .../src/__tests__/ReactDOMTextarea-test.js | 5 ++++- packages/react-dom/src/shared/DOMProperty.js | 8 +++++++- .../src/shared/ReactDOMUnknownPropertyHook.js | 17 ----------------- 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMInput-test.js b/packages/react-dom/src/__tests__/ReactDOMInput-test.js index 5f558895295..3710fa40081 100644 --- a/packages/react-dom/src/__tests__/ReactDOMInput-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMInput-test.js @@ -400,14 +400,20 @@ describe('ReactDOMInput', () => { it('should display "true" for `defaultValue` of `true`', () => { let stub = ; - const node = ReactDOM.render(stub, container); + let node; + expect(() => (node = ReactDOM.render(stub, container))).toWarnDev( + 'Received `true` for a non-boolean attribute `defaultValue`', + ); expect(node.value).toBe('true'); }); it('should display "false" for `defaultValue` of `false`', () => { let stub = ; - const node = ReactDOM.render(stub, container); + let node; + expect(() => (node = ReactDOM.render(stub, container))).toWarnDev( + 'Received `false` for a non-boolean attribute `defaultValue`', + ); expect(node.value).toBe('false'); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMTextarea-test.js b/packages/react-dom/src/__tests__/ReactDOMTextarea-test.js index fa5d717974a..0c40c1ff36f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMTextarea-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMTextarea-test.js @@ -63,7 +63,10 @@ describe('ReactDOMTextarea', () => { it('should display "false" for `defaultValue` of `false`', () => { const stub =