Enable warning for defaultProps on function components for everyone (#…

…25699) This also fixes a gap where were weren't warning on memo components.
facebook · Apr 16, 2024 · 5894232 · 5894232
1 parent 73bfaa1
commit 5894232
Show file tree

Hide file tree

Showing 17 changed files with 142 additions and 41 deletions.
diff --git a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js
@@ -882,7 +882,11 @@ describe('ReactHooksInspectionIntegration', () => {
 
     await LazyFoo;
 
-    Scheduler.unstable_flushAll();
+    expect(() => {
+      Scheduler.unstable_flushAll();
+    }).toErrorDev([
+      'Foo: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.',
+    ]);
 
     const childFiber = renderer.root._currentFiber();
     const tree = ReactDebugTools.inspectHooksOfFiber(childFiber);

diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
@@ -264,6 +264,7 @@ describe('ReactDOMFizzServer', () => {
   }
 
   // @gate experimental
+  // @gate !warnAboutDefaultPropsOnFunctionComponents || !__DEV__
   it('should asynchronously load a lazy component', async () => {
     let resolveA;
     const LazyA = React.lazy(() => {

diff --git a/...ReactDeprecationWarnings-test.internal.js → ..._tests__/ReactDeprecationWarnings-test.js b/...ReactDeprecationWarnings-test.internal.js → ..._tests__/ReactDeprecationWarnings-test.js
@@ -10,7 +10,6 @@
 'use strict';
 
 let React;
-let ReactFeatureFlags;
 let ReactNoop;
 let Scheduler;
 let JSXDEVRuntime;
@@ -19,19 +18,11 @@ describe('ReactDeprecationWarnings', () => {
   beforeEach(() => {
     jest.resetModules();
     React = require('react');
-    ReactFeatureFlags = require('shared/ReactFeatureFlags');
     ReactNoop = require('react-noop-renderer');
     Scheduler = require('scheduler');
     if (__DEV__) {
       JSXDEVRuntime = require('react/jsx-dev-runtime');
     }
-    ReactFeatureFlags.warnAboutDefaultPropsOnFunctionComponents = true;
-    ReactFeatureFlags.warnAboutStringRefs = true;
-  });
-
-  afterEach(() => {
-    ReactFeatureFlags.warnAboutDefaultPropsOnFunctionComponents = false;
-    ReactFeatureFlags.warnAboutStringRefs = false;
   });
 
   it('should warn when given defaultProps', () => {
@@ -51,6 +42,27 @@ describe('ReactDeprecationWarnings', () => {
     );
   });
 
+  it('should warn when given defaultProps on a memoized function', () => {
+    const MemoComponent = React.memo(function FunctionalComponent(props) {
+      return null;
+    });
+
+    MemoComponent.defaultProps = {
+      testProp: true,
+    };
+
+    ReactNoop.render(
+      <div>
+        <MemoComponent />
+      </div>,
+    );
+    expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev(
+      'Warning: FunctionalComponent: Support for defaultProps ' +
+        'will be removed from memo components in a future major ' +
+        'release. Use JavaScript default parameters instead.',
+    );
+  });
+
   it('should warn when given string refs', () => {
     class RefComponent extends React.Component {
       render() {
@@ -74,9 +86,7 @@ describe('ReactDeprecationWarnings', () => {
     );
   });
 
-  it('should not warn when owner and self are the same for string refs', () => {
-    ReactFeatureFlags.warnAboutStringRefs = false;
-
+  it('should warn when owner and self are the same for string refs', () => {
     class RefComponent extends React.Component {
       render() {
         return null;
@@ -87,7 +97,11 @@ describe('ReactDeprecationWarnings', () => {
         return <RefComponent ref="refComponent" __self={this} />;
       }
     }
-    ReactNoop.renderLegacySyncRoot(<Component />);
+    expect(() => {
+      ReactNoop.renderLegacySyncRoot(<Component />);
+    }).toErrorDev([
+      'Component "Component" contains the string ref "refComponent". Support for string refs will be removed in a future major release.',
+    ]);
     expect(Scheduler).toFlushWithoutYielding();
   });
 

diff --git a/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js b/packages/react-dom/src/__tests__/ReactFunctionComponent-test.js
@@ -367,11 +367,12 @@ describe('ReactFunctionComponent', () => {
     Child.defaultProps = {test: 2};
     Child.propTypes = {test: PropTypes.string};
 
-    expect(() => ReactTestUtils.renderIntoDocument(<Child />)).toErrorDev(
+    expect(() => ReactTestUtils.renderIntoDocument(<Child />)).toErrorDev([
+      'Warning: Child: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.',
       'Warning: Failed prop type: Invalid prop `test` of type `number` ' +
         'supplied to `Child`, expected `string`.\n' +
         '    in Child (at **)',
-    );
+    ]);
   });
 
   it('should receive context', () => {

diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js
@@ -496,6 +496,20 @@ function updateMemoComponent(
           getComponentNameFromType(type),
         );
       }
+      if (
+        warnAboutDefaultPropsOnFunctionComponents &&
+        Component.defaultProps !== undefined
+      ) {
+        const componentName = getComponentNameFromType(type) || 'Unknown';
+        if (!didWarnAboutDefaultPropsOnFunctionComponent[componentName]) {
+          console.error(
+            '%s: Support for defaultProps will be removed from memo components ' +
+              'in a future major release. Use JavaScript default parameters instead.',
+            componentName,
+          );
+          didWarnAboutDefaultPropsOnFunctionComponent[componentName] = true;
+        }
+      }
     }
     const child = createFiberFromTypeAndProps(
       Component.type,

diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js
@@ -496,6 +496,20 @@ function updateMemoComponent(
           getComponentNameFromType(type),
         );
       }
+      if (
+        warnAboutDefaultPropsOnFunctionComponents &&
+        Component.defaultProps !== undefined
+      ) {
+        const componentName = getComponentNameFromType(type) || 'Unknown';
+        if (!didWarnAboutDefaultPropsOnFunctionComponent[componentName]) {
+          console.error(
+            '%s: Support for defaultProps will be removed from memo components ' +
+              'in a future major release. Use JavaScript default parameters instead.',
+            componentName,
+          );
+          didWarnAboutDefaultPropsOnFunctionComponent[componentName] = true;
+        }
+      }
     }
     const child = createFiberFromTypeAndProps(
       Component.type,

diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js
@@ -293,7 +293,12 @@ describe('ReactLazy', () => {
 
     await Promise.resolve();
 
-    expect(Scheduler).toFlushAndYield(['Hi']);
+    expect(() => expect(Scheduler).toFlushAndYield(['Hi'])).toErrorDev(
+      'Warning: T: Support for defaultProps ' +
+        'will be removed from function components in a future major ' +
+        'release. Use JavaScript default parameters instead.',
+    );
+
     expect(root).toMatchRenderedOutput('Hi');
 
     T.defaultProps = {text: 'Hi again'};
@@ -343,7 +348,14 @@ describe('ReactLazy', () => {
 
     await Promise.resolve();
 
-    expect(Scheduler).toFlushAndYield(['Lazy', 'Sibling', 'A']);
+    expect(() =>
+      expect(Scheduler).toFlushAndYield(['Lazy', 'Sibling', 'A']),
+    ).toErrorDev(
+      'Warning: LazyImpl: Support for defaultProps ' +
+        'will be removed from function components in a future major ' +
+        'release. Use JavaScript default parameters instead.',
+    );
+
     expect(root).toMatchRenderedOutput('SiblingA');
 
     // Lazy should not re-render
@@ -643,7 +655,12 @@ describe('ReactLazy', () => {
     expect(root).not.toMatchRenderedOutput('Hi Bye');
 
     await Promise.resolve();
-    expect(Scheduler).toFlushAndYield(['Hi Bye']);
+    expect(() => expect(Scheduler).toFlushAndYield(['Hi Bye'])).toErrorDev(
+      'Warning: T: Support for defaultProps ' +
+        'will be removed from function components in a future major ' +
+        'release. Use JavaScript default parameters instead.',
+    );
+
     expect(root).toMatchRenderedOutput('Hi Bye');
 
     root.update(
@@ -732,7 +749,11 @@ describe('ReactLazy', () => {
     );
   });
 
-  async function verifyInnerPropTypesAreChecked(Add) {
+  async function verifyInnerPropTypesAreChecked(
+    Add,
+    shouldWarnAboutFunctionDefaultProps,
+    shouldWarnAboutMemoDefaultProps,
+  ) {
     const LazyAdd = lazy(() => fakeImport(Add));
     expect(() => {
       LazyAdd.propTypes = {};
@@ -753,15 +774,28 @@ describe('ReactLazy', () => {
     );
 
     expect(Scheduler).toFlushAndYield(['Loading...']);
+
     expect(root).not.toMatchRenderedOutput('22');
 
     // Mount
     await Promise.resolve();
     expect(() => {
       Scheduler.unstable_flushAll();
-    }).toErrorDev([
-      'Invalid prop `inner` of type `string` supplied to `Add`, expected `number`.',
-    ]);
+    }).toErrorDev(
+      shouldWarnAboutFunctionDefaultProps
+        ? [
+            'Add: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.',
+            'Invalid prop `inner` of type `string` supplied to `Add`, expected `number`.',
+          ]
+        : shouldWarnAboutMemoDefaultProps
+        ? [
+            'Add: Support for defaultProps will be removed from memo components in a future major release. Use JavaScript default parameters instead.',
+            'Invalid prop `inner` of type `string` supplied to `Add`, expected `number`.',
+          ]
+        : [
+            'Invalid prop `inner` of type `string` supplied to `Add`, expected `number`.',
+          ],
+    );
     expect(root).toMatchRenderedOutput('22');
 
     // Update
@@ -792,7 +826,7 @@ describe('ReactLazy', () => {
     Add.defaultProps = {
       innerWithDefault: 42,
     };
-    await verifyInnerPropTypesAreChecked(Add);
+    await verifyInnerPropTypesAreChecked(Add, true);
   });
 
   it('respects propTypes on function component without defaultProps', async () => {
@@ -874,7 +908,7 @@ describe('ReactLazy', () => {
     Add.defaultProps = {
       innerWithDefault: 42,
     };
-    await verifyInnerPropTypesAreChecked(Add);
+    await verifyInnerPropTypesAreChecked(Add, false, true);
   });
 
   it('respects propTypes on outer memo component without defaultProps', async () => {
@@ -901,7 +935,7 @@ describe('ReactLazy', () => {
     Add.defaultProps = {
       innerWithDefault: 42,
     };
-    await verifyInnerPropTypesAreChecked(React.memo(Add));
+    await verifyInnerPropTypesAreChecked(React.memo(Add), true);
   });
 
   it('respects propTypes on inner memo component without defaultProps', async () => {
@@ -944,9 +978,10 @@ describe('ReactLazy', () => {
     await Promise.resolve();
     expect(() => {
       expect(Scheduler).toFlushAndYield(['Inner default text']);
-    }).toErrorDev(
+    }).toErrorDev([
+      'T: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.',
       'The prop `text` is marked as required in `T`, but its value is `undefined`',
-    );
+    ]);
     expect(root).toMatchRenderedOutput('Inner default text');
 
     // Update
@@ -1058,7 +1093,11 @@ describe('ReactLazy', () => {
 
     // Mount
     await Promise.resolve();
-    expect(Scheduler).toFlushWithoutYielding();
+    expect(() => {
+      expect(Scheduler).toFlushWithoutYielding();
+    }).toErrorDev(
+      'Unknown: Support for defaultProps will be removed from memo components in a future major release. Use JavaScript default parameters instead.',
+    );
     expect(root).toMatchRenderedOutput('4');
 
     // Update (shallowly equal)
@@ -1142,7 +1181,12 @@ describe('ReactLazy', () => {
 
     // Mount
     await Promise.resolve();
-    expect(Scheduler).toFlushWithoutYielding();
+    expect(() => {
+      expect(Scheduler).toFlushWithoutYielding();
+    }).toErrorDev([
+      'Memo: Support for defaultProps will be removed from memo components in a future major release. Use JavaScript default parameters instead.',
+      'Unknown: Support for defaultProps will be removed from memo components in a future major release. Use JavaScript default parameters instead.',
+    ]);
     expect(root).toMatchRenderedOutput('4');
 
     // Update

diff --git a/packages/react-reconciler/src/__tests__/ReactMemo-test.js b/packages/react-reconciler/src/__tests__/ReactMemo-test.js
@@ -75,6 +75,7 @@ describe('memo', () => {
     }
     ReactNoop.render(<Outer />);
     expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev([
+      'App: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.',
       'Warning: Function components cannot be given refs. Attempts to access ' +
         'this ref will fail.',
     ]);
@@ -441,7 +442,11 @@ describe('memo', () => {
         );
         expect(Scheduler).toFlushAndYield(['Loading...']);
         await Promise.resolve();
-        expect(Scheduler).toFlushAndYield([15]);
+        expect(() => {
+          expect(Scheduler).toFlushAndYield([15]);
+        }).toErrorDev([
+          'Counter: Support for defaultProps will be removed from memo components in a future major release. Use JavaScript default parameters instead.',
+        ]);
         expect(ReactNoop.getChildren()).toEqual([span(15)]);
 
         // Should bail out because props have not changed
@@ -552,7 +557,11 @@ describe('memo', () => {
             <Outer />
           </div>,
         );
-        expect(Scheduler).toFlushWithoutYielding();
+        expect(() => {
+          expect(Scheduler).toFlushWithoutYielding();
+        }).toErrorDev([
+          'Inner: Support for defaultProps will be removed from memo components in a future major release. Use JavaScript default parameters instead.',
+        ]);
 
         // Mount
         expect(() => {

diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js
@@ -205,7 +205,7 @@ export const disableTextareaChildren = false;
 // Part of the simplification of React.createElement so we can eventually move
 // from React.createElement to React.jsx
 // https://github.com/reactjs/rfcs/blob/createlement-rfc/text/0000-create-element-changes.md
-export const warnAboutDefaultPropsOnFunctionComponents = false; // deprecate later, not 18.0
+export const warnAboutDefaultPropsOnFunctionComponents = true; // deprecate later, not 18.0
 
 // Enables a warning when trying to spread a 'key' to an element;
 // a deprecated pattern we want to get rid of in the future

diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js
@@ -39,7 +39,7 @@ export const warnAboutDeprecatedLifecycles = true;
 export const enableScopeAPI = false;
 export const enableCreateEventHandleAPI = false;
 export const enableSuspenseCallback = false;
-export const warnAboutDefaultPropsOnFunctionComponents = false;
+export const warnAboutDefaultPropsOnFunctionComponents = true;
 export const warnAboutStringRefs = true;
 export const disableLegacyContext = false;
 export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;

diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js
@@ -29,7 +29,7 @@ export const enableSchedulerDebugging = false;
 export const enableScopeAPI = false;
 export const enableCreateEventHandleAPI = false;
 export const enableSuspenseCallback = false;
-export const warnAboutDefaultPropsOnFunctionComponents = false;
+export const warnAboutDefaultPropsOnFunctionComponents = true;
 export const warnAboutStringRefs = true;
 export const disableLegacyContext = false;
 export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;

diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js
@@ -29,7 +29,7 @@ export const enableSchedulerDebugging = false;
 export const enableScopeAPI = false;
 export const enableCreateEventHandleAPI = false;
 export const enableSuspenseCallback = false;
-export const warnAboutDefaultPropsOnFunctionComponents = false;
+export const warnAboutDefaultPropsOnFunctionComponents = true;
 export const warnAboutStringRefs = true;
 export const disableLegacyContext = false;
 export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;

diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js
@@ -29,7 +29,7 @@ export const enableSchedulerDebugging = false;
 export const enableScopeAPI = false;
 export const enableCreateEventHandleAPI = false;
 export const enableSuspenseCallback = false;
-export const warnAboutDefaultPropsOnFunctionComponents = false;
+export const warnAboutDefaultPropsOnFunctionComponents = true;
 export const warnAboutStringRefs = true;
 export const disableLegacyContext = false;
 export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;