Skip to content
Please note that GitHub no longer supports Internet Explorer.

We recommend upgrading to the latest Microsoft Edge, Google Chrome, or Firefox.

Learn more
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

Bug: startTransition suspends immediately in odd circumstances #17911

Open
arackaf opened this issue Jan 25, 2020 · 6 comments
Open

Bug: startTransition suspends immediately in odd circumstances #17911

arackaf opened this issue Jan 25, 2020 · 6 comments

Comments

@arackaf
Copy link

@arackaf arackaf commented Jan 25, 2020

React version: 0.0.0-experimental-f42431abe

Please note that I do realize my repro steps are poor at best. I'm not filing this issue in hopes of support; I'm only filing this issue to provide one more datum point to help diagnose what I believe to be a bug, which I'm assuming you'll see more reports of.

Steps To Reproduce

tl;dr - there are some circumstances when a thrown promise inside a hook causes an immediate suspense, instead of respecting the startTransition it's inside of.

startTransition for me is always, in this case, called outside of the normal React handlers. In this case history.listen

https://github.com/arackaf/booklist/blob/special/suspense-blog/react/modules/books/booksSearchState.ts#L75

This is my Suspense-enabled hook that's called as a result of the state update inside the code above

https://github.com/arackaf/micro-graphql-react/blob/feature/suspense/src/useQuery.js#L22

the Promise throwing happens here

https://github.com/arackaf/micro-graphql-react/blob/feature/suspense/src/queryManager.js#L116

Here's the specific chain of events that leads to the breakage.

Things work so long as there's always been an existing promise, for the queryManager to throw, ie line 116 in the code immediately above. But the minute there's cached results, and the queryManager invoke's the hook's setState method (which it passed it), ie line 120 here

https://github.com/arackaf/micro-graphql-react/blob/feature/suspense/src/queryManager.js#L120

then all future suspenses immediately start suspending, and I always get my fallback, always.

The setState method is shared between the hook, and queryManager here

https://github.com/arackaf/micro-graphql-react/blob/feature/suspense/src/useQuery.js#L17

I stress that I do not need support, here; this is just a side project, an unimportant one, and this whole branch is only for Suspense experimenting. But something does definitely appear to be wrong, and I'm hoping this report can help you guys find it.

The current behavior

startTransition only works until the hook's setState is called from the queryManager it creates, at which point startTransition always triggers hard suspenses, showing my fallback.

The expected behavior

startTransition should never trigger the fallback until the timeout has expired.

@arackaf

This comment has been minimized.

Copy link
Author

@arackaf arackaf commented Feb 1, 2020

Just FYI same behavior on the 241c4467e build

@gaearon

This comment has been minimized.

Copy link
Member

@gaearon gaearon commented Feb 2, 2020

Can you create a reduced one-file example with no libraries?

@arackaf

This comment has been minimized.

Copy link
Author

@arackaf arackaf commented Feb 2, 2020

I can try to find some more time to put into this but no promises.

I have to believe there’s something going wrong in how my useQuery hook is sharing the updateState function with the queryManager. I’m most interested in hearing if that’s violating any React constraints which might be causing this.

@arackaf

This comment has been minimized.

Copy link
Author

@arackaf arackaf commented Feb 4, 2020

@gaearon - I managed to get this reproduced! :D

https://codesandbox.io/s/priceless-shirley-o6sfg

If you click pretty much any of the buttons, you'll often see things suspend.

Note the call to useLayoutEffect on line 83. That's the cause of this. Comment that out, and everything is perfect.

Sorry for all the extra crap in the sandbox - I chased down a ton of false leads here, and honestly don't have the time to clean it all up—I need to get back to work! :)

But I'm stoked to finally be able to give you something hopefully actionable.

Lastly, I do realize the layoutEffect isn't even needed, in this particular case, or probably in my original code which surfaced this odd behavior. But it does seem like a bug, unless I'm missing something.

@arackaf

This comment has been minimized.

Copy link
Author

@arackaf arackaf commented Feb 4, 2020

Also - h/t to Daishi Kato for helping to get this sandbox started :D

https://twitter.com/dai_shi/status/1223746342289231877

@dai-shi

This comment has been minimized.

Copy link

@dai-shi dai-shi commented Feb 4, 2020

So, the layout effect runs when a child component throws a promise. Is it a bug or a behavior.

(Note, my first comment in the twitter thread was pointless.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.