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

Suspense issues #15471

Closed
bvaughn opened this issue Apr 22, 2019 · 7 comments
Closed

Suspense issues #15471

bvaughn opened this issue Apr 22, 2019 · 7 comments

Comments

@bvaughn
Copy link
Contributor

bvaughn commented Apr 22, 2019

I'm currently updating the new React DevTools to use suspense for more interactions (bvaughn/react-devtools-experimental#196). This has uncovered three pain points that we should try to address:

  1. Using the "two setState pattern" can be awkward if the priority is "normal" or higher (as was the case in my "keydown" event handler) because scheduler.next uses "normal" priority for this case. (This also applies to e.g. non-React event handlers, since the default priority for these is "normal" as well.) Temporary workaround - manually run the first update with user-blocking priority.

  2. Offscreen updates cause user-blocking and normal priority work to be batched together (for unknown reasons). This makes interactions feel unresponsive. Temporary workaround - none (other than to stop using offscreen priority).

  3. Fallback UI shown too quickly. One of the main reasons to use suspense in the "inspected element" panel to begin with is to avoid a quick flash of the "Loading..." fallback when a new element is selected. However, it seems like this is still happening even with suspense. Temporary workaround - none, since I don't know the cause.

Edit I think the first two items I reported were caused by a second/duplicate scheduler package that had gotten installed in DevTools as a transient dependency.

@bvaughn
Copy link
Contributor Author

bvaughn commented Apr 22, 2019

As a side note, it seems unexpected that "keydown" and "click" handlers (using React's event system) get invoked at "normal" priority rather than "user-blocking".

Edit After stepping through a few call stacks, I realized that this is because the project has two copies of the scheduler module.

@bvaughn
Copy link
Contributor Author

bvaughn commented Apr 22, 2019

I think the first two items I reported were caused by a second/duplicate scheduler package that had gotten installed in DevTools as a transient dependency. 😦

I'm still seeing an unexpected flash of the "Loading..." fallback though:

flashhh-Kapture 2019-04-22 at 13 13 41

@bvaughn
Copy link
Contributor Author

bvaughn commented Apr 22, 2019

It looks like the fallback is being committed right before the resolved data.

@philipp-spiess
Copy link
Contributor

After stepping through a few call stacks, I realized that this is because the project has two copies of the scheduler module

Ohh! I was running into the same issues yesterday when I updated an example to use scheduler@0.14 and react-dom@16.8.6 and was super confused about what’s going on. Thanks for posting about that here!

Since the Scheduler has its own version number, I suspect that others will run into similar issues eventually. Do you think this is worth a warning? I suspect that this won’t be a big issue later when the scheduler will hit v1 as breaking changes would be more alarming.

@bvaughn
Copy link
Contributor Author

bvaughn commented Apr 22, 2019

Yeah, some kind of warning message would have been helpful, @philipp-spiess

@bvaughn
Copy link
Contributor Author

bvaughn commented Apr 22, 2019

I figured out the cause of the temporary "Loading..." fallback commit too.

The new usage of suspense in the DevTools relies on data about the selected Fiber being fetched (asynchronously) across a "bridge" between the DevTools extension and the page context where React is running. The code that was sending the "inspect" message across the Bridge was inside of an effect handler (since it needed to attach an event listener). The way I had originally written the code, this effect itself was blocking on the suspended render (since both it and the code that suspended were reading from the same selected-fiber-id prop).

The fix for this was to change the side effect code to read from a different prop, one that was updated by a higher-priority state update, so that it got fired before the suspending render.

This is kind of subtle and is something we'll need to add good documentation around before the public API gets broader adoption. For now though I'm going to close this issue since it seems like there are no outstanding bugs.

@bvaughn bvaughn closed this as completed Apr 22, 2019
@NE-SmallTown
Copy link
Contributor

NE-SmallTown commented Apr 28, 2019

I think maybe you need edit the No.1 content of the thread because you just mentioned suspense but the fact is concurrentmode && suspense

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

3 participants