React: Suspense ping shouldn't suspend if all suspended content is already in fallback mode

[RichieAHB]

When using suspense and hooks - specifically useState and useEffect - nested async renders (using react-cache) cause state hooks not to be updated. Specifically this happens between the first promise settling and the last promise settling in a Suspense component. However the state updates concerned can be outside of the Suspense root: https://codesandbox.io/s/wznnxz9pvw

In the linked example, moving the mouse will update the displayed coordinates for the first 2000ms (the time at which the first thrown promise resolves) however, after this the UI will not show any updated coordinates until after the final nested promise in the adjacent Suspense root settles.

I may both be using Suspense wrong (given the understandable lack of documentation) as I've been having other bugs with this setup, specifically the above example will sometimes never render beyond the "loading ..." placeholder. This is especially noticeable with setting higher timeouts in the Timeout component.

react 16.7.0-alpha
react-dom 16.7.0-alpha
react-cache 2.0.0-alpha

[arianon]

For some reason, disabling Concurrent Mode fixed your problem

Maybe it has to do with the scheduler?

[lmatteis]

I don't think suspense will end up being used inside useEffect. It's meant to run in the render. I'm thinking it'll have its own hook. A wild guess:

const movies = usePromise(() => {
 return fetchMovies(moviesUrl)
}, [moviesUrl])

moviesUrl being part of the cache key

[aweary]

This doesn't appear to be related to hooks, as you have the same issue when using a class component. If you force the update inside the mousemove handler to be flushed synchronously by wrapping it with ReactDOM.flushSync, it works as expected.

React makes the distinction between interactive and non-interactive events, where updates that occur as a result of an interactive effect events (like click, keypress, focus) are flushed immediately--even in concurrent mode. When we say "flushed" we mean "the DOM is updated with the new UI".

With non-interactive events, that's not the case. mousemove is considered a non-interactive event, so the update gets deprioritized. In this case it seems like you're encountering a starvation issue where it's not flushing any of the updates while its sibling is suspended. That seems unexpected, but I'm not sure. @acdlite?

ReactDOM.flushSync can be used to force any update to be flushed synchronously (immediately) which is why it resolves the issue here.

[RichieAHB]

Ah good spot, should have double checked this before raising the issue. As
you point out it does still seem unexpected but thanks for narrowing it
down and away from hooks!
On Tue, 30 Oct 2018 at 02:52, Brandon Dail [e-mail] wrote:

This doesn't appear to be related to hooks, as you have the same issue
when using a class component https://codesandbox.io/s/zn9r8znkym. If
you force the update inside the mousemove handler to be flushed
synchronously by wrapping it with ReactDOM.flushSync, it works as
expected.

React makes the distinction between interactive and non-interactive
events, where interactive events (like click, keypress, focus) trigger
updates that are flushed immediately even in concurrent mode.

With non-interactive events, that's not the case. mousemove is considered
a non-interactive event, so the update gets de-prioritized. In this case it
seems like you're encountering a starvation issue where it's not flushing
any of the updates. That seems unexpected, but I'm not sure. @acdlite
https://github.com/acdlite?

ReactDOM.flushSync can be used to force any update to be flushed
synchronously (immediately) which is why it resolves the issue here.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/facebook/react/issues/14012#issuecomment-434152860,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABk1296arwDQNf4bXgSF2RSuMJ3J_dW1ks5up779gaJpZM4X-MPQ
.

[gaearon]

Discussed this a bit. It's expected for real updates but confusing for pings. @sebmarkbage said "The ping itself should not need to suspend if all suspended content is already in fallback mode."

[RichieAHB]

@gaearon is there any chance you could explain the difference between a “ping” and a “real update” and why it would be expected for one and not the other?

[aweary]

@RichieAHB a "real update" is just a regular update that occurs when you update state. I believe a ping occurs when the Promise that was thrown for a suspended components resolves or rejects, "pinging" React so that it knows to try rendering again.

@gaearon is that right?

[RichieAHB]

@aweary ah that makes sense, I guess in that instance there’s no real state to update anywhere as the state change has happened outside React (in the cache). So pinging is just a way of forcing another render with the newly fetched resource in the cache 👌🏻

[davidmccabe]

@gaearon I'm experiencing an issue that I think might be the same as this one, but I'm not sure if I understand what's written here correctly. Would you mind saying whether this is the same issue so that I can skip making a repro case if it's already a KP?

I have a component that is successively suspended on multiple different promises -- that is, while promise 1 is still pending, it re-renders and suspends on promise 2, then while both are still pending it re-renders and suspends on promise 3, and so on. If this has happened with n promises, then when
_any one_ of those promises resolves, React renders the component n times, resulting in something like (n^2)/2 extra re-renders in total. These renders happen via resolveRetryThenable -> retryTimedOutBoundary and result in a commit with no changed components as seen in the React profiler.

Is that the same issue that's described here?

[gaearon]

This appears to be fixed on master.
https://codesandbox.io/s/compassionate-http-eeujc