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

React 16.9 - Warning: componentWillMount has been renamed, and is not recommended for use. #1835

Closed
SCasarotto opened this issue Aug 9, 2019 · 22 comments · Fixed by #1839
Closed

Comments

@SCasarotto
Copy link

SCasarotto commented Aug 9, 2019

Do you want to request a feature or report a bug?

Reporting a bug. This should simply require a small code update if we want to leave the unsafe lifecycle methods. That something I can make a PR for but I didn't know if this would be good time to move away from these lifecycle methods all together.

What is the current behavior?

image

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: http://jsfiddle.net/ndLhnegs/).

Create React Project with React@16.9 or greater and then install recharts and attempt to render a chart.

What is the expected behavior?

No warnings.

Which versions of Recharts, and which browser / OS are affected by this issue? Did this work in previous versions of Recharts?

Recharts@1.7.0

@avindra
Copy link
Contributor

avindra commented Aug 12, 2019

componentWillMount and componentWillReceiveProps will be removed in future React.

The maintainer is looking for contribs #1833

I'm curious if React 16 will be the supported target. If so, then we can remove /replace all the deprecated lifecycle methods.

@avindra
Copy link
Contributor

avindra commented Aug 12, 2019

https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html#gradual-migration-path

React 16.3 introduces the UNSAFE_ prefix, and 17 will remove it. So, it seems like minimum required React for all consumers changing to 16.3 may be required to move forward. BC break, maybe major version bump it too.

@semoal
Copy link
Contributor

semoal commented Aug 12, 2019

Hi guys, I'm glad to help with this "small" code update.
We have: 14 componentWillReceiveProps, and just one componentWillMount.

componentWillMount could be easily fixed with a constructor initialization or possibly with componentDidMount(not quite sure about this)

componentWillReceiveProps, it's a little more complicated, we should check if they perform a side effect, in that case use componentDidUpdate, if not use memoization.

  • Area
  • Bar
  • Brush
  • Line
  • Scatter
  • generateCategorialChart
  • Sankey
  • Treemap
  • Text
  • Funnel
  • Pie
  • Radar
  • RadialBar
  • AnimationDecorator

@damianobarbati
Copy link

@semoal actually the ideal solution in the long term is not to move to componentDidUpdate but to move to hooks and effects.

That's a clear path and future-proof approach, but I well understand that brings a lot of codebase rewrite.

@paramsinghvc
Copy link

Till the time you aren't moving recharts to hooks, at least, these unsafe methods need to be refactored, they show long warnings in the console.

@cmpaul
Copy link

cmpaul commented Oct 4, 2019

I agree with @paramsinghvc , maybe do an interim fix and plan for a larger refactor in the coming major release?

@ro-savage
Copy link

@damianobarbati - Hooks aren't the only way to do things. There are still things that require lifecycle methods, and there is no current plans to depreciate the current lifecycle methods. They are used extensively within the Facebook codebase.

Updating some/all the components to hooks may help simplify some of the components, but isn't need at all for React 17 compatibility.

The main issue here is @xile611 is asking for maintainers (which is fine) but hasn't appointed anyone yet and isn't around to approve PRs.

@timneyens
Copy link

timneyens commented Oct 15, 2019

Thanks for merging @xile611. Sadly I am still getting an error on one of the dependencies of recharts. It seems the Animate component from react-smooth still has a componentWillReceiveProps. There is a PR on the react-smooth library from recharts: recharts/react-smooth#29
Can this be merged and updated as well?

image

@Babettestam
Copy link

Also still having the warning after updating to the latest version (1.8.3). Seems to be when using the ComposedChart.

Screenshot 2019-10-17 at 17 25 28

@Chiiip
Copy link

Chiiip commented Oct 22, 2019

I am facing the same issue, with recharts@1.8.5.

image

@mwskwong
Copy link

mwskwong commented Oct 23, 2019

I am facing the same issue, with recharts@1.8.5.

image

The maintainer has had tried to migrate the deprecated lifecycles to others like componentDidUpdate in version 1.8.0 - 1.8.2. Yet this causes lots of unexpected behaviours since the lifecycles have different behaviours (e.g. even though it is recommended to migrate fromcomponentWillUpdate to componentDidUpdate, they behave very differently).

The changes have ultimately reverted in a later release and a full conversion is planned but will take quite an amount of time.

@Chiiip
Copy link

Chiiip commented Oct 23, 2019

Considering that changing all of these lifecycle methods it's not that easy/quick, i would suggest, for now, the addition of the UNSAFE_ prefix to every method call. I believe it's a simpler thing to do.

That way, we get rid of the warnings with no major changes in the codebase and when React 17 is released, at least the deprecation of these methods won't be a breaking change, even though they would need to be refactored later.

UPDATE: I just noticed that #1905 already applied this fix and has been merged.

@mguida22
Copy link

It looks like this was fixed in #1905 then rolled back due to a problem supporting react@15 with the UNSAFE_ prefix (8ae2390).

Is there any path forward to resolve these warnings in react@16 that is still compatible with react@15 @xile611?

I'd be happy to contribute a PR with a bit of guidance on how to best resolve this.

@amirburbea
Copy link

amirburbea commented Oct 29, 2019 via email

@josephmaxim
Copy link

Still getting the warning when using ComposedChart, Line components.

@mwskwong
Copy link

It looks like this was fixed in #1905 then rolled back due to a problem supporting react@15 with the UNSAFE_ prefix (8ae2390).

@josephmaxim Of course you do, did you even read the posts above?

@axhamre
Copy link

axhamre commented Nov 4, 2019

Perhaps reopen the issues as it's resurfaced?

@damianobarbati
Copy link

Yes, seeing this again

@mikefrancis
Copy link

It's a bit hacky and I have no idea how safe this is but this seems to be fixed in the v2 beta:

yarn add recharts@v2.0.0-beta.1

or

npm install recharts@v2.0.0-beta.1

@avindra
Copy link
Contributor

avindra commented Dec 4, 2019

@mikefrancis probably safe. we should all test and report any issues.

the beta also fixes use of core-js 2.x (whereas create-react-app and other modern build tools tend to use 3.x now).

@mikefrancis
Copy link

@avindra hopefully v2 will be released soon then 🤞

@oneillsp96
Copy link

any status updates on a stable v2 release date?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.