Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enabled warnAboutDeprecatedLifecycles flag by default #15186

Merged
merged 1 commit into from Mar 27, 2019

Conversation

@bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Mar 21, 2019

Also updated tests to account for this. (Mostly this involved merging some "internal" tests that are no longer internal.)

Note that we still only warn about "unsafe" lifecycle methods in strict mode, but now we will always warn about the "legacy" lifecycles.

@bvaughn bvaughn force-pushed the bvaughn:warnAboutDeprecatedLifecycles branch from 862b335 to 8742dce Mar 21, 2019
@bvaughn bvaughn requested a review from sebmarkbage Mar 27, 2019
@bvaughn
Copy link
Contributor Author

@bvaughn bvaughn commented Mar 27, 2019

Ping!

Copy link
Member

@sebmarkbage sebmarkbage left a comment

looksgooddidnotread

@bvaughn
Copy link
Contributor Author

@bvaughn bvaughn commented Mar 27, 2019

rightbackatyabuddy

@bvaughn bvaughn merged commit d8cb10f into facebook:master Mar 27, 2019
1 check passed
1 check passed
ci/circleci Your tests passed on CircleCI!
Details
@bvaughn bvaughn deleted the bvaughn:warnAboutDeprecatedLifecycles branch Mar 27, 2019
).toLowPriorityWarnDev(
[
'componentWillMount is deprecated and will be removed in the next major version. ' +
'Use componentDidMount instead. As a temporary workaround, ' +

This comment has been minimized.

@gaearon

gaearon Mar 28, 2019
Member

"As a temporary workaround" — people will 100% read it as "UNSAFE_ version will be removed in 17". We've already seen this confusion despite explicit assurances it won't be. We need to emphasize this in the message.

This comment has been minimized.

@bvaughn

bvaughn Mar 28, 2019
Author Contributor

The workaround is still temporary, in that using the (renamed) method is still unsafe.

If you have proposals for an alternate error message, post a PR, but I don't think we should make it sound like ongoing use of the unsafe lifecycles is totally fine either.

This comment has been minimized.

@sebmarkbage

sebmarkbage Mar 28, 2019
Member

I'm not sure it's better to error on the side of less scary.

It is really unsafe even with "batched" Suspense too, and just generally a risk factor in server rendering. Not just concurrent mode. So you'll likely be left behind one way or another.

So even though we probably won't completely remove it, it's probably a good idea to treat it almost as bad as if we would.

Does it matter if you can upgrade to React 18 but not use any of the new features in it?

This was referenced Mar 10, 2020
@xoob xoob mentioned this pull request May 12, 2020
0 of 17 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.