react-router-v3: Add UNSAFE_ prefixes to deprecated lifecycle methods #6883
Conversation
…ing on React version Bump React version
|
Any thoughts on this approach vs reduxjs/react-redux#1383? |
Don't feel super strongly here, but my thoughts around this:
thoughts? happy to make changes to align the two approaches 👍 |
I would prefer if we could do feature detection on this, but I agree version checking is the next best option.
Yeah, it makes it easier to see where it happens. Also, I think it might transpile to smaller code. |
|
Yeah, fundamentally they end up with the same behaviour and the differences are minor. I'm feature checking, but (as @bishwei correctly points out) I'm checking for a feature that happened to come in the same release not the actual feature that really matters in this case (support for Inline vs later prototype modification I don't mind either way; inline makes it clear that that function name might be modified later when reading the initial definition, if that's important to know for some reason, prototype modification groups all of the modifications in one place and makes a infinitesimally smaller number of comparisons :D If there's a significant difference in transpilation size, maybe that should be the guide - try it and see? |
|
@timdorr, changes made to consolidate the renaming of the methods! |
|
LGTM; if you're happy with this approach @timdorr then I'll update reduxjs/react-redux#1383 to be consistent in implementation and hopefully we can get that merged too. |
modules/ContextUtils.js
Outdated
| @@ -15,15 +15,15 @@ function makeContextName(name) { | |||
| return `@@contextSubscriber/${name}` | |||
| } | |||
|
|
|||
| const prefixUnsafeLicycleMethods = parseFloat(React.version) >= 16.3 | |||
| const prefixUnsafeLifeycleMethods = parseFloat(React.version) >= 16.3 | |||
There was a problem hiding this comment.
Forgot a c. Lifeycle -> Lifecycle
|
I'll work on getting this out shortly. |
|
Is there a rough plan for when this will be released? |
|
It was released in 3.2.4: https://github.com/ReactTraining/react-router/releases/tag/v3.2.4 |
|
Ah. I have misread this thread, sorry. I seem to be getting this issue on v5.0.1, but it must be due to another package. |
|
Yes, 5.0 doesn't have this problem. |
| @@ -14,13 +15,15 @@ function makeContextName(name) { | |||
| return `@@contextSubscriber/${name}` | |||
| } | |||
|
|
|||
| const prefixUnsafeLifecycleMethods = parseFloat(React.version) >= 16.3 | |||
There was a problem hiding this comment.
@timdorr as I mentioned over in reduxjs/react-redux#1383 (comment) , this parses the latest version of React (16.10.1) as 16.1, so this is broken.
React 16.9 starts to warn whenever
componentWillMount,componentWillReceiveProps, orcomponentWillUpdateare used without theUNSAFE_prefixes.This PR aims to address this by dynamically appending the
UNSAFE_prefixes based on React version. The main motivation for doing this is so v3 does not break compatibility with consumers who are using React < 16.3 that don't support the prefixed lifecycled methods.Note: I only updated the lifecycle methods for the exported modules, not the ones in the examples.