tweak unsafe lifecycle warnings #15431

threepointone · 2019-04-17T12:15:42Z

explicitly recommend actions as a way to silence the warning
be clear that the renaming only works for non strict modes
include codemod suggestion

tbh it's still not great, but started a PR to bikeshed. lemme know how to make this better.

This is what it looks like - (updated 18/05)

sizebot · 2019-04-17T12:26:24Z

ReactDOM: size: 0.0%, gzip: -0.0%

Details of bundled changes.

Comparing: 4221565...1ba1642

react-dom

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
react-dom.development.js	+0.2%	+0.2%	810.69 KB	812.22 KB	185.02 KB	185.32 KB	UMD_DEV
react-dom.production.min.js	0.0%	-0.0%	103.58 KB	103.58 KB	33.65 KB	33.65 KB	UMD_PROD
react-dom.profiling.min.js	0.0%	-0.0%	106.73 KB	106.73 KB	34.61 KB	34.61 KB	UMD_PROFILING
react-dom.development.js	+0.2%	+0.2%	805.17 KB	806.7 KB	183.49 KB	183.79 KB	NODE_DEV
react-dom.production.min.js	0.0%	-0.0%	103.58 KB	103.58 KB	33.1 KB	33.1 KB	NODE_PROD
react-dom.profiling.min.js	0.0%	-0.0%	106.9 KB	106.9 KB	33.96 KB	33.95 KB	NODE_PROFILING
ReactDOM-dev.js	+0.2%	+0.2%	829.52 KB	831.27 KB	184.87 KB	185.17 KB	FB_WWW_DEV
ReactDOM-prod.js	0.0%	-0.0%	337.24 KB	337.24 KB	62.68 KB	62.67 KB	FB_WWW_PROD
ReactDOM-profiling.js	0.0%	-0.0%	342.62 KB	342.62 KB	63.6 KB	63.6 KB	FB_WWW_PROFILING
react-dom-unstable-fire.development.js	+0.2%	+0.2%	811.02 KB	812.55 KB	185.16 KB	185.46 KB	UMD_DEV
react-dom-unstable-fire.production.min.js	0.0%	-0.0%	103.6 KB	103.6 KB	33.66 KB	33.66 KB	UMD_PROD
react-dom-unstable-fire.profiling.min.js	0.0%	-0.0%	106.75 KB	106.75 KB	34.62 KB	34.62 KB	UMD_PROFILING
react-dom-unstable-fire.development.js	+0.2%	+0.2%	805.5 KB	807.03 KB	183.62 KB	183.93 KB	NODE_DEV
react-dom-unstable-fire.production.min.js	0.0%	-0.0%	103.59 KB	103.59 KB	33.11 KB	33.11 KB	NODE_PROD
react-dom-unstable-fire.profiling.min.js	0.0%	-0.0%	106.91 KB	106.91 KB	33.96 KB	33.96 KB	NODE_PROFILING
ReactFire-dev.js	+0.2%	+0.2%	828.71 KB	830.46 KB	184.89 KB	185.19 KB	FB_WWW_DEV
ReactFire-profiling.js	0.0%	-0.0%	330.56 KB	330.56 KB	61.17 KB	61.17 KB	FB_WWW_PROFILING
react-dom-test-utils.development.js	0.0%	-0.0%	54.19 KB	54.19 KB	14.98 KB	14.98 KB	UMD_DEV
react-dom-test-utils.production.min.js	0.0%	-0.0%	10.52 KB	10.52 KB	3.9 KB	3.89 KB	UMD_PROD
react-dom-test-utils.production.min.js	0.0%	-0.0%	10.31 KB	10.31 KB	3.82 KB	3.82 KB	NODE_PROD
react-dom-unstable-native-dependencies.development.js	0.0%	-0.0%	60.76 KB	60.76 KB	15.85 KB	15.85 KB	UMD_DEV
react-dom-unstable-native-dependencies.production.min.js	0.0%	-0.1%	10.69 KB	10.69 KB	3.67 KB	3.67 KB	UMD_PROD
react-dom-unstable-native-dependencies.development.js	0.0%	-0.0%	60.43 KB	60.43 KB	15.72 KB	15.72 KB	NODE_DEV
react-dom-unstable-native-dependencies.production.min.js	0.0%	-0.1%	10.42 KB	10.42 KB	3.57 KB	3.56 KB	NODE_PROD
react-dom-server.browser.development.js	+0.4%	+0.5%	136.73 KB	137.23 KB	35.92 KB	36.12 KB	UMD_DEV
react-dom-server.browser.production.min.js	0.0%	-0.0%	19.18 KB	19.18 KB	7.21 KB	7.21 KB	UMD_PROD
react-dom-server.browser.development.js	+0.4%	+0.6%	132.86 KB	133.36 KB	34.99 KB	35.18 KB	NODE_DEV
react-dom-server.browser.production.min.js	0.0%	-0.0%	19.1 KB	19.1 KB	7.2 KB	7.2 KB	NODE_PROD
ReactDOMServer-dev.js	+0.4%	+0.6%	134.95 KB	135.54 KB	34.61 KB	34.81 KB	FB_WWW_DEV
react-dom-server.node.development.js	+0.4%	+0.5%	134.8 KB	135.31 KB	35.54 KB	35.73 KB	NODE_DEV
react-dom-server.node.production.min.js	0.0%	-0.0%	19.96 KB	19.96 KB	7.51 KB	7.51 KB	NODE_PROD
react-dom-unstable-fizz.browser.development.js	0.0%	-0.1%	3.66 KB	3.66 KB	1.45 KB	1.45 KB	UMD_DEV
react-dom-unstable-fizz.browser.production.min.js	0.0%	-0.1%	1.21 KB	1.21 KB	707 B	706 B	UMD_PROD
react-dom-unstable-fizz.browser.development.js	0.0%	-0.1%	3.49 KB	3.49 KB	1.41 KB	1.41 KB	NODE_DEV
react-dom-unstable-fizz.browser.production.min.js	0.0%	-0.2%	1.05 KB	1.05 KB	637 B	636 B	NODE_PROD
react-dom-unstable-fizz.node.development.js	0.0%	-0.1%	3.74 KB	3.74 KB	1.43 KB	1.43 KB	NODE_DEV
react-dom-unstable-fizz.node.production.min.js	0.0%	-0.1%	1.1 KB	1.1 KB	668 B	667 B	NODE_PROD

react-art

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
react-art.development.js	+0.3%	+0.3%	556.46 KB	557.99 KB	121.46 KB	121.77 KB	UMD_DEV
react-art.production.min.js	0.0%	-0.0%	95.49 KB	95.49 KB	29.31 KB	29.31 KB	UMD_PROD
react-art.development.js	+0.3%	+0.3%	487.49 KB	489.02 KB	104.07 KB	104.37 KB	NODE_DEV
react-art.production.min.js	0.0%	-0.0%	60.46 KB	60.46 KB	18.61 KB	18.61 KB	NODE_PROD
ReactART-dev.js	+0.4%	+0.3%	496.65 KB	498.4 KB	103.14 KB	103.45 KB	FB_WWW_DEV
ReactART-prod.js	0.0%	-0.0%	194.81 KB	194.81 KB	33.14 KB	33.14 KB	FB_WWW_PROD

react-native-renderer

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
ReactNativeRenderer-dev.js	+0.3%	+0.2%	628.2 KB	629.96 KB	133.33 KB	133.63 KB	RN_FB_DEV
ReactNativeRenderer-profiling.js	0.0%	-0.0%	251.77 KB	251.77 KB	43.91 KB	43.91 KB	RN_FB_PROFILING
ReactNativeRenderer-dev.js	+0.3%	+0.2%	628.12 KB	629.87 KB	133.29 KB	133.59 KB	RN_OSS_DEV
ReactFabric-dev.js	+0.3%	+0.2%	616.89 KB	618.64 KB	130.62 KB	130.92 KB	RN_FB_DEV
ReactFabric-dev.js	+0.3%	+0.2%	616.79 KB	618.55 KB	130.59 KB	130.88 KB	RN_OSS_DEV
ReactFabric-profiling.js	0.0%	0.0%	245.03 KB	245.03 KB	42.69 KB	42.69 KB	RN_OSS_PROFILING

react-test-renderer

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
react-test-renderer.development.js	+0.3%	+0.3%	500.9 KB	502.43 KB	106.78 KB	107.09 KB	UMD_DEV
react-test-renderer.production.min.js	0.0%	-0.0%	61.68 KB	61.68 KB	18.93 KB	18.93 KB	UMD_PROD
react-test-renderer.development.js	+0.3%	+0.3%	496.44 KB	497.97 KB	105.68 KB	105.98 KB	NODE_DEV
react-test-renderer.production.min.js	0.0%	-0.0%	61.36 KB	61.36 KB	18.8 KB	18.79 KB	NODE_PROD
ReactTestRenderer-dev.js	+0.3%	+0.3%	507.22 KB	508.97 KB	105.37 KB	105.67 KB	FB_WWW_DEV
react-test-renderer-shallow.development.js	0.0%	-0.0%	41.88 KB	41.88 KB	10.71 KB	10.7 KB	UMD_DEV
react-test-renderer-shallow.production.min.js	0.0%	-0.0%	11.63 KB	11.63 KB	3.55 KB	3.55 KB	UMD_PROD
react-test-renderer-shallow.development.js	0.0%	-0.0%	36.11 KB	36.11 KB	9.36 KB	9.36 KB	NODE_DEV
react-test-renderer-shallow.production.min.js	0.0%	-0.1%	11.81 KB	11.81 KB	3.67 KB	3.67 KB	NODE_PROD

react-reconciler

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
react-reconciler.development.js	+0.3%	+0.3%	485.02 KB	486.55 KB	102.5 KB	102.8 KB	NODE_DEV
react-reconciler.production.min.js	0.0%	-0.0%	61.39 KB	61.39 KB	18.35 KB	18.35 KB	NODE_PROD
react-reconciler-persistent.development.js	+0.3%	+0.3%	482.93 KB	484.46 KB	101.63 KB	101.92 KB	NODE_DEV
react-reconciler-reflection.production.min.js	0.0%	-0.1%	2.43 KB	2.43 KB	1.09 KB	1.09 KB	NODE_PROD

Generated by 🚫 dangerJS

bvaughn · 2019-04-17T15:04:24Z

Possible alternate wording. Don't know if it's great either, just sharing for discussion.

componentWillMount is deprecated and will be removed in the next major version.

We suggest doing one of the following to silence this warning:

Our recommended upgrade path is to replace "componentWillMount" with "componentDidMount". Example code is available at: https://fb.me/react-async-component-lifecycle-hooks

To silence the warning, rename "componentWillMount" to "UNSAFE_componentWillMount". Note that the warning will still be logged in strict mode. To automatically rename all unsafe lifecycle methods in your codebase, run npx react-codemod rename-unsafe-lifecycles <path/to/code> in your command line.

Please update the following components: SomeComponent, AnotherComponent

Learn more about this warning here:
https://fb.me/react-async-component-lifecycle-hooks

threepointone · 2019-04-18T12:33:46Z

Updated with suggestions, did the same for the server rendered warning which I'd missed, updated tests. I also updated the screenshot in the description. ^

threepointone · 2019-04-18T12:36:44Z

a followup 'todo' - make a warnings page we can point folks to, that's basically the blogpost, but reordered to emphasize solutions/suggestions.

threepointone · 2019-04-18T14:12:54Z

I forgot to add Note that the warning will still be logged in strict mode. which I actually like. Will update.

packages/react-dom/src/server/ReactPartialRenderer.js

…ll be logged in strict mode".

threepointone · 2019-04-18T16:46:02Z

updated PR and screenshot

bvaughn

Back with a few minor suggestions but overall 👍

It's collapsing my comment in partial renderer bcause it thinks it's resolved, but it's not 😁

packages/react-dom/src/server/ReactPartialRenderer.js

packages/react-reconciler/src/ReactStrictModeWarnings.js

bvaughn · 2019-04-18T17:02:33Z

packages/react-reconciler/src/ReactStrictModeWarnings.js

+          '- If you initialize state in componentWillMount, move this logic into the constructor.\n' +
+          '- If you fetch data or perform other side effects in componentWillMount, ' +
+          'move this logic into componentDidMount.\n' +
+          '- To rename all deprecated lifecycles to their new names, you can run ' +


Maybe we should give a hint as to why folks might not want to just rename and ignore? Maybe we could replace

"To rename all deprecated lifecycles to their new names, ..."

...with

"To prevent this warning from being logged to the console, ..."

...so it sounds a little more like you're just choosing to ignore the warning without fixing the problem?

(here and below)

How about "To ignore this warning from being logged to the console..."

rather, "to disable this warning..."

It's so wordy :(

I think that we're saying the warning will still log in strict mode, and that we've moved this to the bottom of the list, is sufficient. I'll fight you on this briannson.

packages/react-reconciler/src/ReactStrictModeWarnings.js

bvaughn · 2019-04-18T17:06:42Z

packages/react-reconciler/src/ReactStrictModeWarnings.js

+          'We suggest doing one of the following:\n' +
+          '- If you fetch data or perform other side effects in componentWillUpdate, ' +
+          'move this logic into componentDidUpdate.\n' +
+          "- If you're reading DOM properties before an update, " +


Legit use cases for getSnapshotBeforeUpdate seem so uncommon, I wonder if it's worth mentioning here at all? I don't have a strong preference one way or another, but a slight preference to maybe not mention it here for fear of encouraging bad patterns for those maybe lacking context of when the method should be used.

I think I'll leave it here. We seem fairly comprehensive for the others, it would be frustrating if the dev had to go to a doc to discover a new api for just this one case.

I guess I'm concerned that if people just move stuff to that method, without the added context of why it exists, they might be tempted to do mutations/side effects in it.

They would probably try componentDidUpdate before that though? I dunno tbh. I could remove this, but seems like we should use same reasoning to remove getDerivedStateFromProps if so.

threepointone · 2019-04-18T19:15:40Z

updated PR and screenshot

sophiebits · 2019-04-18T20:41:30Z

I would not try to explain when to use which lifecycle method in the warning text. I think it is just too small a space and we should encourage people to read the blog posts instead. It's better to make the warning easier to grok quickly and then have a place for people to read when they actually want to get into it. (Most people will need to read more to be sure in their changes anyway.)

My suggestion would be to write something along the following lines:

In non-strict mode:

Use of componentWillMount is not recommended and may indicate bugs in your code.

* Rename componentWillMount in your code to UNSAFE_componentWillMount to suppress this warning. 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`.
* Move component logic from componentWillMount into componentDidMount (preferred in most cases) or the constructor. See https://fb.me/react-async-component-lifecycle-hooks for details.

Please update the following components: %s

In strict mode if componentWillMount is used:

Use of componentWillMount and UNSAFE_componentWillMount is not recommended and may indicate bugs in your code.

* Rename componentWillMount in your code to UNSAFE_componentWillMount. 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`.
* Move component logic from componentWillMount into componentDidMount (preferred in most cases) or the constructor. See https://fb.me/react-async-component-lifecycle-hooks for details.

Please update the following components: %s

In strict mode if componentWillMount is not used:

Use of UNSAFE_componentWillMount is not recommended and may indicate bugs in your code.

Move component logic from UNSAFE_componentWillMount into componentDidMount (preferred in most cases) or the constructor. See https://fb.me/react-async-component-lifecycle-hooks for details.

Please update the following components: %s

I think it's also important there is a place to learn more about the auto-rename script. People are gonna be a little hesitant unless they know what it'll do. (Which files will it run on? etc)

threepointone · 2019-04-29T12:03:31Z

Sorry I got a while to get back to this. Some thoughts -

I would not try to explain when to use which lifecycle method in the warning text.

Trying to reconcile this with the suggestion, it's only one sentence shorter than what the PR currently has. I think a quick reference of alternatives is indeed valuable, and don't want folks to have to read a long post for every warning (and the link is pretty obvious, so they will go there if they don't comprehend immediately what to do.

I agree we should have different warning with strict/non strict mode, hacking on it asap.

RyanLamansky · 2019-05-22T15:40:36Z

This messaging doesn't harmonize well with the current state of the server renderer, which works with componentWillMount but not componentDidMount.

- redoes facebook#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

threepointone · 2019-07-11T09:13:00Z

abandoning this for #16103, which uses all the feedback from here, and then some. see y'all there!

- redoes facebook#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

- 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

Sunil Pai added 8 commits April 3, 2019 17:11

mark react-events as private so we publish script skips it for now

c35e37a

Merge remote-tracking branch 'upstream/master'

dc605c1

Merge remote-tracking branch 'upstream/master'

a09bd0f

Merge remote-tracking branch 'upstream/master'

48ffcee

Merge remote-tracking branch 'upstream/master'

7cc1379

Merge remote-tracking branch 'upstream/master'

e86d051

tweak unsafe lifecycle warnings to include codemod suggestion

e3b06c6

stray comment

d884ec3

threepointone requested review from bvaughn and gaearon April 17, 2019 12:15

facebook-github-bot added the CLA Signed label Apr 17, 2019

more tweaks, update more tests

ea444d2

threepointone requested a review from sophiebits April 18, 2019 13:26

bvaughn reviewed Apr 18, 2019

View reviewed changes

packages/react-dom/src/server/ReactPartialRenderer.js Outdated Show resolved Hide resolved

move the codemod line to the end, add "Note that the warning will sti…

0523909

…ll be logged in strict mode".

bvaughn approved these changes Apr 18, 2019

View reviewed changes

review stuff

1ba1642

ziishaned approved these changes Apr 18, 2019

View reviewed changes

threepointone mentioned this pull request Jul 10, 2019

unify deprecated/unsafe lifecycle warnings, pass tests #16103

Merged

threepointone closed this Jul 11, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tweak unsafe lifecycle warnings #15431

tweak unsafe lifecycle warnings #15431

threepointone commented Apr 17, 2019 •

edited

Loading

sizebot commented Apr 17, 2019 •

edited

Loading

react-dom

react-art

react-native-renderer

react-test-renderer

react-reconciler

bvaughn commented Apr 17, 2019

threepointone commented Apr 18, 2019

threepointone commented Apr 18, 2019

threepointone commented Apr 18, 2019

threepointone commented Apr 18, 2019

bvaughn left a comment •

edited

Loading

bvaughn Apr 18, 2019

threepointone Apr 18, 2019

threepointone Apr 18, 2019

threepointone Apr 18, 2019

threepointone Apr 18, 2019

bvaughn Apr 18, 2019

threepointone Apr 18, 2019

bvaughn Apr 18, 2019

threepointone Apr 18, 2019

threepointone commented Apr 18, 2019

sophiebits commented Apr 18, 2019 •

edited

Loading

threepointone commented Apr 29, 2019

RyanLamansky commented May 22, 2019

threepointone commented Jul 11, 2019

tweak unsafe lifecycle warnings #15431

tweak unsafe lifecycle warnings #15431

Conversation

threepointone commented Apr 17, 2019 • edited Loading

sizebot commented Apr 17, 2019 • edited Loading

react-dom

react-art

react-native-renderer

react-test-renderer

react-reconciler

bvaughn commented Apr 17, 2019

threepointone commented Apr 18, 2019

threepointone commented Apr 18, 2019

threepointone commented Apr 18, 2019

threepointone commented Apr 18, 2019

bvaughn left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

threepointone commented Apr 18, 2019

sophiebits commented Apr 18, 2019 • edited Loading

threepointone commented Apr 29, 2019

RyanLamansky commented May 22, 2019

threepointone commented Jul 11, 2019

threepointone commented Apr 17, 2019 •

edited

Loading

sizebot commented Apr 17, 2019 •

edited

Loading

bvaughn left a comment •

edited

Loading

sophiebits commented Apr 18, 2019 •

edited

Loading