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

Add unsafe to component mount #250

Merged
merged 1 commit into from Jan 2, 2020

Conversation

JackHowa
Copy link
Contributor

Noticed a deprecation warning ahead of React 17. I've been working on trying to narrow down my project's warnings.

Screen Shot 2019-10-17 at 4 58 21 PM

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

@arananegra
Copy link

What about refactoring componentWillMount to componentDidMount?

@JackHowa
Copy link
Contributor Author

JackHowa commented Oct 28, 2019

I'm looking into tests as well for this. Wasn't sure since render will run after componentDidMount and saw the comment. It's could be a good refactor. https://github.com/mjrussell/redux-auth-wrapper/pull/250/files#diff-5f8ff02aeff466e04cfee5839e0f3bb7L16

  render() {
    // Redirect should happen before this is rendered
    return null
  }

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 95e9321 on JackHowa:unsafe-component-codemod into fecffb1 on mjrussell:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 95e9321 on JackHowa:unsafe-component-codemod into fecffb1 on mjrussell:master.

@JackHowa
Copy link
Contributor Author

what do you think @mjrussell ?

@JackHowa
Copy link
Contributor Author

Let me know if you have any feedback @mjrussell. I'm trying to wrap up prs before the holidays so not nagging then

@pavolgolias
Copy link

Any progress on this? It would be great to fix compatibility for upcoming React 17.

@JackHowa
Copy link
Contributor Author

JackHowa commented Dec 5, 2019

I added the unsafe prefix there to maintain compatibility. Haven't heard from @mjrussell yet on this @pavolgolias

@mjrussell
Copy link
Owner

Sorry all, I've been really busy at work lately. I'll try to review this this weekend and get it in.

@pavolgolias
Copy link

Perfect, thanks in advance ;)

@JackHowa
Copy link
Contributor Author

Just checking in on this @mjrussell. Sorry to be a bother

@mjrussell
Copy link
Owner

Not a bother! Sorry I clearly don't have enough time to maintain this repo well. Going to get this into a new release now

@mjrussell mjrussell merged commit f5b0060 into mjrussell:master Jan 2, 2020
@mjrussell
Copy link
Owner

mjrussell commented Jan 2, 2020

This ended up being a bit painful...I had to upgrade React, fix a lot of test deps and use a different library instead of react-router-redux.

Most of the changes are here - #257

But I haven't yet updated the examples or docs. This will be a new major release due to all the breaking changes

@mjrussell
Copy link
Owner

Just did the examples in #259. Once I do the docs I can cut a new version

@arananegra
Copy link

Thanks for addressing this. I really love this package and I really appreciate it!

@mjrussell
Copy link
Owner

mjrussell commented Jan 3, 2020

Version 3.0.0 which was just released on npm includes this. Thanks everyone for your patience. Let me know if you run into any issues

@JackHowa
Copy link
Contributor Author

Thanks for working on this @mjrussell

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 this pull request may close these issues.

None yet

5 participants