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 11, 2019
1 parent 8533c0a commit ec7b480
Show file tree
Hide file tree
Showing 17 changed files with 325 additions and 394 deletions.
1 change: 1 addition & 0 deletions fixtures/dom/public/act-dom.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
this page tests whether act runs properly in a browser.
<br />
your console should say "5"
<div id='app'/>
<script src="scheduler-unstable_mock.development.js"></script>
<script src="react.development.js"></script>
<script type="text/javascript">
Expand Down
50 changes: 24 additions & 26 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,17 +1409,9 @@ 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',
'componentWillMount has been renamed',
'componentWillReceiveProps has been renamed',
'componentWillUpdate has been renamed',
],
{withoutStack: true},
);
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 component logic from componentWillMount into componentDidMount (preferred in most cases) ' +
'or the constructor.\n' +
'\nPlease update the following components: %s',
componentName,
);
didWarnAboutDeprecatedWillMount[componentName] = true;
Expand Down
5 changes: 0 additions & 5 deletions packages/react-reconciler/src/ReactFiberClassComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -806,11 +806,6 @@ function mountClassInstance(
}

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

ReactStrictModeWarnings.recordLegacyContextWarning(
workInProgress,
instance,
Expand Down
1 change: 0 additions & 1 deletion packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -2247,7 +2247,6 @@ function checkForNestedUpdates() {

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

if (warnAboutDeprecatedLifecycles) {
Expand Down
Loading

0 comments on commit ec7b480

Please sign in to comment.