Skip to content

Commit 5894232

Browse files
sebmarkbageacdlite
sebmarkbage
authored and
acdlite
committed
Enable warning for defaultProps on function components for everyone (#25699)
This also fixes a gap where were weren't warning on memo components.
1 parent 73bfaa1 commit 5894232

17 files changed

+142
-41
lines changed

‎packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -882,7 +882,11 @@ describe('ReactHooksInspectionIntegration', () => {
882882

883883
await LazyFoo;
884884

885-
Scheduler.unstable_flushAll();
885+
expect(() => {
886+
Scheduler.unstable_flushAll();
887+
}).toErrorDev([
888+
'Foo: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.',
889+
]);
886890

887891
const childFiber = renderer.root._currentFiber();
888892
const tree = ReactDebugTools.inspectHooksOfFiber(childFiber);

‎packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,7 @@ describe('ReactDOMFizzServer', () => {
264264
}
265265

266266
// @gate experimental
267+
// @gate !warnAboutDefaultPropsOnFunctionComponents || !__DEV__
267268
it('should asynchronously load a lazy component', async () => {
268269
let resolveA;
269270
const LazyA = React.lazy(() => {

‎packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.internal.js‎ renamed to ‎packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.js‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
'use strict';
1111

1212
let React;
13-
let ReactFeatureFlags;
1413
let ReactNoop;
1514
let Scheduler;
1615
let JSXDEVRuntime;
@@ -19,19 +18,11 @@ describe('ReactDeprecationWarnings', () => {
1918
beforeEach(() => {
2019
jest.resetModules();
2120
React = require('react');
22-
ReactFeatureFlags = require('shared/ReactFeatureFlags');
2321
ReactNoop = require('react-noop-renderer');
2422
Scheduler = require('scheduler');
2523
if (__DEV__) {
2624
JSXDEVRuntime = require('react/jsx-dev-runtime');
2725
}
28-
ReactFeatureFlags.warnAboutDefaultPropsOnFunctionComponents = true;
29-
ReactFeatureFlags.warnAboutStringRefs = true;
30-
});
31-
32-
afterEach(() => {
33-
ReactFeatureFlags.warnAboutDefaultPropsOnFunctionComponents = false;
34-
ReactFeatureFlags.warnAboutStringRefs = false;
3526
});
3627

3728
it('should warn when given defaultProps', () => {
@@ -51,6 +42,27 @@ describe('ReactDeprecationWarnings', () => {
5142
);
5243
});
5344

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+
5466
it('should warn when given string refs', () => {
5567
class RefComponent extends React.Component {
5668
render() {
@@ -74,9 +86,7 @@ describe('ReactDeprecationWarnings', () => {
7486
);
7587
});
7688

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', () => {
8090
class RefComponent extends React.Component {
8191
render() {
8292
return null;
@@ -87,7 +97,11 @@ describe('ReactDeprecationWarnings', () => {
8797
return <RefComponent ref="refComponent" __self={this} />;
8898
}
8999
}
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+
]);
91105
expect(Scheduler).toFlushWithoutYielding();
92106
});
93107

‎packages/react-dom/src/__tests__/ReactFunctionComponent-test.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -367,11 +367,12 @@ describe('ReactFunctionComponent', () => {
367367
Child.defaultProps = {test: 2};
368368
Child.propTypes = {test: PropTypes.string};
369369

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.',
371372
'Warning: Failed prop type: Invalid prop `test` of type `number` ' +
372373
'supplied to `Child`, expected `string`.\n' +
373374
' in Child (at **)',
374-
);
375+
]);
375376
});
376377

377378
it('should receive context', () => {

‎packages/react-reconciler/src/ReactFiberBeginWork.new.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,20 @@ function updateMemoComponent(
496496
getComponentNameFromType(type),
497497
);
498498
}
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+
}
499513
}
500514
const child = createFiberFromTypeAndProps(
501515
Component.type,

‎packages/react-reconciler/src/ReactFiberBeginWork.old.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,20 @@ function updateMemoComponent(
496496
getComponentNameFromType(type),
497497
);
498498
}
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+
}
499513
}
500514
const child = createFiberFromTypeAndProps(
501515
Component.type,

‎packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js‎

Lines changed: 58 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,12 @@ describe('ReactLazy', () => {
293293

294294
await Promise.resolve();
295295

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+
297302
expect(root).toMatchRenderedOutput('Hi');
298303

299304
T.defaultProps = {text: 'Hi again'};
@@ -343,7 +348,14 @@ describe('ReactLazy', () => {
343348

344349
await Promise.resolve();
345350

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+
347359
expect(root).toMatchRenderedOutput('SiblingA');
348360

349361
// Lazy should not re-render
@@ -643,7 +655,12 @@ describe('ReactLazy', () => {
643655
expect(root).not.toMatchRenderedOutput('Hi Bye');
644656

645657
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+
647664
expect(root).toMatchRenderedOutput('Hi Bye');
648665

649666
root.update(
@@ -732,7 +749,11 @@ describe('ReactLazy', () => {
732749
);
733750
});
734751

735-
async function verifyInnerPropTypesAreChecked(Add) {
752+
async function verifyInnerPropTypesAreChecked(
753+
Add,
754+
shouldWarnAboutFunctionDefaultProps,
755+
shouldWarnAboutMemoDefaultProps,
756+
) {
736757
const LazyAdd = lazy(() => fakeImport(Add));
737758
expect(() => {
738759
LazyAdd.propTypes = {};
@@ -753,15 +774,28 @@ describe('ReactLazy', () => {
753774
);
754775

755776
expect(Scheduler).toFlushAndYield(['Loading...']);
777+
756778
expect(root).not.toMatchRenderedOutput('22');
757779

758780
// Mount
759781
await Promise.resolve();
760782
expect(() => {
761783
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+
);
765799
expect(root).toMatchRenderedOutput('22');
766800

767801
// Update
@@ -792,7 +826,7 @@ describe('ReactLazy', () => {
792826
Add.defaultProps = {
793827
innerWithDefault: 42,
794828
};
795-
await verifyInnerPropTypesAreChecked(Add);
829+
await verifyInnerPropTypesAreChecked(Add, true);
796830
});
797831

798832
it('respects propTypes on function component without defaultProps', async () => {
@@ -874,7 +908,7 @@ describe('ReactLazy', () => {
874908
Add.defaultProps = {
875909
innerWithDefault: 42,
876910
};
877-
await verifyInnerPropTypesAreChecked(Add);
911+
await verifyInnerPropTypesAreChecked(Add, false, true);
878912
});
879913

880914
it('respects propTypes on outer memo component without defaultProps', async () => {
@@ -901,7 +935,7 @@ describe('ReactLazy', () => {
901935
Add.defaultProps = {
902936
innerWithDefault: 42,
903937
};
904-
await verifyInnerPropTypesAreChecked(React.memo(Add));
938+
await verifyInnerPropTypesAreChecked(React.memo(Add), true);
905939
});
906940

907941
it('respects propTypes on inner memo component without defaultProps', async () => {
@@ -944,9 +978,10 @@ describe('ReactLazy', () => {
944978
await Promise.resolve();
945979
expect(() => {
946980
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.',
948983
'The prop `text` is marked as required in `T`, but its value is `undefined`',
949-
);
984+
]);
950985
expect(root).toMatchRenderedOutput('Inner default text');
951986

952987
// Update
@@ -1058,7 +1093,11 @@ describe('ReactLazy', () => {
10581093

10591094
// Mount
10601095
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+
);
10621101
expect(root).toMatchRenderedOutput('4');
10631102

10641103
// Update (shallowly equal)
@@ -1142,7 +1181,12 @@ describe('ReactLazy', () => {
11421181

11431182
// Mount
11441183
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+
]);
11461190
expect(root).toMatchRenderedOutput('4');
11471191

11481192
// Update

‎packages/react-reconciler/src/__tests__/ReactMemo-test.js‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ describe('memo', () => {
7575
}
7676
ReactNoop.render(<Outer />);
7777
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.',
7879
'Warning: Function components cannot be given refs. Attempts to access ' +
7980
'this ref will fail.',
8081
]);
@@ -441,7 +442,11 @@ describe('memo', () => {
441442
);
442443
expect(Scheduler).toFlushAndYield(['Loading...']);
443444
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+
]);
445450
expect(ReactNoop.getChildren()).toEqual([span(15)]);
446451

447452
// Should bail out because props have not changed
@@ -552,7 +557,11 @@ describe('memo', () => {
552557
<Outer />
553558
</div>,
554559
);
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+
]);
556565

557566
// Mount
558567
expect(() => {

‎packages/shared/ReactFeatureFlags.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ export const disableTextareaChildren = false;
205205
// Part of the simplification of React.createElement so we can eventually move
206206
// from React.createElement to React.jsx
207207
// https://github.com/reactjs/rfcs/blob/createlement-rfc/text/0000-create-element-changes.md
208-
export const warnAboutDefaultPropsOnFunctionComponents = false; // deprecate later, not 18.0
208+
export const warnAboutDefaultPropsOnFunctionComponents = true; // deprecate later, not 18.0
209209

210210
// Enables a warning when trying to spread a 'key' to an element;
211211
// a deprecated pattern we want to get rid of in the future

‎packages/shared/forks/ReactFeatureFlags.native-fb.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export const warnAboutDeprecatedLifecycles = true;
3939
export const enableScopeAPI = false;
4040
export const enableCreateEventHandleAPI = false;
4141
export const enableSuspenseCallback = false;
42-
export const warnAboutDefaultPropsOnFunctionComponents = false;
42+
export const warnAboutDefaultPropsOnFunctionComponents = true;
4343
export const warnAboutStringRefs = true;
4444
export const disableLegacyContext = false;
4545
export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;

0 commit comments

Comments
 (0)