facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
228.85k stars 46.83k forks source link

Bug: Suspense should hide Portals deeper in the tree #26612

Open gaearon opened 1 year ago

gaearon commented 1 year ago

When Portal is a direct child of Suspense, suspending hides the portal: https://codesandbox.io/s/cocky-boyd-mlq2ko?file=/src/App.js

But when a Portal is deeper in the tree, suspending fails to hide the portal nodes: https://codesandbox.io/s/nostalgic-fog-udyhuz?file=/src/App.js

We need to fix this to recursively hide portals. There's a question of which traversal to use, and how it combines with existing traversal. E.g. does "hiding" happen before layout effects run?

We would also need to decide what to do with legacy mode. One option is to "fix" it there too. But that might be difficult to roll out. It might also be tricky to implement. Another option is to leave it as is (the current behavior is leaving a "hole" in place of the suspended component: https://codesandbox.io/s/elastic-ptolemy-2u6qel?file=/src/App.js). Then we'd need to make sure we at least keep that buggy behavior.

We might want to add an internal-only warning (to be muted but logged on devservers) to track newly hidden nodes portals that wouldn't have been hidden with the previous algorithm. Then maybe this would let us know where to fix the UI and avoid the UI regression caused by modals disappearing.

Summary of work that needs to be done:

quiteeasy commented 1 year ago

Hi @gaearon, it seems there is no active development on this issue right now. I would start working on it if possible.