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

changed lifecycle usages to be react 17 compliant #531

Closed
wants to merge 1 commit into from
Closed

changed lifecycle usages to be react 17 compliant #531

wants to merge 1 commit into from

Conversation

s-kem
Copy link

@s-kem s-kem commented Oct 14, 2019

Fixes issue referenced in #523

React 16.8 and above throws warnings on usages for these now deprecated methods, and they will be removed in React 17, leaving only the UNSAFE versions

@alexkrolick
Copy link
Collaborator

I don't think this is the right solution, since this library is pretty much all side-effects we actually need to be async-safe #509 (comment)

@alexkrolick
Copy link
Collaborator

Rewriting it should also fix loads of outstanding issues ;)

@s-kem
Copy link
Author

s-kem commented Oct 15, 2019

I understand if there's an underlying problem that this isn't addressing, but my understanding here is the UNSAFE tagged functions are exactly equivalent to the originals, resulting in no change in behaviour.

This isn't a proper solution, I'd agree, but it's at least a bandaid to silence the warnings in the current versions of react, and without which this library becomes unusable in React 17.

@alexkrolick
Copy link
Collaborator

My point is that it almost certainly will not work with the unsafe methods in React 17, so all this accomplishes is silencing the warnings. Which we can release if you want, but it will still need a rewrite. The UNSAFE_ methods are most unsafe in exactly the ways we are using them now (DOM synchronization side effects).

@fahadahmed
Copy link

Hi team,

are there any plans to make the package work with React 17 and hooks? If yes, what are the timelines, if any.

Thanks
Fahad

@alexkrolick
Copy link
Collaborator

Closing in favor of #549

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.

3 participants