Skip to content

Commit fe27ef4

Browse files
sebmarkbagemofeiZ
authored andcommitted
Enable warning for defaultProps on function components for everyone (facebook#25699)
This also fixes a gap where were weren't warning on memo components.
1 parent 1d39f1e commit fe27ef4

17 files changed

+209
-79
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
@@ -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
@@ -302,6 +302,18 @@ describe('ReactDOMFizzServer', () => {
302302
}
303303

304304
it('should asynchronously load a lazy component', async () => {
305+
const originalConsoleError = console.error;
306+
const mockError = jest.fn();
307+
console.error = (...args) => {
308+
if (args.length > 1) {
309+
if (typeof args[1] === 'object') {
310+
mockError(args[0].split('\n')[0]);
311+
return;
312+
}
313+
}
314+
mockError(...args.map(normalizeCodeLocInfo));
315+
};
316+
305317
let resolveA;
306318
const LazyA = React.lazy(() => {
307319
return new Promise(r => {
@@ -324,48 +336,66 @@ describe('ReactDOMFizzServer', () => {
324336
punctuation: '!',
325337
};
326338

327-
await act(async () => {
328-
const {pipe} = renderToPipeableStream(
329-
<div>
330-
<div>
331-
<Suspense fallback={<Text text="Loading..." />}>
332-
<LazyA text="Hello" />
333-
</Suspense>
334-
</div>
339+
try {
340+
await act(async () => {
341+
const {pipe} = renderToPipeableStream(
335342
<div>
336-
<Suspense fallback={<Text text="Loading..." />}>
337-
<LazyB text="world" />
338-
</Suspense>
339-
</div>
343+
<div>
344+
<Suspense fallback={<Text text="Loading..." />}>
345+
<LazyA text="Hello" />
346+
</Suspense>
347+
</div>
348+
<div>
349+
<Suspense fallback={<Text text="Loading..." />}>
350+
<LazyB text="world" />
351+
</Suspense>
352+
</div>
353+
</div>,
354+
);
355+
pipe(writable);
356+
});
357+
358+
expect(getVisibleChildren(container)).toEqual(
359+
<div>
360+
<div>Loading...</div>
361+
<div>Loading...</div>
362+
</div>,
363+
);
364+
await act(async () => {
365+
resolveA({default: Text});
366+
});
367+
expect(getVisibleChildren(container)).toEqual(
368+
<div>
369+
<div>Hello</div>
370+
<div>Loading...</div>
371+
</div>,
372+
);
373+
await act(async () => {
374+
resolveB({default: TextWithPunctuation});
375+
});
376+
expect(getVisibleChildren(container)).toEqual(
377+
<div>
378+
<div>Hello</div>
379+
<div>world!</div>
340380
</div>,
341381
);
342-
pipe(writable);
343-
});
344382

345-
expect(getVisibleChildren(container)).toEqual(
346-
<div>
347-
<div>Loading...</div>
348-
<div>Loading...</div>
349-
</div>,
350-
);
351-
await act(async () => {
352-
resolveA({default: Text});
353-
});
354-
expect(getVisibleChildren(container)).toEqual(
355-
<div>
356-
<div>Hello</div>
357-
<div>Loading...</div>
358-
</div>,
359-
);
360-
await act(async () => {
361-
resolveB({default: TextWithPunctuation});
362-
});
363-
expect(getVisibleChildren(container)).toEqual(
364-
<div>
365-
<div>Hello</div>
366-
<div>world!</div>
367-
</div>,
368-
);
383+
if (__DEV__) {
384+
expect(mockError).toHaveBeenCalledWith(
385+
'Warning: %s: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.%s',
386+
'TextWithPunctuation',
387+
'\n in TextWithPunctuation (at **)\n' +
388+
' in Lazy (at **)\n' +
389+
' in Suspense (at **)\n' +
390+
' in div (at **)\n' +
391+
' in div (at **)',
392+
);
393+
} else {
394+
expect(mockError).not.toHaveBeenCalled();
395+
}
396+
} finally {
397+
console.error = originalConsoleError;
398+
}
369399
});
370400

371401
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)