-
Notifications
You must be signed in to change notification settings - Fork 49.6k
[DevTools] Exclude Suspense boundaries in hidden Activity #34756
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
base: main
Are you sure you want to change the base?
[DevTools] Exclude Suspense boundaries in hidden Activity #34756
Conversation
21137eb
to
2a2cca5
Compare
function App({hidden}) { | ||
return ( | ||
<> | ||
<Activity mode={hidden ? 'hidden' : 'visiible'}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<Activity mode={hidden ? 'hidden' : 'visiible'}> | |
<Activity mode={hidden ? 'hidden' : 'visible'}> |
if (isHiddenOffscreen(fiber)) { | ||
// hidden Activity is noisy. | ||
// Including it may show overlapping Suspense rects | ||
} else if (isSuspendedOffscreen(fiber)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn’t this always be false because isHiddenOffscreen above is a superset of isSuspendedOffscreen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. I handled the suspended case first in the other places but not here. Now we always handle suspended before hidden.
Though I'm not sure how to make this matter in the mount path. In the current test, fiber.child
is always null
when mounting a suspended Offscreen. I guess we can't hit this in modern Suspense where we don't commit an inconsistent tree. But I believe React 17 could commit an inconsistent, hidden tree where this would matter?
2a2cca5
to
3c482be
Compare
Including Suspense in hidden Activity can be confusing since that likely ends up to overlapping rects (e.g. when each tab content is in an Activity or when previous "pages" are kept in Activity for a better backwards-forwards cache.
We now differentiate between Offscreen used by Suspense and Offscreen used by Activity. Activity-Offscreen will just unmount all nodes. Suspense-Offscreen will stay as-is and unmount nodes except Suspense nodes in the Suspense tree.