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

Fix <StrictMode> warnings #897

Closed
BenoitClaveau opened this issue Mar 8, 2018 · 35 comments
Closed

Fix <StrictMode> warnings #897

BenoitClaveau opened this issue Mar 8, 2018 · 35 comments

Comments

@BenoitClaveau
Copy link

Is it possible to replace componentWillReceiveProps because is deprecated to remove React 16+ warning.

@timdorr
Copy link
Member

timdorr commented Mar 8, 2018

cWRP isn't deprecated until React 16.4. As of this moment, 16.3 isn't even out. We're looking at larger refactors around async stuff in React 17 in #890.

@timdorr timdorr closed this as completed Mar 8, 2018
@BenoitClaveau
Copy link
Author

cWRP seems to be depreacted.

screenshot_1520587667

@TrySound
Copy link
Contributor

TrySound commented Mar 9, 2018

@BenoitClaveau Which version of react do you use? It shouldn't happen until 16.4.

@respectTheCode
Copy link

This started showing up after updating to react-native 0.54.0.

@respectTheCode
Copy link

downgrading to react 16.2.0 doesn't remove the warnings

@TrySound
Copy link
Contributor

TrySound commented Mar 9, 2018

I guess the problem is react-native. AFAIK they published package with react alpha which had this problem. Try to downgrade it.

@BenoitClaveau
Copy link
Author

under "react": "16.2.0",

@respectTheCode
Copy link

The warnings are coming from react-native so downgrading react isn't enough. react-native 0.53 with react 16.2.0 doesn't have the warnings

@TrySound
Copy link
Contributor

TrySound commented Mar 9, 2018

Yes, I meant downgrade react-native.

@anonrig
Copy link

anonrig commented Mar 17, 2018

Downgrading shouldn't be the option, since the latest valid version of react-native does deprecated these functions. Is there a ETA on the change?

@catamphetamine
Copy link

catamphetamine commented Apr 2, 2018

getDerivedStateFromProps is easy enough using the react-lifecycles-compat thing which isn't a breaking change.
And context also has a non-breaking polyfill (though only for React 16 which would be a breaking change):
https://github.com/thejameskyle/create-react-context

@Hypnosphi
Copy link
Contributor

Hypnosphi commented Apr 2, 2018

@timdorr I would like to make a PR migrating react-redux to new lifecycles using react-lifecycles-compat because I don't want StrictMode to warn me about my Connected components. Does it make sense to you?

I'm surprised I have to mention it on this repo, but this React blogpost says that library authors don't have to wait until actual deprecation happens
https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html

The primary purpose of the upcoming version 16.3 release is to enable open source project maintainers to update their libraries in advance of any deprecation warnings.

@gaearon gaearon reopened this Apr 2, 2018
@gaearon
Copy link
Contributor

gaearon commented Apr 2, 2018

Yes, the purpose of 16.3 was to let libraries start migrating before the deprecation warnings.

@catamphetamine
Copy link

catamphetamine commented Apr 2, 2018

@gaearon I also think it would make a lot of sense for you to include a link to create-react-context polyfill same as you did for react-lifecycles-compat because otherwise people don't even know that they can transition their libraries to the new context without breaking React 16 users (though breaking all older Reacts).
Specifically I had to google a lot until I accidentally came across that create-react-context polyfill thing. It would make life easier for a lot of component mainterners if you included that link as a sidenote in the announcement blog post.

UPD: Oh, actually, it was not you, it was "Brian Vaughn", ok.

@timdorr timdorr changed the title Use static getDervicedStateFromProps instead of componentWillReceiveProps Fix <StrictMode> warnings Apr 2, 2018
@timdorr
Copy link
Member

timdorr commented Apr 2, 2018

Updating the title to better reflect what this is about. I'll put up a failing PR in a sec that others can build off of.

Edit: Looks like we're on React 15 still. I'm updating things to Jest and then React 16. And then I'll get to this. Bikeshedding at maximum speed!

@timdorr
Copy link
Member

timdorr commented Apr 2, 2018

OK, phew, got all that done. We're now on React 16, Jest, and react-test-renderer.

Here's a failing test: #918

@Hypnosphi
Copy link
Contributor

Hypnosphi commented Apr 8, 2018

@catamphetamine Looks like create-react-context can actually be used to support React 15 and below. One should just be carefull and always pass a singe child element to Provider and always return a single element from Consumer child function. See jamiebuilds/create-react-context#15

@jamiebuilds
Copy link

@Hypnosphi Yeah... create-react-context only polyfills React.createContext you are still limited to the rendering features of the versions of React you are compatible with.

@packetstracer
Copy link

has this fix been released? which version?
I'm using:

"react": "^16.3.2",
"react-redux": "^5.0.6",

and still getting the error in strict-mode.

For example:

componentWillReceiveProps: Please update the following components to use static getDerivedStateFromProps instead: Connect(ProfileHandlerHoc)

componentWillUpdate: Please update the following components to use componentDidUpdate instead: Connect(ProfileHandlerHoc)

@Hypnosphi
Copy link
Contributor

@packetstracer #919 isn't merged yet

@markerikson
Copy link
Contributor

@packetstracer : please update to React-Redux 5.0.7 first, and see if that fixes the issue for you.

@ritchieanesco
Copy link

i've updated to react-redux 5.0.7 and also getting the same warnings as @packetstracer

@packetstracer
Copy link

still throwing these errors after update

componentWillReceiveProps: Please update the following components to use static getDerivedStateFromProps instead: Connect(DemoSideBarNavContainer), Connect(LanguageSelectorContainer), Connect(ProfileHandlerHoc)

componentWillUpdate: Please update the following components to use componentDidUpdate instead: Connect(DemoSideBarNavContainer), Connect(LanguageSelectorContainer), Connect(ProfileHandlerHoc)

package.json content
"react-redux": "^5.0.7",

cat node_modules/react-redux/package.json

{
  "name": "react-redux",
  "version": "5.0.7",
  "description": "Official React bindings for Redux",

Any clue on what could be?

@timdorr
Copy link
Member

timdorr commented May 18, 2018

Those aren't fixed in 5.0.7. We have an outstanding PR to fix them.

@slim-hmidi
Copy link

So what is the alternative to use until the PR was merged to fix the warnings?

@Hypnosphi
Copy link
Contributor

@slim-hmidi are you building a library or an app? If the latter, you shouldn't need StrictMode for now

@slim-hmidi
Copy link

@Hypnosphi I'm building an application.

@daraclare
Copy link

Hey folks, any update on the PR being merged? Thanks!

@grzegorzjudas
Copy link

Anyone coming here with this problem, you can use react-redux@next with npm (resolved to 5.1.0-test.1 as of now).

@josephcsible
Copy link

Can this issue be reopened, since the commit that fixed it was reverted in 73691e5?

@timdorr
Copy link
Member

timdorr commented Oct 10, 2018

Sure. Though either #995 or #1000 is going to solve it.

@brunolemos
Copy link

Now that #1000 was merged, please make sure it works with StrictMode as well before releasing! 🙌 cc @timdorr @markerikson

@markerikson
Copy link
Contributor

markerikson commented Nov 6, 2018

Tbh, I haven't tried that part out yet myself... but go install react-redux@next and let us know!

Purely off the top of my head, it ought to be just fine. We no longer use cWRP or cWU at all.

@brunolemos
Copy link

brunolemos commented Nov 6, 2018

Just tested 6.0.0-beta.2, it did fix the warnings! :) thanks.

Now it's just react-native left :P facebook/react-native#22186

@timdorr
Copy link
Member

timdorr commented Nov 7, 2018

Clooooosing out!

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.