Skip to content
This repository has been archived by the owner on Feb 14, 2023. It is now read-only.

Console warning when adding reactn to class #134

Closed
umbertoghio opened this issue Nov 22, 2019 · 20 comments
Closed

Console warning when adding reactn to class #134

umbertoghio opened this issue Nov 22, 2019 · 20 comments
Labels
help wanted The owner cannot resolve this on their own.

Comments

@umbertoghio
Copy link
Contributor

Hello,
I notice some warnings when adding reactn to a class:

`componentWillUpdate has been renamed, and is not recommended for use. See https://fb.me/react-async-component-lifecycle-hooks for details.

  • Move data fetching code or side effects to componentDidUpdate.
  • Rename componentWillUpdate to UNSAFE_componentWillUpdate to suppress this warning in non-strict mode. 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 in your project source folder.

Please update the following components: ##ComponentName##`

How to reproduce:

Create a new project with create-react-app
Create a component
`import React, { Component } from 'react'

export default class Compo extends Component {
render() {
return (


I am a Component

)
}
}
`
and include it in App.js

Run -> No warnings
change first line to
`import React, { Component } from 'reactn'
get warning:

`react-dom.development.js:12449 Warning: componentWillUpdate has been renamed, and is not recommended for use. See https://fb.me/react-unsafe-component-lifecycles for details.

  • Move data fetching code or side effects to componentDidUpdate.
  • Rename componentWillUpdate to UNSAFE_componentWillUpdate to suppress this warning in non-strict mode. 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 in your project source folder.

Please update the following components: Compo`

@quisido
Copy link
Collaborator

quisido commented Nov 24, 2019

This is just a deprecation warning, and ReactN should function fine despite it. That said, I would like to get this log out of your developer environment where possible.

Code trail:
When a ReactN class component is instantiated, it binds necessary lifecycle methods for managing global state.
One of those lifecycle methods is componentWillUpdate.
componentWillUpdate removes the global subscription listeners.

The intention of this method is to remove subscriptions that will no longer exist on the next render cycle. For example, if I am subscribed to changes in property x the first render, but I do not use property x in the second render, I no longer need to be subscribed to it.

I do not believe there is an alternative to using componentWillUpdate for this, because there are no other lifecycle hooks that trigger on the component instance prior to render.

The best course of action may be to do (pseudo code):

  • If UNSAFE_componentWillUpdate exists on this class's prototype, use it.
  • Otherwise, use componentWillUpdate.

I am unsure if UNSAFE_* exists on the prototype, and if not, there needs to be another way to determine which method to use.

@umbertoghio
Copy link
Contributor Author

Hi,
adding UNSAFE would clear up console :-)
If you are just removing the subscription and not doing any other effects (like cancelling the re-render), would it be an option to place the code in the getSnapshotBeforeUpdate() ?
And if render order does not matter what about componentDidUpdate() method?

@quisido
Copy link
Collaborator

quisido commented Nov 25, 2019

componentDidUpdate fires after render, while this method needs to fire pre-render.

  • step 1: remove existing subscriptions
  • step 2: render, which creates new subscriptions

If removing the subscriptions occurred post-render, then there would be no subscriptions to state change.

I am not sure if getSnapshotBeforeUpdate would work or not. The change needs to be backwards compatible, and I think the only way of doing that is to check "if [the method] exists on the prototype" and hope that the method is on the prototype!

@umbertoghio
Copy link
Contributor Author

You are right 😁, what about other methods that fire before render?
getDerivedStateFromProps()
shouldComponentUpdate()
the latter one seems the most backward compatible

@MoKhajavi75
Copy link

You are right , what about other methods that fire before render?
getDerivedStateFromProps()
shouldComponentUpdate()
the latter one seems the most backward compatible

Hey

take a look at this

@quisido quisido added the help wanted The owner cannot resolve this on their own. label Nov 30, 2019
@tpjnorton
Copy link

For now, is it not worth using the codemod the react guys wrote to patch these methods? they'll still be there in React 17, and the warnings are pretty annoying, especially on React Native.

Can codemod with:

npx react-codemod rename-unsafe-lifecycles

@umbertoghio
Copy link
Contributor Author

Older version of react that are still supported by reactn may not have the UNSAFE_* method...

Will try this weekend to fork it and thinker with lifecycle method and give some feedback

For now, is it not worth using the codemod the react guys wrote to patch these methods? they'll still be there in React 17, and the warnings are pretty annoying, especially on React Native.

Can codemod with:

npx react-codemod rename-unsafe-lifecycles

@siapdev
Copy link
Contributor

siapdev commented Jan 9, 2020

Hi @CharlesStover, I have attempted the pseudocode you mentioned and it works!
Made a PR that satisfies all tests, hope that helps!
Umberto Ghio

@umbertoghio
Copy link
Contributor Author

Thank you for accepting the PR 👍 , I made another one by using shouldComponentUpdate instead of UNSAFE_componentWillUpdate if using newer react to avoid deprecated methods.
Test passes and made a test build repo, you can try it out with
yarn add umbertoghio/reactn-testshouldupdate
I've tested in a fairly sized project and seems to work just fine 😁

@quisido
Copy link
Collaborator

quisido commented Jan 10, 2020

Thank you all very much for your contributions. :)

@quisido quisido closed this as completed Jan 10, 2020
@jasonbodily
Copy link

I'm sorry I know this is closed, but I'm not sure I understand the solution? I have the latest version of react-native (0.61.5) and the latest version of reactn (2.2.5), and I have absolutely nowhere in my code that I'm using deprecated methods like componentWillUpdate (and I tried npx react-codemod rename-unsafe-lifecycles). My logs keep getting inundated with this warning to the point that it's difficult to debug other problems. Was there a consensus on how to suppress this?

@umbertoghio
Copy link
Contributor Author

@jasonbodily hi, seems that the current release on npm does not include yet this fix.

You may temporarily clone from this master or temporarily use version “SIAPCN/reactnUnsafe” in your package.json instead of 2.2.5, it contains the workaround that suppressed the warnings

@jasonbodily
Copy link

Thank you!

@quisido
Copy link
Collaborator

quisido commented Jan 31, 2020

@umbertoghio Unless Travis botched deployment, NPM should match the master branch at this time at v2.2.5.

@umbertoghio
Copy link
Contributor Author

Indeed, I confirm the last changes are in 2.2.5
I'm going ahead and archive SIAPCN/reactnUnsafe
Thanks!!

@quisido
Copy link
Collaborator

quisido commented Jan 31, 2020

Are you aware of why your fix in 2.2.5 wouldn't address Jason's issue?

@umbertoghio
Copy link
Contributor Author

umbertoghio commented Jan 31, 2020

Hi, I just verified my work project (was still using forked) and with 2.2.5 all componentWillUpdate warnings that used to get on older versions are gone 😄 (both on React Web and React Native).

@jasonbodily
Have you tried deleting yarn.lock (or nom shrink-wrap), the node_modules folder and reinstall your dependencies?

Are you sure the warnings are coming from your reactn components and not from third party components?

As in my work project is using an older Expo I setup a new react-native setup, tried 2.2.5 and I got no warnings (See screenshots)....so I can't really explain why my fix are still causing the warnings.

image

Edit: In the screenshot I was in debug, imported reactn instead of React, Compo is a regular class component. I got a warning to validate setup when adding componentWillUpdate method.

@jasonbodily
Copy link

jasonbodily commented Feb 1, 2020

Those pesky logs are gone! Clearing and rebuilding did it, so thank you! And many thanks for going out of your way to address my concerns!

@TuckerMassad
Copy link

TuckerMassad commented Feb 19, 2020

Hi, I recently upgraded my react-native to 0.61.5 and reactn to 2.2.6 in a project of mine and I seem to be getting the same Warning: componentWillUpdate has been renamed, and is not recommended for use. console warnings as described above. Was just wondering if this issue is still being addressed, or if I am missing a step in the upgrading process that would cause these errors to persist? Not a serious problem as it doesn't cause anything to break, but all components in my application are built using ReactN so I get the Please update the following components: x,y,z for each component thats rendered on the current screen. Thanks a bunch in advance!

@quisido
Copy link
Collaborator

quisido commented Feb 19, 2020

Hi @TuckerMassad . This issue was fixed to the best of our ability in 2.2.5. If you can try yarn add [email protected] to make sure that it did not regress in 2.2.6, that may be helpful in tracking down this issue.

If 2.2.5 gives you the same warnings, could you follow the directions in this comment to see if that resolves your issues as it did for Jason.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
help wanted The owner cannot resolve this on their own.
Projects
None yet
Development

No branches or pull requests

7 participants