@samitouri / QOS-React / commits / 7b17f7bbf3

Enable warning for defaultProps on function components for everyone (#25699)

This also fixes a gap where were weren't warning on memo components.

Sebastian Markbåge committed Nov 17, 2022 at 12:22 UTC 7b17f7bbf3243c2890cb830902be8ef8b51db3da
17 files changed +209 -79
packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js
+5 -1
@@ -898,7 +898,11 @@ describe('ReactHooksInspectionIntegration', () => {
898
899 await LazyFoo;
900
901 - Scheduler.unstable_flushAll();
901 + expect(() => {
902 + Scheduler.unstable_flushAll();
903 + }).toErrorDev([
904 + 'Foo: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.',
905 + ]);
906
907 const childFiber = renderer.root._currentFiber();
908 const tree = ReactDebugTools.inspectHooksOfFiber(childFiber);
packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
+68 -38
@@ -291,6 +291,18 @@ describe('ReactDOMFizzServer', () => {
291 }
292
293 it('should asynchronously load a lazy component', async () => {
294 + const originalConsoleError = console.error;
295 + const mockError = jest.fn();
296 + console.error = (...args) => {
297 + if (args.length > 1) {
298 + if (typeof args[1] === 'object') {
299 + mockError(args[0].split('\n')[0]);
300 + return;
301 + }
302 + }
303 + mockError(...args.map(normalizeCodeLocInfo));
304 + };
305 +
306 let resolveA;
307 const LazyA = React.lazy(() => {
308 return new Promise(r => {
@@ -313,48 +325,66 @@ describe('ReactDOMFizzServer', () => {
325 punctuation: '!',
326 };
327
316 - await act(async () => {
317 - const {pipe} = renderToPipeableStream(
318 - <div>
319 - <div>
320 - <Suspense fallback={<Text text="Loading..." />}>
321 - <LazyA text="Hello" />
322 - </Suspense>
323 - </div>
328 + try {
329 + await act(async () => {
330 + const {pipe} = renderToPipeableStream(
331 <div>
325 - <Suspense fallback={<Text text="Loading..." />}>
326 - <LazyB text="world" />
327 - </Suspense>
328 - </div>
332 + <div>
333 + <Suspense fallback={<Text text="Loading..." />}>
334 + <LazyA text="Hello" />
335 + </Suspense>
336 + </div>
337 + <div>
338 + <Suspense fallback={<Text text="Loading..." />}>
339 + <LazyB text="world" />
340 + </Suspense>
341 + </div>
342 + </div>,
343 + );
344 + pipe(writable);
345 + });
346 +
347 + expect(getVisibleChildren(container)).toEqual(
348 + <div>
349 + <div>Loading...</div>
350 + <div>Loading...</div>
351 + </div>,
352 + );
353 + await act(async () => {
354 + resolveA({default: Text});
355 + });
356 + expect(getVisibleChildren(container)).toEqual(
357 + <div>
358 + <div>Hello</div>
359 + <div>Loading...</div>
360 + </div>,
361 + );
362 + await act(async () => {
363 + resolveB({default: TextWithPunctuation});
364 + });
365 + expect(getVisibleChildren(container)).toEqual(
366 + <div>
367 + <div>Hello</div>
368 + <div>world!</div>
369 </div>,
370 );
331 - pipe(writable);
332 - });
371
334 - expect(getVisibleChildren(container)).toEqual(
335 - <div>
336 - <div>Loading...</div>
337 - <div>Loading...</div>
338 - </div>,
339 - );
340 - await act(async () => {
341 - resolveA({default: Text});
342 - });
343 - expect(getVisibleChildren(container)).toEqual(
344 - <div>
345 - <div>Hello</div>
346 - <div>Loading...</div>
347 - </div>,
348 - );
349 - await act(async () => {
350 - resolveB({default: TextWithPunctuation});
351 - });
352 - expect(getVisibleChildren(container)).toEqual(
353 - <div>
354 - <div>Hello</div>
355 - <div>world!</div>
356 - </div>,
357 - );
372 + if (__DEV__) {
373 + expect(mockError).toHaveBeenCalledWith(
374 + 'Warning: %s: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.%s',
375 + 'TextWithPunctuation',
376 + '\n in TextWithPunctuation (at **)\n' +
377 + ' in Lazy (at **)\n' +
378 + ' in Suspense (at **)\n' +
379 + ' in div (at **)\n' +
380 + ' in div (at **)',
381 + );
382 + } else {
383 + expect(mockError).not.toHaveBeenCalled();
384 + }
385 + } finally {
386 + console.error = originalConsoleError;
387 + }
388 });
389
390 it('#23331: does not warn about hydration mismatches if something suspended in an earlier sibling', async () => {
packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.js renamed
+27 -13
@@ -10,7 +10,6 @@
10 'use strict';
11
12 let React;
13 -let ReactFeatureFlags;
13 let ReactNoop;
14 let Scheduler;
15 let JSXDEVRuntime;
@@ -19,19 +18,11 @@ describe('ReactDeprecationWarnings', () => {
18 beforeEach(() => {
19 jest.resetModules();
20 React = require('react');
22 - ReactFeatureFlags = require('shared/ReactFeatureFlags');
21 ReactNoop = require('react-noop-renderer');
22 Scheduler = require('scheduler');
23 if (__DEV__) {
24 JSXDEVRuntime = require('react/jsx-dev-runtime');
25 }
28 - ReactFeatureFlags.warnAboutDefaultPropsOnFunctionComponents = true;
29 - ReactFeatureFlags.warnAboutStringRefs = true;
30 - });
31 -
32 - afterEach(() => {
33 - ReactFeatureFlags.warnAboutDefaultPropsOnFunctionComponents = false;
34 - ReactFeatureFlags.warnAboutStringRefs = false;
26 });
27
28 it('should warn when given defaultProps', () => {
@@ -51,6 +42,27 @@ describe('ReactDeprecationWarnings', () => {
42 );
43 });
44
45 + it('should warn when given defaultProps on a memoized function', () => {
46 + const MemoComponent = React.memo(function FunctionalComponent(props) {
47 + return null;
48 + });
49 +
50 + MemoComponent.defaultProps = {
51 + testProp: true,
52 + };
53 +
54 + ReactNoop.render(
55 + <div>
56 + <MemoComponent />
57 + </div>,
58 + );
59 + expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev(
60 + 'Warning: FunctionalComponent: Support for defaultProps ' +
61 + 'will be removed from memo components in a future major ' +
62 + 'release. Use JavaScript default parameters instead.',
63 + );
64 + });
65 +
66 it('should warn when given string refs', () => {
67 class RefComponent extends React.Component {
68 render() {
@@ -74,9 +86,7 @@ describe('ReactDeprecationWarnings', () => {
86 );
87 });
88
77 - it('should not warn when owner and self are the same for string refs', () => {
78 - ReactFeatureFlags.warnAboutStringRefs = false;
79 -
89 + it('should warn when owner and self are the same for string refs', () => {
90 class RefComponent extends React.Component {
91 render() {
92 return null;
@@ -87,7 +97,11 @@ describe('ReactDeprecationWarnings', () => {
97 return <RefComponent ref="refComponent" __self={this} />;
98 }
99 }
90 - ReactNoop.renderLegacySyncRoot(<Component />);
100 + expect(() => {
101 + ReactNoop.renderLegacySyncRoot(<Component />);
102 + }).toErrorDev([
103 + 'Component "Component" contains the string ref "refComponent". Support for string refs will be removed in a future major release.',
104 + ]);
105 expect(Scheduler).toFlushWithoutYielding();
106 });
107
packages/react-dom/src/__tests__/ReactFunctionComponent-test.js
+3 -2
@@ -367,11 +367,12 @@ describe('ReactFunctionComponent', () => {
367 Child.defaultProps = {test: 2};
368 Child.propTypes = {test: PropTypes.string};
369
370 - expect(() => ReactTestUtils.renderIntoDocument(<Child />)).toErrorDev(
370 + expect(() => ReactTestUtils.renderIntoDocument(<Child />)).toErrorDev([
371 + 'Warning: Child: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.',
372 'Warning: Failed prop type: Invalid prop `test` of type `number` ' +
373 'supplied to `Child`, expected `string`.\n' +
374 ' in Child (at **)',
374 - );
375 + ]);
376 });
377
378 it('should receive context', () => {
packages/react-reconciler/src/ReactFiberBeginWork.new.js
+14
@@ -496,6 +496,20 @@ function updateMemoComponent(
496 getComponentNameFromType(type),
497 );
498 }
499 + if (
500 + warnAboutDefaultPropsOnFunctionComponents &&
501 + Component.defaultProps !== undefined
502 + ) {
503 + const componentName = getComponentNameFromType(type) || 'Unknown';
504 + if (!didWarnAboutDefaultPropsOnFunctionComponent[componentName]) {
505 + console.error(
506 + '%s: Support for defaultProps will be removed from memo components ' +
507 + 'in a future major release. Use JavaScript default parameters instead.',
508 + componentName,
509 + );
510 + didWarnAboutDefaultPropsOnFunctionComponent[componentName] = true;
511 + }
512 + }
513 }
514 const child = createFiberFromTypeAndProps(
515 Component.type,
packages/react-reconciler/src/ReactFiberBeginWork.old.js
+14
@@ -496,6 +496,20 @@ function updateMemoComponent(
496 getComponentNameFromType(type),
497 );
498 }
499 + if (
500 + warnAboutDefaultPropsOnFunctionComponents &&
501 + Component.defaultProps !== undefined
502 + ) {
503 + const componentName = getComponentNameFromType(type) || 'Unknown';
504 + if (!didWarnAboutDefaultPropsOnFunctionComponent[componentName]) {
505 + console.error(
506 + '%s: Support for defaultProps will be removed from memo components ' +
507 + 'in a future major release. Use JavaScript default parameters instead.',
508 + componentName,
509 + );
510 + didWarnAboutDefaultPropsOnFunctionComponent[componentName] = true;
511 + }
512 + }
513 }
514 const child = createFiberFromTypeAndProps(
515 Component.type,
packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js
+58 -14
@@ -293,7 +293,12 @@ describe('ReactLazy', () => {
293
294 await Promise.resolve();
295
296 - expect(Scheduler).toFlushAndYield(['Hi']);
296 + expect(() => expect(Scheduler).toFlushAndYield(['Hi'])).toErrorDev(
297 + 'Warning: T: Support for defaultProps ' +
298 + 'will be removed from function components in a future major ' +
299 + 'release. Use JavaScript default parameters instead.',
300 + );
301 +
302 expect(root).toMatchRenderedOutput('Hi');
303
304 T.defaultProps = {text: 'Hi again'};
@@ -343,7 +348,14 @@ describe('ReactLazy', () => {
348
349 await Promise.resolve();
350
346 - expect(Scheduler).toFlushAndYield(['Lazy', 'Sibling', 'A']);
351 + expect(() =>
352 + expect(Scheduler).toFlushAndYield(['Lazy', 'Sibling', 'A']),
353 + ).toErrorDev(
354 + 'Warning: LazyImpl: Support for defaultProps ' +
355 + 'will be removed from function components in a future major ' +
356 + 'release. Use JavaScript default parameters instead.',
357 + );
358 +
359 expect(root).toMatchRenderedOutput('SiblingA');
360
361 // Lazy should not re-render
@@ -643,7 +655,12 @@ describe('ReactLazy', () => {
655 expect(root).not.toMatchRenderedOutput('Hi Bye');
656
657 await Promise.resolve();
646 - expect(Scheduler).toFlushAndYield(['Hi Bye']);
658 + expect(() => expect(Scheduler).toFlushAndYield(['Hi Bye'])).toErrorDev(
659 + 'Warning: T: Support for defaultProps ' +
660 + 'will be removed from function components in a future major ' +
661 + 'release. Use JavaScript default parameters instead.',
662 + );
663 +
664 expect(root).toMatchRenderedOutput('Hi Bye');
665
666 root.update(
@@ -732,7 +749,11 @@ describe('ReactLazy', () => {
749 );
750 });
751
735 - async function verifyInnerPropTypesAreChecked(Add) {
752 + async function verifyInnerPropTypesAreChecked(
753 + Add,
754 + shouldWarnAboutFunctionDefaultProps,
755 + shouldWarnAboutMemoDefaultProps,
756 + ) {
757 const LazyAdd = lazy(() => fakeImport(Add));
758 expect(() => {
759 LazyAdd.propTypes = {};
@@ -753,15 +774,28 @@ describe('ReactLazy', () => {
774 );
775
776 expect(Scheduler).toFlushAndYield(['Loading...']);
777 +
778 expect(root).not.toMatchRenderedOutput('22');
779
780 // Mount
781 await Promise.resolve();
782 expect(() => {
783 Scheduler.unstable_flushAll();
762 - }).toErrorDev([
763 - 'Invalid prop `inner` of type `string` supplied to `Add`, expected `number`.',
764 - ]);
784 + }).toErrorDev(
785 + shouldWarnAboutFunctionDefaultProps
786 + ? [
787 + 'Add: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.',
788 + 'Invalid prop `inner` of type `string` supplied to `Add`, expected `number`.',
789 + ]
790 + : shouldWarnAboutMemoDefaultProps
791 + ? [
792 + 'Add: Support for defaultProps will be removed from memo components in a future major release. Use JavaScript default parameters instead.',
793 + 'Invalid prop `inner` of type `string` supplied to `Add`, expected `number`.',
794 + ]
795 + : [
796 + 'Invalid prop `inner` of type `string` supplied to `Add`, expected `number`.',
797 + ],
798 + );
799 expect(root).toMatchRenderedOutput('22');
800
801 // Update
@@ -792,7 +826,7 @@ describe('ReactLazy', () => {
826 Add.defaultProps = {
827 innerWithDefault: 42,
828 };
795 - await verifyInnerPropTypesAreChecked(Add);
829 + await verifyInnerPropTypesAreChecked(Add, true);
830 });
831
832 it('respects propTypes on function component without defaultProps', async () => {
@@ -874,7 +908,7 @@ describe('ReactLazy', () => {
908 Add.defaultProps = {
909 innerWithDefault: 42,
910 };
877 - await verifyInnerPropTypesAreChecked(Add);
911 + await verifyInnerPropTypesAreChecked(Add, false, true);
912 });
913
914 it('respects propTypes on outer memo component without defaultProps', async () => {
@@ -901,7 +935,7 @@ describe('ReactLazy', () => {
935 Add.defaultProps = {
936 innerWithDefault: 42,
937 };
904 - await verifyInnerPropTypesAreChecked(React.memo(Add));
938 + await verifyInnerPropTypesAreChecked(React.memo(Add), true);
939 });
940
941 it('respects propTypes on inner memo component without defaultProps', async () => {
@@ -944,9 +978,10 @@ describe('ReactLazy', () => {
978 await Promise.resolve();
979 expect(() => {
980 expect(Scheduler).toFlushAndYield(['Inner default text']);
947 - }).toErrorDev(
981 + }).toErrorDev([
982 + 'T: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.',
983 'The prop `text` is marked as required in `T`, but its value is `undefined`',
949 - );
984 + ]);
985 expect(root).toMatchRenderedOutput('Inner default text');
986
987 // Update
@@ -1058,7 +1093,11 @@ describe('ReactLazy', () => {
1093
1094 // Mount
1095 await Promise.resolve();
1061 - expect(Scheduler).toFlushWithoutYielding();
1096 + expect(() => {
1097 + expect(Scheduler).toFlushWithoutYielding();
1098 + }).toErrorDev(
1099 + 'Unknown: Support for defaultProps will be removed from memo components in a future major release. Use JavaScript default parameters instead.',
1100 + );
1101 expect(root).toMatchRenderedOutput('4');
1102
1103 // Update (shallowly equal)
@@ -1142,7 +1181,12 @@ describe('ReactLazy', () => {
1181
1182 // Mount
1183 await Promise.resolve();
1145 - expect(Scheduler).toFlushWithoutYielding();
1184 + expect(() => {
1185 + expect(Scheduler).toFlushWithoutYielding();
1186 + }).toErrorDev([
1187 + 'Memo: Support for defaultProps will be removed from memo components in a future major release. Use JavaScript default parameters instead.',
1188 + 'Unknown: Support for defaultProps will be removed from memo components in a future major release. Use JavaScript default parameters instead.',
1189 + ]);
1190 expect(root).toMatchRenderedOutput('4');
1191
1192 // Update
packages/react-reconciler/src/__tests__/ReactMemo-test.js
+11 -2
@@ -75,6 +75,7 @@ describe('memo', () => {
75 }
76 ReactNoop.render(<Outer />);
77 expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev([
78 + 'App: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.',
79 'Warning: Function components cannot be given refs. Attempts to access ' +
80 'this ref will fail.',
81 ]);
@@ -441,7 +442,11 @@ describe('memo', () => {
442 );
443 expect(Scheduler).toFlushAndYield(['Loading...']);
444 await Promise.resolve();
444 - expect(Scheduler).toFlushAndYield([15]);
445 + expect(() => {
446 + expect(Scheduler).toFlushAndYield([15]);
447 + }).toErrorDev([
448 + 'Counter: Support for defaultProps will be removed from memo components in a future major release. Use JavaScript default parameters instead.',
449 + ]);
450 expect(ReactNoop.getChildren()).toEqual([span(15)]);
451
452 // Should bail out because props have not changed
@@ -552,7 +557,11 @@ describe('memo', () => {
557 <Outer />
558 </div>,
559 );
555 - expect(Scheduler).toFlushWithoutYielding();
560 + expect(() => {
561 + expect(Scheduler).toFlushWithoutYielding();
562 + }).toErrorDev([
563 + 'Inner: Support for defaultProps will be removed from memo components in a future major release. Use JavaScript default parameters instead.',
564 + ]);
565
566 // Mount
567 expect(() => {
packages/shared/ReactFeatureFlags.js
+1 -1
@@ -213,7 +213,7 @@ export const disableTextareaChildren = false;
213 // Part of the simplification of React.createElement so we can eventually move
214 // from React.createElement to React.jsx
215 // https://github.com/reactjs/rfcs/blob/createlement-rfc/text/0000-create-element-changes.md
216 -export const warnAboutDefaultPropsOnFunctionComponents = false; // deprecate later, not 18.0
216 +export const warnAboutDefaultPropsOnFunctionComponents = true; // deprecate later, not 18.0
217
218 // Enables a warning when trying to spread a 'key' to an element;
219 // a deprecated pattern we want to get rid of in the future
packages/shared/forks/ReactFeatureFlags.native-fb.js
+1 -1
@@ -41,7 +41,7 @@ export const warnAboutDeprecatedLifecycles = true;
41 export const enableScopeAPI = false;
42 export const enableCreateEventHandleAPI = false;
43 export const enableSuspenseCallback = false;
44 -export const warnAboutDefaultPropsOnFunctionComponents = false;
44 +export const warnAboutDefaultPropsOnFunctionComponents = true;
45 export const warnAboutStringRefs = true;
46 export const disableLegacyContext = false;
47 export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
packages/shared/forks/ReactFeatureFlags.native-oss.js
+1 -1
@@ -31,7 +31,7 @@ export const enableSchedulerDebugging = false;
31 export const enableScopeAPI = false;
32 export const enableCreateEventHandleAPI = false;
33 export const enableSuspenseCallback = false;
34 -export const warnAboutDefaultPropsOnFunctionComponents = false;
34 +export const warnAboutDefaultPropsOnFunctionComponents = true;
35 export const warnAboutStringRefs = true;
36 export const disableLegacyContext = false;
37 export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.js
+1 -1
@@ -31,7 +31,7 @@ export const enableSchedulerDebugging = false;
31 export const enableScopeAPI = false;
32 export const enableCreateEventHandleAPI = false;
33 export const enableSuspenseCallback = false;
34 -export const warnAboutDefaultPropsOnFunctionComponents = false;
34 +export const warnAboutDefaultPropsOnFunctionComponents = true;
35 export const warnAboutStringRefs = true;
36 export const disableLegacyContext = false;
37 export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.native.js
+1 -1
@@ -31,7 +31,7 @@ export const enableSchedulerDebugging = false;
31 export const enableScopeAPI = false;
32 export const enableCreateEventHandleAPI = false;
33 export const enableSuspenseCallback = false;
34 -export const warnAboutDefaultPropsOnFunctionComponents = false;
34 +export const warnAboutDefaultPropsOnFunctionComponents = true;
35 export const warnAboutStringRefs = true;
36 export const disableLegacyContext = false;
37 export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
+1 -1
@@ -31,7 +31,7 @@ export const disableInputAttributeSyncing = false;
31 export const enableScopeAPI = true;
32 export const enableCreateEventHandleAPI = false;
33 export const enableSuspenseCallback = true;
34 -export const warnAboutDefaultPropsOnFunctionComponents = false;
34 +export const warnAboutDefaultPropsOnFunctionComponents = true;
35 export const warnAboutStringRefs = true;
36 export const disableLegacyContext = false;
37 export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
packages/shared/forks/ReactFeatureFlags.testing.js
+1 -1
@@ -31,7 +31,7 @@ export const enableSchedulerDebugging = false;
31 export const enableScopeAPI = false;
32 export const enableCreateEventHandleAPI = false;
33 export const enableSuspenseCallback = false;
34 -export const warnAboutDefaultPropsOnFunctionComponents = false;
34 +export const warnAboutDefaultPropsOnFunctionComponents = true;
35 export const warnAboutStringRefs = true;
36 export const disableLegacyContext = false;
37 export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
packages/shared/forks/ReactFeatureFlags.testing.www.js
+1 -1
@@ -31,7 +31,7 @@ export const enableSchedulerDebugging = false;
31 export const enableScopeAPI = true;
32 export const enableCreateEventHandleAPI = true;
33 export const enableSuspenseCallback = true;
34 -export const warnAboutDefaultPropsOnFunctionComponents = false;
34 +export const warnAboutDefaultPropsOnFunctionComponents = true;
35 export const warnAboutStringRefs = true;
36 export const disableLegacyContext = __EXPERIMENTAL__;
37 export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
packages/shared/forks/ReactFeatureFlags.www.js
+1 -1
@@ -67,7 +67,7 @@ export const enableSchedulerDebugging = true;
67 export const warnAboutDeprecatedLifecycles = true;
68 export const disableLegacyContext = __EXPERIMENTAL__;
69 export const warnAboutStringRefs = true;
70 -export const warnAboutDefaultPropsOnFunctionComponents = false;
70 +export const warnAboutDefaultPropsOnFunctionComponents = true;
71 export const enableGetInspectorDataForInstanceInProduction = false;
72
73 export const enableCache = true;