Skip to content

"de-opting to synchronous mode" in use-subscription README #17201

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

Closed
jamesknelson opened this issue Oct 28, 2019 · 4 comments
Closed

"de-opting to synchronous mode" in use-subscription README #17201

jamesknelson opened this issue Oct 28, 2019 · 4 comments

Comments

@jamesknelson
Copy link

jamesknelson commented Oct 28, 2019

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

Outdated README (maybe?)

What do the docs currently say?

use-subscription is safe to use in concurrent mode. However, it achieves correctness by sometimes de-opting to synchronous mode, obviating the benefits of concurrent rendering.

In the linked issue, @bvaughn explains that this is referring to chains of synchronous updates using componentDidUpdate. However, the useSubscription hook now uses a passive useEffect(), as opposed to a synchronous componentDidUpdate().

Would this mean that it's no longer "de-opting to sync mode", and the warning could be removed from the README?

@stale
Copy link

stale bot commented Jan 27, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Jan 27, 2020
@jamesknelson
Copy link
Author

Would still love an answer on this one!

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Jan 27, 2020
@bvaughn
Copy link
Contributor

bvaughn commented Jan 27, 2020

The wording of the README isn't super accurate for useSubscription. I kind of copy-pasted that READ (with some tweaks) from createSubscription.

That being said, useSubscription() still has some downsides (both deopts and potential tearing). The decision to use a passive effect instead of an active one was a tradeoff between lesser bad options essentially.

I took the time this afternoon to create a Gist outlining the de-opt and potential tearing cases for useSubscription and a new hook (not yet created but soon to be RFCed) called useMutableSource:
https://gist.github.com/bvaughn/054b82781bec875345bd85a5b1344698

If you'd like to send a PR to update the wording on the use-subscription README, tag me as a reviewer. Else I will do a pass eventually when I get to implementing this new hook.

@bvaughn bvaughn closed this as completed Jan 27, 2020
@dai-shi
Copy link
Contributor

dai-shi commented Jan 29, 2020

#15317 (comment)
@kitten Have you seen this issue?

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

No branches or pull requests

4 participants