search

reduxjs / react-redux

Official React bindings for Redux
https://react-redux.js.org
MIT License
23.38k stars 3.36k forks source link

React 16.9 Deprecation *Will* warning #1374

Closed BiosBoy closed 5 years ago

BiosBoy commented 5 years ago

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

What is the current behavior? React throw a warning about all deprecated WILL methods inside react-redux. Example: https://i.ibb.co/mbWrWYC/Screen-Shot-2019-08-09-at-11-06-13-AM.png

What is the expected behavior? As long as React works on cut out deprecation methods, this package (react-redux) should also be up to date with.

Which versions of React, ReactDOM/React Native, Redux, and React Redux are you using? Which browser and OS are affected by this issue? Did this work in previous versions of React Redux? React: 16.9.0 Redux: 16.9.0 React-Redux: 7.1.0

markerikson commented 5 years ago

That can't be right, because we stopped using componentWillUpdate as of React-Redux v6, and v7 is implemented with hooks inside so it doesn't even use class components.

Please double-check the library versions you actually have installed in your app.

mikecousins commented 5 years ago

I'm seeing this with the Provider component as well.

breadcrumbs.js:76 Warning: componentWillReceiveProps has been renamed, and is not recommended for use. See https://fb.me/react-async-component-lifecycle-hooks for details.

Please update the following components: Provider

markerikson commented 5 years ago

Well, as you can see, Provider does not use any of those lifecycles in v7.

So, whatever you're seeing, it's not due to our current version. We removed all the deprecated lifecycles in v6.

mikecousins commented 5 years ago

Weird, it must be an error in something below it that's getting misreported or something...

BiosBoy commented 5 years ago

Hm... seems like really the problem was inside misversion of imported react-redux. All good now, sorry.

mikecousins commented 5 years ago

It's very strange.

yarn list --pattern react-redux yarn list v1.17.3 ├─ @types/react-redux@7.1.2 └─ react-redux@7.1.0 Done in 0.71s.

image

mikecousins commented 5 years ago

It must be misreporting the error based on a child having legacy context and cwrp....

BiosBoy commented 5 years ago

@mikecousins in my case it was exactly so.

elyobo commented 5 years ago

Any chance of backporting the fix to the 5.x series (or even, for the moment, the UNSAFE_ version)? enzyme still doesn't support the newer context API, which you also moved to in v6, making upgrading impossible for us at the moment and this generates a huge amount of warning noise.

markerikson commented 5 years ago

@elyobo : we tried to update the code to get rid of the lifecycle warnings as a 5.x release, and that failed completely. So, the "fix" was a complete rewrite, first in v6 and then in v7. Nothing we can backport.

We might be able to do a rename to UNSAFE and put out a 5.x patch, but the real answer is you should stop using v5 and update to v7. We're not responsible for Enzyme's inability to keep up with React.

Your other option, of course, would be to stay on React 16.8 to avoid the warnings.

timdorr commented 5 years ago

Also, adding UNSAFE_ prefixes would break support for React 15 and 0.14. We're facing the same issue on React Router. I'm not sure there's a good fix yet.

markerikson commented 5 years ago

@elyobo : based on Twitter responses, I'm not clear on what the limitations are with context and Enzyme. It sounds as if it ought to be supported with mount(), but if it doesn't work with shallow(), that may actually be a React test utils thing:

https://twitter.com/dan_abramov/status/1164158172673794048

Either way, though, you really should update to v7.

gaearon commented 5 years ago

that may actually be a React test utils thing

It's not a "thing" per se. Shallow rendering renders one level deep. Context by definition adds a level. So you can't really expect it to work if you use shallow rendering. My suggestion would be to not use shallow rendering.

elyobo commented 5 years ago

Thanks @markerikson for your response and @gaearon for chipping in as well.

I'm not using shallow rendering, my issues are with mount (it was working with earlier react-redux and it wouldn't have been if I was using shallow). I assumed it was something that I needed to change for the upgrade, but ljharb's comment here says (on Jan 2 2019, so a while ago)

enzyme does not yet support createContext. See #1553.

The linked issue https://github.com/airbnb/enzyme/issues/1553 has several issues mentioned for the new context API but as far as I can see it's still not complete.

Specifically with regards to this

Also, adding UNSAFE_ prefixes would break support for React 15 and 0.14. We're facing the same issue on React Router. I'm not sure there's a good fix yet.

If it was done naively then yes it would break support for earlier React, but I was envisaging some sort of feature detection that would identify which version of the lifecycle event it should use based on the React version in use and that could therefore maintain backwards compatibility as well. Not sure if that's viable?

I'd like to upgrade, but the options don't look great (dropping a lot of tests, or possibly rewriting them using one of the other test libs) and so my inclination is to stick with an enzyme 5.x and probably react 16.8.x until enzyme catches up (which looks to be their intention) and eases the upgrade path.

Reducing the noise of the warning, and providing react 17.x compatibility as well (in case enzyme takes a long time!) in the 5.x branch would make that more pleasant but it's not critical - it is just noise for now.

EDIT

Detecting the react version in use and using the right name seems like it would be straightforward enough in react-redux - usage of the deprecated livecycle events is very limited, and the version is available at require('react').version so detection is trivial. Unsure how much use react-router makes of the deprecated events though @timdorr.

elyobo commented 5 years ago

:point_up: Added an example of how feature detection in the 5.x branch could work to support both older and newer React versions without the noise and with forward compatibility for React 17.x.

AndyOGo commented 5 years ago

hmh, we have "react-redux": "^7.1.0", installed, though we still get:

Warning: componentWillReceiveProps 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.
  • If you're updating state whenever props change, refactor your code to use memoization techniques or move it to static getDerivedStateFromProps. Learn more at: https://fb.me/react-derived-state
  • Rename componentWillReceiveProps to UNSAFEcomponentWillReceiveProps 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: Connect(Anchor)

pprachit09 commented 5 years ago

Same issue for me as @AndyOGo even after updating to ^7.1.0

markerikson commented 5 years ago

Again, I can confirm that this is not a problem with React-Redux in v7, because we do not use these lifecycle methods. This is some kind of false positive.

peterkrieg commented 4 years ago

@AndyOGo @pprachit09 did you ever find a solution to this? I know this isn't react-redux's fault at all, since there is nowhere these deprecated methods are being used.. Is there a known false-positive documented react bug that people are aware of?

I'm running react 16.13.1, react-redux 7.2.0 and seeing this exact error, for Provider. Adds a lot of noise to logs, especially for running tests.

mikecousins commented 4 years ago

It seems to just misreport one of it's parents sometimes. For me it was the react-stripe-elements library that was causing the actual issue which was a child of my Provider component.

peterkrieg commented 4 years ago

Ah thanks, good call - bumping react-stripe-elements to at least 5.0.0 fixed it for me. I was trying to upgrade different packages but didn't try that one. Cheers! 🎉

mikecousins commented 4 years ago

And you should move to @react/stripe-js instead as react-stripe-elements is deprectated now 👍

gaearon commented 4 years ago

If you see it misreporting a component, please file an issue with a reproducing case in React.

mikecousins commented 4 years ago

Here's a replication sandbox, I'll open an issue in React as well.

https://codesandbox.io/s/trusting-ramanujan-0yhqb

mikecousins commented 4 years ago

https://github.com/facebook/react/issues/18431

mikecousins commented 4 years ago

I dug into this more and I think it's because inside the react-stripe-elements library their component is named Provider as well....