Skip to content

Commit

Permalink
unify deprecated/unsafe lifecycle warnings, pass tests
Browse files Browse the repository at this point in the history
- redoes #15431 from scratch, taking on the feedback there
- unifies the messaging between "deprecated" and UNSAFE_ lifecycle messages. It reorganizes ReactStrictModeWarnings.js to capture and flush all the lifecycle warnings in one procedure each.
- matches the warning from ReactPartialRenderer to match the above change
- passes all the tests
- this also turns on `warnAboutDeprecatedLifecycles` for the test renderer. I think we missed doing so it previously. In a future PR, I'll remove the feature flag altogether.
- this DOES NOT do the same treatment for context warnings, I'll do that in another PR too
  • Loading branch information
threepointone committed Jul 12, 2019
1 parent b766904 commit abe5f20
Show file tree
Hide file tree
Showing 16 changed files with 410 additions and 361 deletions.
73 changes: 46 additions & 27 deletions packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -709,9 +709,9 @@ describe('ReactComponentLifeCycle', () => {
);
}).toLowPriorityWarnDev(
[
'componentWillMount is deprecated',
'componentWillReceiveProps is deprecated',
'componentWillUpdate is deprecated',
'componentWillMount has been renamed',
'componentWillReceiveProps has been renamed',
'componentWillUpdate has been renamed',
],
{withoutStack: true},
);
Expand Down Expand Up @@ -748,9 +748,9 @@ describe('ReactComponentLifeCycle', () => {
);
}).toLowPriorityWarnDev(
[
'componentWillMount is deprecated',
'componentWillReceiveProps is deprecated',
'componentWillUpdate is deprecated',
'componentWillMount has been renamed',
'componentWillReceiveProps has been renamed',
'componentWillUpdate has been renamed',
],
{withoutStack: true},
);
Expand Down Expand Up @@ -815,7 +815,10 @@ describe('ReactComponentLifeCycle', () => {
{withoutStack: true},
);
}).toLowPriorityWarnDev(
['componentWillMount is deprecated', 'componentWillUpdate is deprecated'],
[
'componentWillMount has been renamed',
'componentWillUpdate has been renamed',
],
{withoutStack: true},
);

Expand Down Expand Up @@ -863,7 +866,7 @@ describe('ReactComponentLifeCycle', () => {
'https://fb.me/react-async-component-lifecycle-hooks',
{withoutStack: true},
);
}).toLowPriorityWarnDev(['componentWillMount is deprecated'], {
}).toLowPriorityWarnDev(['componentWillMount has been renamed'], {
withoutStack: true,
});

Expand All @@ -887,7 +890,7 @@ describe('ReactComponentLifeCycle', () => {
'https://fb.me/react-async-component-lifecycle-hooks',
{withoutStack: true},
);
}).toLowPriorityWarnDev(['componentWillReceiveProps is deprecated'], {
}).toLowPriorityWarnDev(['componentWillReceiveProps has been renamed'], {
withoutStack: true,
});
});
Expand Down Expand Up @@ -921,7 +924,10 @@ describe('ReactComponentLifeCycle', () => {
{withoutStack: true},
);
}).toLowPriorityWarnDev(
['componentWillMount is deprecated', 'componentWillUpdate is deprecated'],
[
'componentWillMount has been renamed',
'componentWillUpdate has been renamed',
],
{withoutStack: true},
);

Expand Down Expand Up @@ -967,7 +973,7 @@ describe('ReactComponentLifeCycle', () => {
'https://fb.me/react-async-component-lifecycle-hooks',
{withoutStack: true},
);
}).toLowPriorityWarnDev(['componentWillMount is deprecated'], {
}).toLowPriorityWarnDev(['componentWillMount has been renamed'], {
withoutStack: true,
});

Expand All @@ -990,7 +996,7 @@ describe('ReactComponentLifeCycle', () => {
'https://fb.me/react-async-component-lifecycle-hooks',
{withoutStack: true},
);
}).toLowPriorityWarnDev(['componentWillReceiveProps is deprecated'], {
}).toLowPriorityWarnDev(['componentWillReceiveProps has been renamed'], {
withoutStack: true,
});
});
Expand Down Expand Up @@ -1130,9 +1136,9 @@ describe('ReactComponentLifeCycle', () => {
ReactDOM.render(<MyComponent foo="bar" />, div),
).toLowPriorityWarnDev(
[
'componentWillMount is deprecated',
'componentWillReceiveProps is deprecated',
'componentWillUpdate is deprecated',
'componentWillMount has been renamed',
'componentWillReceiveProps has been renamed',
'componentWillUpdate has been renamed',
],
{withoutStack: true},
);
Expand Down Expand Up @@ -1403,22 +1409,35 @@ describe('ReactComponentLifeCycle', () => {
ReactDOM.render(<MyComponent x={1} />, container),
).toLowPriorityWarnDev(
[
'componentWillMount is deprecated and will be removed in the next major version. ' +
'Use componentDidMount instead. As a temporary workaround, ' +
'you can rename to UNSAFE_componentWillMount.' +
'\n\nPlease update the following components: MyComponent',
'componentWillReceiveProps is deprecated and will be removed in the next major version. ' +
'Use static getDerivedStateFromProps instead.' +
'\n\nPlease update the following components: MyComponent',
'componentWillUpdate is deprecated and will be removed in the next major version. ' +
'Use componentDidUpdate instead. As a temporary workaround, ' +
'you can rename to UNSAFE_componentWillUpdate.' +
'\n\nPlease update the following components: MyComponent',
/* eslint-disable max-len */
`Warning: componentWillMount has been renamed, and is not recommended for use. See https://fb.me/react-async-component-lifecycle-hooks for details.
* Move code from componentWillMount to componentDidMount (preferred in most cases) or the constructor.
* Rename componentWillMount to UNSAFE_componentWillMount to suppress this warning in non-strict mode. In React 17.x, only the UNSAFE_ name will work.
* To rename all deprecated lifecycles to their new names, you can run \`npx react-codemod rename-unsafe-lifecycles /path/to/code\`.
Please update the following components: MyComponent`,
`Warning: componentWillReceiveProps has been renamed, and is not recommended for use. See https://fb.me/react-async-component-lifecycle-hooks for details.
* Move data fetching code or side effects from componentWillReceiveProps to componentDidUpdate.
* If you're updating state whenever props change, refactor your code to use memoization techniques, or move it to static getDerivedStateFromProps. Learn more at: https://fb.me/react-derived-state
* Rename componentWillReceiveProps to UNSAFE_componentWillReceiveProps to suppress this warning in non-strict mode. In React 17.x, only the UNSAFE_ name will work.
* To rename all deprecated lifecycles to their new names, you can run \`npx react-codemod rename-unsafe-lifecycles /path/to/code\`.
Please update the following components: MyComponent`,
`Warning: componentWillUpdate has been renamed, and is not recommended for use. See https://fb.me/react-async-component-lifecycle-hooks for details.
* Move data fetching code or side effects from componentWillUpdate to componentDidUpdate.
* Rename componentWillUpdate to UNSAFE_componentWillUpdate to suppress this warning in non-strict mode. In React 17.x, only the UNSAFE_ name will work.
* To rename all deprecated lifecycles to their new names, you can run \`npx react-codemod rename-unsafe-lifecycles /path/to/code\`.
Please update the following components: MyComponent`,
/* eslint-enable max-len */
],
{withoutStack: true},
);

// Dedupe check (update and instantiate new
// Dedupe check (update and instantiate new)
ReactDOM.render(<MyComponent x={2} />, container);
ReactDOM.render(<MyComponent key="new" x={1} />, container);
});
Expand Down
17 changes: 7 additions & 10 deletions packages/react-dom/src/__tests__/ReactDOMServerLifecycles-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ describe('ReactDOMServerLifecycles', () => {

expect(() =>
ReactDOMServer.renderToString(<Component />),
).toLowPriorityWarnDev('componentWillMount() is deprecated', {
).toLowPriorityWarnDev('componentWillMount has been renamed', {
withoutStack: true,
});
expect(log).toEqual(['componentWillMount', 'UNSAFE_componentWillMount']);
Expand Down Expand Up @@ -286,10 +286,9 @@ describe('ReactDOMServerLifecycles', () => {

expect(() =>
ReactDOMServer.renderToString(<Component />),
).toLowPriorityWarnDev(
'Component: componentWillMount() is deprecated and will be removed in the next major version.',
{withoutStack: true},
);
).toLowPriorityWarnDev('componentWillMount has been renamed', {
withoutStack: true,
});
});

it('should warn about deprecated lifecycle hooks', () => {
Expand All @@ -302,11 +301,9 @@ describe('ReactDOMServerLifecycles', () => {

expect(() =>
ReactDOMServer.renderToString(<Component />),
).toLowPriorityWarnDev(
'Warning: Component: componentWillMount() is deprecated and will be removed ' +
'in the next major version.',
{withoutStack: true},
);
).toLowPriorityWarnDev('componentWillMount has been renamed', {
withoutStack: true,
});

// De-duped
ReactDOMServer.renderToString(<Component />);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -362,20 +362,16 @@ describe('ReactDOMServerHydration', () => {
const element = document.createElement('div');
expect(() => {
element.innerHTML = ReactDOMServer.renderToString(markup);
}).toLowPriorityWarnDev(
['componentWillMount() is deprecated and will be removed'],
{withoutStack: true},
);
}).toLowPriorityWarnDev(['componentWillMount has been renamed'], {
withoutStack: true,
});
expect(element.textContent).toBe('Hi');

expect(() => {
expect(() => ReactDOM.hydrate(markup, element)).toWarnDev(
'Please update the following components to use componentDidMount instead: ComponentWithWarning',
);
}).toLowPriorityWarnDev(
['componentWillMount is deprecated and will be removed'],
{withoutStack: true},
);
ReactDOM.hydrate(markup, element);
}).toLowPriorityWarnDev(['componentWillMount has been renamed'], {
withoutStack: true,
});
expect(element.textContent).toBe('Hi');
});

Expand Down
13 changes: 6 additions & 7 deletions packages/react-dom/src/server/ReactPartialRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -572,13 +572,12 @@ function resolve(
if (!didWarnAboutDeprecatedWillMount[componentName]) {
lowPriorityWarning(
false,
'%s: componentWillMount() is deprecated and will be ' +
'removed in the next major version. Read about the motivations ' +
'behind this change: ' +
'https://fb.me/react-async-component-lifecycle-hooks' +
'\n\n' +
'As a temporary workaround, you can rename to ' +
'UNSAFE_componentWillMount instead.',
// keep this warning in sync with ReactStrictModeWarning.js
'componentWillMount has been renamed, and is not recommended for use. ' +
'See https://fb.me/react-async-component-lifecycle-hooks for details.\n\n' +
'* Move code from componentWillMount to componentDidMount (preferred in most cases) ' +
'or the constructor.\n' +
'\nPlease update the following components: %s',
componentName,
);
didWarnAboutDeprecatedWillMount[componentName] = true;
Expand Down
7 changes: 1 addition & 6 deletions packages/react-reconciler/src/ReactFiberClassComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -806,19 +806,14 @@ function mountClassInstance(
}

if (workInProgress.mode & StrictMode) {
ReactStrictModeWarnings.recordUnsafeLifecycleWarnings(
workInProgress,
instance,
);

ReactStrictModeWarnings.recordLegacyContextWarning(
workInProgress,
instance,
);
}

if (warnAboutDeprecatedLifecycles) {
ReactStrictModeWarnings.recordDeprecationWarnings(
ReactStrictModeWarnings.recordUnsafeLifecycleWarnings(
workInProgress,
instance,
);
Expand Down
3 changes: 1 addition & 2 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -2247,11 +2247,10 @@ function checkForNestedUpdates() {

function flushRenderPhaseStrictModeWarningsInDEV() {
if (__DEV__) {
ReactStrictModeWarnings.flushPendingUnsafeLifecycleWarnings();
ReactStrictModeWarnings.flushLegacyContextWarning();

if (warnAboutDeprecatedLifecycles) {
ReactStrictModeWarnings.flushPendingDeprecationWarnings();
ReactStrictModeWarnings.flushPendingUnsafeLifecycleWarnings();
}
}
}
Expand Down
Loading

0 comments on commit abe5f20

Please sign in to comment.