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

Update methods to be compatible with React.StrictMode #151

Merged
merged 3 commits into from
Jun 19, 2019

Conversation

jpreynat
Copy link
Contributor

@jpreynat jpreynat commented Jun 6, 2019

This pull request doesn't fix (the following issue) #143 but addresses the removal of now deprecated methods in React 16 to be compatible with <React.StrictMode>.
Since it now uses the getDerivedStateFromProps lifecycle method in its actual state, I set the minimum required version of react and react-dom to ^16.4.0 which includes the bugfix for this method.

I don't have time to improve it better (switching to hooks, etc...) right now, but would be happy to do so when I can find some spare time later.

Pre-Flight Checklist

I have made sure to

  • ? make sure this PR is set to merge to the correct X-0-stable branch
  • add a Storybook example (if necessary)
  • manually test all the Storybook examples
  • run $ yarn run build to build the project code & static example
  • check that the example in docs/index.html works: $ yarn run start

@rpearce
Copy link
Owner

rpearce commented Jun 6, 2019

@jpreynat, thanks for bringing up this issue! This will be addressed with #143.

I've now got somebody helping me on this rewrite, though I'm out of office until 17 June.

@jpreynat
Copy link
Contributor Author

jpreynat commented Jun 7, 2019

Thanks for the answer @rpearce.
Do you have an ETA for the release? We need to update this module pretty quick to use React's concurrent mode, so any update would be appreciated.

@rpearce
Copy link
Owner

rpearce commented Jun 7, 2019

@jpreynat No, and I don't suspect it will be released for a while. Given this, I'll review this PR fully before end of next week. I'm currently out of office officially until the 17th, as my wife and I are moving continents, but I'll make some time for this.

@jpreynat
Copy link
Contributor Author

Thanks for the quick update @rpearce. Let me know if you have any feedback on the PR.
In the meantime, good luck with the moves.

@rpearce
Copy link
Owner

rpearce commented Jun 17, 2019

@jpreynat I'm back and settled in the USA and back at work. This will happen ASAP (I'll review tonight or tomorrow)

@jpreynat
Copy link
Contributor Author

Great, thanks for the update!

@rpearce
Copy link
Owner

rpearce commented Jun 18, 2019

Code looking good! Need to go through all the storybook and build stuff and make sure

@rpearce
Copy link
Owner

rpearce commented Jun 19, 2019

If all is well when I test tomorrow, then I'll ship tomorrow

@rpearce
Copy link
Owner

rpearce commented Jun 19, 2019

Looks good, my friend! There are going to be massssssssssive changes coming down the pipeline to redo basically everything in the coming months. For now, I will:

  1. merge this
  2. update the changelog accordingly
  3. add you to the All Contributors list
  4. publish
  5. check back here

Thank you for your patience and contribution. If you have any other desires for this library, please create an Issue, and I'll consider it for the rewrite!

@rpearce rpearce merged commit f46658f into rpearce:master Jun 19, 2019
@rpearce
Copy link
Owner

rpearce commented Jun 19, 2019

@jpreynat this has been published as 3.1.0. Please let me know if you have any issues.

@jpreynat
Copy link
Contributor Author

Thanks @rpearce for the fast release.
I will keep you updated if anything goes wrong as soon as I've tested it.

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

Successfully merging this pull request may close these issues.

2 participants