Skip to content

Commit 7b17f7b

Browse files
sebmarkbagesebmarkbage
authored
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 6fb8133 commit 7b17f7b

17 files changed

Lines changed: 209 additions & 79 deletions

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

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

899899
await LazyFoo;
900900

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+
]);
902906

903907
const childFiber = renderer.root._currentFiber();
904908
const tree = ReactDebugTools.inspectHooksOfFiber(childFiber);

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

Lines changed: 68 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,18 @@ describe('ReactDOMFizzServer', () => {
291291
}
292292

293293
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+
294306
let resolveA;
295307
const LazyA = React.lazy(() => {
296308
return new Promise(r => {
@@ -313,48 +325,66 @@ describe('ReactDOMFizzServer', () => {
313325
punctuation: '!',
314326
};
315327

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(
324331
<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>
329369
</div>,
330370
);
331-
pipe(writable);
332-
});
333371

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+
}
358388
});
359389

360390
it('#23331: does not warn about hydration mismatches if something suspended in an earlier sibling', async () => {

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,

0 commit comments

Comments
 (0)