-
Notifications
You must be signed in to change notification settings - Fork 49.7k
[DevTools] Send Suspense rects to frontend #34170
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
Conversation
| ); | ||
| // We snapshot each step once so it doesn't regress.d | ||
| snapshots.push(print(store)); | ||
| snapshots.push(print(store, false, null, false)); |
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.
These are flipping between null rects and [] rects. Need to double check if intentional and if we can avoid that by rewriting tests.
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.
In this test we're moving Host components from primary content to fallbacks. This means we move rect attribution to the shell which means rects change between tests. This test is really just for the Component tree so we can safely ignore Suspense tree at the moment.
Printing the Suspense tree in the store already proofed helpful though: a9e9cae (#34170)
| } | ||
| const fiberInstance = suspenseNode.instance; | ||
| if (fiberInstance.kind !== FIBER_INSTANCE) { | ||
| // TODO: Resizes of filtered Suspense nodes are currently dropped. |
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.
Need to figure out how to get IDs for filtered nodes. IDs of filtered instances are hidden on purpose so I need to double check if it's safe to use the ID here.
3de6916 to
64b14f1
Compare
64b14f1 to
a9e9cae
Compare
| // just use the Fiber anyway. | ||
| newSuspenseNode.rects = measureInstance(newInstance); | ||
| // Fallbacks get attributed to the parent so we only measure if we're | ||
| // showing primary content. |
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.
So I intentionally measured them even for the fallback state so that if it's stuck like that you at least have something to click on and to show where it's going on.
Once it resolves it should stay on the resolved rect. It's a little weird when you have inner suspense in fallbacks because they could potentially overlap with the old content but those are weird regardless.
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.
Technically it should probably resize if in fallback state and it has never ever been in content state but I didn't bother tracking that.
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.
I think we probably should still keep measuring the fallback for a SuspenseNode that mounts in fallback state and maybe fix any issues with update erasing that.
But stamping to unblock the store.
No description provided.