Skip to content

Commit

Permalink
[DevTools] Refactor Forcing Fallback / Error of Suspense / Error Boun…
Browse files Browse the repository at this point in the history
…daries (#30870)

First, this basically reverts
1f3892e
to use a Map/Set to track what is forced to suspend/error again instead
of flags on the Instance. The difference is that now the key in the
Fiber itself instead of the ID. Critically this avoids the
fiberToFiberInstance map to look up whether or not a Fiber should be
forced to suspend when asked by the renderer.

This also allows us to force suspend/error on filtered instances. It's a
bit unclear what should happen when you try to Suspend or Error a child
but its parent boundary is filtered. It was also inconsistent between
Suspense and Error due to how they were implemented.

I think conceptually you're trying to simulate what would happen if that
Component errored or suspended so it would be misleading if we triggered
a different boundary than would happen in real life. So I think we
should trigger the nearest unfiltered Fiber, not the nearest Instance.
The consequence of this however is that if this instance was filtered,
there's no way to undo it without refreshing or removing the filter.
This is an edge case though since it's unusual you'd filter these in the
first place.

It used to be that Suspense walked the store in the frontend and Error
walked the Fibers in the backend. They also did this somewhat eagerly.
This simplifies and unifies the model by passing the id of what you
clicked in the frontend and then we walk the Fiber tree from there in
the backend to lazily find the boundary. However I also eagerly walk the
tree at first to find whether we have any Suspense or Error boundary
parents at all so we can hide the buttons if not.

This also implements it to work with VirtualInstances using #30865. I
find the nearest Fiber Instance downwards filtered or otherwise. Then
from its parent we find the nearest Error or Suspense boundary. That's
because VirtualInstance will always have their inner Fiber as an
Instance but they might not have their parent since it might be
filtered. Which would potentially cause us to skip over a filtered
parent Suspense boundary.
  • Loading branch information
sebmarkbage authored Sep 5, 2024
1 parent d72e477 commit a06cd9e
Show file tree
Hide file tree
Showing 8 changed files with 170 additions and 257 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2975,16 +2975,12 @@ describe('InspectedElement', () => {
// Inspect <ErrorBoundary /> and see that we cannot toggle error state
// on error boundary itself
let inspectedElement = await inspect(0);
expect(inspectedElement.canToggleError).toBe(false);
expect(inspectedElement.targetErrorBoundaryID).toBe(null);
expect(inspectedElement.canToggleError).toBe(true);

// Inspect <Example />
inspectedElement = await inspect(1);
expect(inspectedElement.canToggleError).toBe(true);
expect(inspectedElement.isErrored).toBe(false);
expect(inspectedElement.targetErrorBoundaryID).toBe(
targetErrorBoundaryID,
);

// Suppress expected error and warning.
const consoleErrorMock = jest
Expand All @@ -3009,20 +3005,13 @@ describe('InspectedElement', () => {
inspectedElement = await inspect(0);
expect(inspectedElement.canToggleError).toBe(true);
expect(inspectedElement.isErrored).toBe(true);
// its error boundary ID is itself because it's caught the error
expect(inspectedElement.targetErrorBoundaryID).toBe(
targetErrorBoundaryID,
);

await toggleError(false);

// We can now inspect <Example /> with ability to toggle again
inspectedElement = await inspect(1);
expect(inspectedElement.canToggleError).toBe(true);
expect(inspectedElement.isErrored).toBe(false);
expect(inspectedElement.targetErrorBoundaryID).toBe(
targetErrorBoundaryID,
);
});
});

Expand Down
Loading

0 comments on commit a06cd9e

Please sign in to comment.