React lifecycle method deprecations

[luisnaranjo733]

3 react lifecycle methods are being gradually deprecated because they cause confusion and get misused often

componentWillMount componentWillReceiveProps componentWillUpdate There are 2 new lifecycle methods intended to "replace" these methods going forward

getDerivedStateFromProps getSnapshotBeforeUpdate Here is the migration path set out by the react team

react@16.3: Add an "UNSAFE_" alias for each of these 3 lifecycle methods (both the original method and the alias will work) react@16.6+: We will start to get a deprecation warning if we use one of the deprecated methods listed above react@17: Original deprecated methods will stop working. The UNSAFE alias will continue to work for the foreseeable future, but we should not use them in new code and make an effort to gradually replace them. More info on why this happened and what it means in this React blog post.

Here is what I think ReactXP should do

• Short term: Find and replace the deprecated methods with the "UNSAFE_" prefixed methods. This is safe for react 16.3 and later. This will allow XP users to not get a deprecation warning from XP, and will ensure longer term support.
• Long term: Replace the UNSAFE_ methods with the new ones as proposed in issue #605

[erictraut]

There has already been some discussion of this in issue #605.

Never mind, I see you linked to this issue.

[luisnaranjo733]

Yes, the reason I opened a new issue is that the previous discussion proposed going straight from the deprecated methods to the new cool thing (skipping the back compat layer that react provided).

I believe that the UNSAFE_* methods can be a good way to meet in the middle and support as many versions of react as possible.

[erictraut]

Yeah, this sounds like a good approach.

[fbartho]

Now that we're working on ReactXP 2.0 with some breaking changes re: React-Native extracting some dependencies, maybe we should try to prioritize this work?

Also, related to upgrading, react-native 60 support is going to be rough, maybe that should roll in with this work too? -- Ticket: #1123

Thoughts?

[erictraut]

That's a reasonable point. @deregtd & @berickson1 - any strong opinions on this?

[deregtd]

I think this is the right approach. We still need to figure out what to do about ReSub, though. Moving to the deprecated names only staves off the inevitable problem over there.

[fbartho]

We cut ReSub from our repository -- Is it used somewhere in ReactXP Core?

[berickson1]

I'm pro going through the migration. I don't think reactxp relies heavily on any of the deprecated lifecycle methods

[berickson1]

ReSub isn't part of the reactxp core - it's used by a bunch of the examples and internal projects that use reactxp. It's been on our list of things to address in the near future but orthogonal to this specific issue

[fbartho]

I did a quick analysis some months ago, and it all looked doable, just a bit of a chore: https://github.com/microsoft/reactxp/issues/605#issue-316964212

[erictraut]

The reason we didn't do the migration previously is because we wanted to retain backward compatibility with versions of react that didn't support the new replacement lifecycle methods. ReactXP 2.0 is introducing an incompatibility with older versions of RN, so it's reasonable that we will sever compatibility with old versions of react as well.

[mikehardy]

Only chiming in to say I use ReSub in my project as well (very useful!), so as far as I can provide confirmation things work with RCs etc, I will be fairly regularly pulling whatever you guys generate, integrating it, and reporting whatever. Or you can ping me if you want confirmation, happy to help

[fbartho]

Relatedly, React 16.9 which hit stable release today moves these deprecated methods to UNSAFE_ (Specifically: componentWillMount componentWillReceiveProps componentWillUpdate)

https://reactjs.org/blog/2019/08/08/react-v16.9.0.html

[mikehardy]

@fbartho - I'm not sure your previous comment is accurate. My read on the migration path was that react 16.3 added the deprecation and the UNSAFE_ method variants but 16.9 still has the deprecated methods in place, so no move happened with 16.9. The change in 16.9 was (I think) just to make the warning much more severe?

Separately, it is worth noting that while react v17.x will drop the deprecated names, it will support the UNSAFE_ ones, which implies there is a long time before the "full" migration to different lifecycle APIs is necessary

Using that in harmony with the reasoning above that seemed to have consensus at the time, I've used the recommended codemod tool from React to make PRs to both reactxp here and ReSub so that at the least the severe warnings with react v16.9 are avoided, and so that these repos will continue to function with react v17

[fbartho]

@mikehardy -- you're right, I was a bit overexcited in my reading of the release announcement. A more careful reading shows you're right.

That said -- I don't think we're in a pressing urgency-type situation. -- This is a necessary migration that will have to happen someday, and there were discussions about releasing ReactXP 2.0 -- for me, that looks like a great opportunity to clean up these things, rather than kicking the can down the road until React 18 & ReactXP 3.0.

When UNSAFE_-methods are present, I believe some stuff gets less efficient in React so we might want to do this for performance anyhow. _EDIT: the specific thread I was thinking about from last week was about Legacy Context thread_

I trust that the official maintainers are more in tune with what needs to be done now vs later than I am, however!

[mikehardy]

@fbartho Very interesting about possible global de-optimizations (listed as "bailouts on referentially equal values during re-render" in the thread), but that appears to be a completely different set of APIs? context-related, not the UNSAFE ones - or is it known that the UNSAFE APIs are all depending on the legacy context API mentioned in the thread you link, such that they trigger the de-optimization? You should probably log that as a separate issue for tracking, and here's the list of current uses:

(virtualenv) mike@kunashir:~/work/react-random/reactxp (unsafe-react-names) % grep -ri contexttypes *|grep -v dist | grep -v node_modules
src/web/Link.tsx:    static contextTypes = {
src/web/GestureView.tsx:    static contextTypes: React.ValidationMap<any> = {
src/web/TextInput.tsx:    static contextTypes: React.ValidationMap<any> = {
src/web/RootView.tsx:    static childContextTypes: React.ValidationMap<any> = {
src/web/RootView.tsx:    static childContextTypes: React.ValidationMap<any> = {
src/web/View.tsx:    static contextTypes: React.ValidationMap<any> = {
src/web/View.tsx:    static childContextTypes: React.ValidationMap<any> = {
src/web/Button.tsx:    static contextTypes = {
src/web/Button.tsx:    static childContextTypes = {
src/web/Image.tsx:    static contextTypes: React.ValidationMap<any> = {
src/web/Image.tsx:    static childContextTypes: React.ValidationMap<any> = {
src/web/Text.tsx:    static contextTypes = {
src/web/Text.tsx:    static childContextTypes: React.ValidationMap<any> = {
src/native-desktop/RootView.tsx:        static childContextTypes: React.ValidationMap<any> = {
src/native-common/Link.tsx:    static contextTypes = {
src/native-common/TextInput.tsx:    static contextTypes: React.ValidationMap<any> = {
src/native-common/View.tsx:    static contextTypes: React.ValidationMap<any> = {
src/native-common/View.tsx:    static childContextTypes: React.ValidationMap<any> = {
src/native-common/Button.tsx:    static contextTypes = {
src/native-common/Button.tsx:    static childContextTypes: React.ValidationMap<any> = {
src/native-common/Image.tsx:    static childContextTypes: React.ValidationMap<any> = {
src/native-common/Text.tsx:    static contextTypes: React.ValidationMap<any> = {
src/native-common/Text.tsx:    static childContextTypes: React.ValidationMap<any> = {
src/common/PopupContainerViewBase.tsx:    static contextTypes: React.ValidationMap<any> = {
src/common/PopupContainerViewBase.tsx:    static childContextTypes: React.ValidationMap<any> = {
src/common/utils/FocusManager.ts:    const contextTypes = Component.contextTypes || {};
src/common/utils/FocusManager.ts:    contextTypes.focusManager = PropTypes.object;
src/common/utils/FocusManager.ts:    Component.contextTypes = contextTypes;
src/windows/View.tsx:    static contextTypes: React.ValidationMap<any> = {
src/windows/View.tsx:        ...ViewCommon.contextTypes
src/windows/View.tsx:    static childContextTypes: React.ValidationMap<any> = {
src/windows/View.tsx:        ...ViewCommon.childContextTypes
src/windows/Button.tsx:    static childContextTypes: React.ValidationMap<any> = {
src/windows/Button.tsx:        ...ButtonBase.childContextTypes
src/windows/Text.tsx:    static contextTypes: React.ValidationMap<any> = {
src/windows/Text.tsx:        ...TextBase.contextTypes
src/windows/Text.tsx:    static childContextTypes: React.ValidationMap<any> = {
src/windows/Text.tsx:        ...TextBase.childContextTypes
(virtualenv) mike@kunashir:~/work/react-random/reactxp (unsafe-react-names) % grep -ri getchildcontext *|grep -v dist | grep -v node_modules
src/web/RootView.tsx:    getChildContext(): MainViewContext {
src/web/RootView.tsx:    getChildContext() {
src/web/View.tsx:    getChildContext() {
src/web/Button.tsx:    getChildContext(): ButtonContext {
src/web/Image.tsx:    getChildContext() {
src/web/Text.tsx:    getChildContext() {
src/native-desktop/RootView.tsx:        getChildContext() {
src/native-common/View.tsx:    getChildContext() {
src/native-common/Button.tsx:    getChildContext(): ButtonContext {
src/native-common/Image.tsx:    getChildContext() {
src/native-common/Text.tsx:    getChildContext() {
src/common/PopupContainerViewBase.tsx:    getChildContext() {
src/windows/View.tsx:    getChildContext() {
src/windows/View.tsx:        const childContext: ViewContext = super.getChildContext();
src/windows/Button.tsx:    getChildContext(): ButtonContext {
src/windows/Button.tsx:        const childContext: ButtonContext = super.getChildContext();
src/windows/Text.tsx:    getChildContext(): TextContext {
src/windows/Text.tsx:        const childContext: TextContext = super.getChildContext();

Agreed this specific change is not pressing (vs the Accessibility API change and other stuff) for ReactXP 2, but this is a well-maintained library so I also respect + defer to general consensus whichever way it goes

[fbartho]

No, you're right, I mis-linked those two issues. I couldn't find a reference for my memory about deoptimized paths due to componentWill* lifecycles -- other than that it's too easy to write bugs using those apis (which is what lead to them deprecating & deleting them!)

I'll file a ticket with the contextTypes data you shared. That's important too!

[fbartho]

I'm motivated to get this feature soon, please let me know if anybody wants to help!