Support filtering Virtual Instances with existing filters.
Server Components are considered "Functions".
In a follow up I'll a new filter for "Environment" which will let you
filter by Client vs Server (and more).
This appends a (filtered) virtual instance path at the end of the fiber
path. If a virtual instance is selected inside the fiber.
The main part of the path is still just the fiber path since that's the
semantically stateful part. Then we just tack on a few virtual path
frames at the end if we're currently selecting a specific Server
Component within the nearest Fiber.
I also took the opportunity to fix a bug which caused selections inside
Suspense boundaries to not be tracked.
This loops over the remainingReconcilingChildren to find existing
FiberInstances that match the updated Fiber. This is the same thing we
already do for virtual instances. This avoids the need for a
`fiberToFiberInstanceMap`.
This loop is fast but there is a downside when the children set is very
large and gets reordered with keys since we might have to loop over the
set multiple times to get to the instances in the bottom. If that
becomes a problem we can optimize it the same way ReactChildFiber does
which is to create a temporary Map only when the children don't line up
properly. That way everything except the first pass can use the Map but
there's no need to create it eagerly.
Now that we have the loop we don't need the previousSibling field so we
can save some memory there.
These don't have their own time since they don't take up any time to
render but they show up in the tree for context. However they never
render themselves. Their base tree time is the base time of their
children. This way they take up the same space as their combined
children in the Profiler tree. (Instead of leaving a blank line which
they did before this PR.)
The frontend doesn't track the difference between a virtual instance and
a Fiber that didn't render this update. This might be a bit confusing as
to why it didn't render. I add the word "client" to make it a bit
clearer and works for both. We should probably have different verbiage
here based on it is a Server Component or something else.
<img width="1103" alt="Screenshot 2024-08-26 at 5 00 47 PM"
src="https://github.com/user-attachments/assets/87b811d4-7024-466a-845d-542493ed3ca2">
I also took the opportunity to remove idToTreeBaseDurationMap and
idToRootMap maps. Cloning the Map isn't really all that super fast
anyway and it means we have to maintain the map continuously as we
render. Instead, we can track it on the instances and then walk the
instances to create a snapshot when starting to profile. This isn't as
fast but really fast too and requires less bookkeeping while rendering
instead which is more sensitive than that one snapshot in the beginning.
We don't have the source location of Server Components on the client
because we don't want to eagerly do the throw trick for all Server
Components just in case. Unfortunately Node.js doesn't expose V8's API
to get a source location of a function.
We do have the owner stacks of the JSX that created it though and at
some point we'll also show that location in DevTools.
However, the realization is that if a Server Component is the owner of
any child. The owner stack of that child will have the owner component's
source location as its bottom stack frame.
The technique I'm implementing here is to track whenever a child mounts
we already have its owner. We track the first discovered owned child's
stack on the owner. Then when we ask for a Source location of the owner
do we parse that stack and extract the location of the bottom frame.
This doesn't give us a location necessarily in the top of the function
but somewhere in the function.
In this case the first owned child is the Container:
<img width="1107" alt="Screenshot 2024-08-22 at 10 24 42 PM"
src="https://github.com/user-attachments/assets/95f32850-24a5-4151-8ce6-b7b89db68aee">
<img width="648" alt="Screenshot 2024-08-22 at 10 24 20 PM"
src="https://github.com/user-attachments/assets/4bcba033-866f-4684-9beb-de09d189deff">
We can even use this technique for Fibers too. Currently I use this as a
fallback in case the error technique didn't work. This covers a case
where nothing errors but you still render a child. This case is actually
quite common:
```
function Foo() {
return <Bar />;
}
```
However, for Fibers we could really just use the `inspect(function)`
technique which works for all cases. At least in Chrome.
Unfortunately, this technique doesn't work if a Component doesn't create
any new JSX but just renders its children. It also currently doesn't
work if the child is filtered since I only look up the owner if an
instance is not filtered. This means that the container in the fixture
can't view source by default since the host component is filtered:
```
export default function Container({children}) {
return <div>{children}</div>;
}
```
<img width="1107" alt="Screenshot 2024-08-22 at 10 24 35 PM"
src="https://github.com/user-attachments/assets/c3f8f9c5-5add-4d35-9290-3a5079e82adc">
I noticed that there is a delay due to the inspection being split into
one part that gets the attribute and another eval that does the
inspection. This is a bit hacky and uses temporary global names that are
leaky. The timeout was presumably to ensure that the first step had
fully propagated but it's slow. As we've learned, it can be throttled,
and it isn't a guarantee either way.
Instead, we can just consolidate these into a single operation that
by-passes the bridge and goes straight to the renderer interface from
the eval.
I did the same for the viewElementSource helper even though that's not
currently in use since #28471 but I think we probably should return to
that technique when it's available since it's more reliable than the
throw - at least in Chrome. I'm not sure about the status of React
Native here. In Firefox, inspecting a function with source maps doesn't
seem to work. It doesn't jump to original code.
This enables finding Server Components on the owner path. Server
Components aren't stateful so there's not actually one specific owner
that it necessarily matches. So it can't be a global look up. E.g. the
same Server Component can be rendered in two places or even nested
inside each other.
Therefore we need to find an appropriate instance using a heuristic. We
can do that by traversing the parent path since the owner is likely also
a parent. Not always but almost always.
To simplify things we can also do the same for Fibers. That brings us
one step closer to being able to get rid of the global
fiberToFiberInstance map since we can just use the shadow tree to find
this information.
This does mean that we can't find owners that aren't parents which is
usually ok. However, there is a test case that's interesting where you
have a React ART tree inside a DOM tree. In that case the owners
actually span multiple renderers and roots so the owner is not on the
parent stack. Usually this is fine since you'd just care about the
owners within React ART but ideally we'd support this. However, I think
that really the fix to this is that the React ART tree itself should
actually show up inside the DOM tree in DevTools and in the virtual
shadow tree because that's conceptually where it belongs. That would
then solve this particular issue. We'd just need some way to associate
the root with a DOM parent when it gets mounted.
Supports showing the key in DevTools on the Server Component that the
key was applied to. We can also use this to reconcile to preserve
instance equality when they're reordered.
One thing that's a bit weird about this is that if you provide an
explicit key on a Server Component that alone doesn't have any
semantics. It's because we pass the key down and let the nearest child
inherit the key or get prefixed by the key.
So you might see the same key as a prefix on the child of the Server
Component too which might be a bit confusing. We could remove the prefix
from children but that might also be a bit confusing if they collide.
The div in this case doesn't have a key explicitly specified. It gets it
from the Server Component parent.
<img width="1107" alt="Screenshot 2024-08-14 at 10 06 36 PM"
src="https://github.com/user-attachments/assets/cfc517cc-e737-44c3-a1be-050049267ee2">
Overall keys get a bit confusing when you apply filter. Especially since
it's so common to actually apply the key on a Host Instance. So you
often don't see the key.
This adds VirtualInstances to the tree. Each Fiber has a list of its
parent Server Components in `_debugInfo`. The algorithm is that when we
enter a set of fibers, we actually traverse level 0 of all the
`_debugInfo` in each fiber. Then level 1 of each `_debugInfo` and so on.
It would be simpler if `_debugInfo` only contained Server Component
since then we could just look at the index in the array but it actually
contains other data as well which leads to multiple passes but we don't
expect it to have a lot of levels before hitting a reified fiber.
Finally when we hit the end a traverse the fiber itself.
This lets us match consecutive `ReactComponentInfo` that are all the
same at the same level. This creates a single VirtualInstance for each
sequence. This lets the same Server Component instance that's a parent
to multiple children appear as a single Instance instead of one per
Fiber.
Since a Server Component's result can be rendered in more than one place
there's not a 1:1 mapping though. If it is in different parents or if
the sequence is interrupted, then it gets split into two different
instances with the same `ReactComponentInfo` data.
The real interesting case is what happens during updates because this
algorithm means that a Fiber can become reparented during an update to
end up in a different VirtualInstance. The ideal would maybe be that the
frontend could deal with this reparenting but instead I basically just
unmount the previous instance (and its children) and mount a new
instance which leads to some interesting scenarios. This is inline with
the strategy I was intending to pursue anyway where instances are
reconciled against the previous children of the same parent instead of
the `fiberToFiberInstance` map - which would let us get rid of that map.
In that case the model is resilient to Fiber being in more than one
place at a time.
However this unmount/remount does mean that we can lose selection when
this happens. We could maybe do something like using the tracked path
like I did for component filters. Ideally it's a weird edge case though
because you'd typically not have it. The main case that it happens now
is for reorders of list of server components. In that case basically all
the children move between server components while the server components
themselves stay in place. We should really include the key in server
components so that we can reconcile them using the key to handle
reorders which would solve the common case anyway.
I convert the name to the `Env(Name)` pattern which allows the
Environment Name to be used as a badge.
<img width="1105" alt="Screenshot 2024-08-13 at 9 55 29 PM"
src="https://github.com/user-attachments/assets/323c20ba-b655-4ee8-84fa-8233f55d2999">
(Screenshot is with #30667. I haven't tried it with the alternative
fix.)
---------
Co-authored-by: Ruslan Lesiutin <[email protected]>
There was a comment that it's not safe to walk the unmounted fiber tree
which I'm not sure is correct or not but we need to walk the instance
tree to be able to clean up virtual instances anyway. We already walk
the instance tree to clean up "remaining instances".
This is also simpler because we don't need to special case Suspense
boundaries. We simply clean up whatever branch we had before.
The ultimate goal is to also walk the instance tree for updates so we
don't need a fiber to instance map.
Stacked on #30625 and #30657.
This ensures that we only create instances during the commit
reconciliation and that we don't create unnecessary instances for things
that are filtered or not mounted. This ensures that we also can rely on
the reconciliation to do all the clean up. Now everything is created and
deleted as a pair in the same pass.
Previously we were including unfiltered components in the owner stack
which probably doesn't make sense since you're intending to filter them
everywhere presumably. However, it also means that those links were
broken since you can't link into owners that don't exist in the parent
tree.
The main complication is the component filters. It relied on not
unmounting the old instances. I had to update some tests that asserted
on ids that are now shifted.
For warnings/errors tracking I now restore them back into the pending
set when they unmount. Basically it puts them back into their
"pre-commit" state. That way when they remount they’re still there.
For restoring the current selection I use the tracked path mechanism
instead of relying on the id being unchanged. This is better anyway
because if you filter out the currently selected item it's better to
select the nearest match instead of just losing the selection.
This no longer uses the handleCommitFiberUnmount hook to track unmounts.
Instead, we can just unmount the DevToolsInstances that we didn't reuse.
This doesn't account for cleaning up instances that were unnecessarily
created when they weren't in the tree. I have a separate follow up for
that.
This also removes the queuing of untracking. This was added in #21523
but I don't believe it has been needed for a while because the mentioned
flushPendingErrorsAndWarningsAfterDelay hasn't called untrackFiberID for
a while so the race condition doesn't exist. It's hard to tell though
because from the issues there weren't really any repros submitted.
This is the beginning of a refactor of the DevTools Fiber backend. The
new approach is basically that we listen to each commit from Fiber and
traverse the tree - building up a filtered shadow tree. Then we send
diffs based on that tree and perform our own operations against that
instead of using Fibers as the source of truth.
Fiber diffs Elements -> Fibers. The backend diffs Fibers ->
DevToolsInstances as a side-effect it sends deltas to the front end.
This makes the algorithm resilient to a different Fiber implementation
that doesn't use pairs of Fibers (alternates) but instead stateless new
clones each time. In that world we can't treat Fibers as instances. They
can hold onto instances but they're not necessarily 1:1 themselves.
The same thing also applies to Server Components that don't have their
own instances.
The algorithm is more or less the same as React's reconciliation in
ReactChildFiber itself. However, we do a mutable update of the tree as
we go. We also cheat a bit here in the first version in that we still
have fiberToFiberInstance map and alternate which makes reorders easier.
Further down we could do the reorders by adding the previous set to a
temporary map like ChildFiber does but only if they're not already in
order.
This first bit is just about making sure that we produce correct trees.
We have fairly good test coverage already of that already.
In the next few follow ups I'll start simplifying the rest of the logic
by taking advantage of the new tree.
Follow up from #30584.
You can already select a singleton or hoistable (that's not a resource)
in the browser elements panel and it'll select the corresponding node in
the RDT Components panel. That works because it uses the same mechanism
as event dispatching and those need to be able to receive events.
However, you can't select a resource. Because that's conceptually one to
many.
This keeps track of which fiber is acquiring which resource so we can
find all the corresponding instances.
E.g. now you can select the `<link rel="stylesheet">` in the Flight
fixture in the Element panel and then the component that rendered it in
the Components panel will be selected.
If we had a concept multi-selection we could potentially select all of
them. This similar to how a Server Component can be rendered in more
than one place and if we want to select all matching ones. It's kind of
weird though and both cases are edge cases.
Notably imperative preloads do have elements that don't have any
corresponding component but that's ok. So they'll just select `<head>`.
Maybe in dev we could track the owners of those.
This is just for clarity at first.
Before:
- mountFiberRecursively accepts a set of children and flag that says
whether to just do one
- updateFiberRecursively accepts a fiber and loops over its children
- unmountFiberChildrenRecursively accepts a fiber and loops over its
children
After:
- mountFiberRecursively accepts a Fiber and calls
mountChildrenRecursively
- updateFiberRecursively accepts a Fiber and calls
updateChildrenRecursively
- unmountFiberRecursively accepts a Fiber and calls
unmountChildrenRecursively
- mountChildrenRecursively accepts a set of children and loops over each
one
- updateChildrenRecursively accepts a set of children and loops over
each one
- unmountChildrenRecursively accepts a set of children and loops over
each one
So now there's one place where things happens for the single item and
one place where we do the loop.
Basically the new Float types needs to be supported. Resources are a bit
special because they're a DOM specific type but we can expect any other
implementation using resources to provide and instance on this field if
needed.
There's a slightly related case for the reverse lookup. You can already
select a singleton or hoistable (that's not a resource) in the browser
elements panel and it'll select the corresponding node in the RDT
Components panel. That works because it uses the same mechanism as event
dispatching and those need to be able to receive events.
However, you can't select a resource. Because that's conceptually one to
many. We could in principle just search the tree for the first one or
keep a map of currently mounted resources and just pick the first fiber
that created it. So that you can select a resource and see what created
it. Particularly useful when there's only one Fiber which is most of the
time.
---------
Co-authored-by: Ruslan Lesiutin <[email protected]>
This just tracks the `.parent` field properly and uses DevToolsInstances
in more places that used to use IDs or Fibers.
I also use this new parent path when looking up a DevToolsInstance from
a DOM node. This should ideally be simple because the `.parent` field
represents only the unfiltered parents and include any virtual parents.
So we should be able to just get one from nearest Fiber that has one.
However, because we don't currently always clean up the map of
DevToolsInstances (e.g. updateComponentFilters doesn't recursively clean
out everything) it can leave matches hanging that shouldn't be there. So
we need to run the shouldFilterFiber filter to ignore those.
Another interesting implication is that without a FiberInstance we don't
have a way to get to a VirtualInstance from a HostComponent. Which means
that even filtered Fibers need to have a FiberInstance if they have a
VirtualInstance parent. Even if we don't actually mount them into the
front-end.
Stacked on #30494 and #30491.
This is setting us up to be able to track Server Components. This is now
split into a FiberInstance (Client Component) and a VirtualInstance
(Server Component). We're not actually creating any VirtualInstances yet
though this is just the data structures.
Server Components and potentially other compiled away or runtime
optimized away (e.g. calling through a function without creating an
intermediate Fiber) don't have a stateful instance. They just represent
the latest data. It's kind of like a React Element.
However, in the DevTools UI we need them to be stateful partly just so
that you can select and refer to them separately. E.g. the same Server
Component output rendered into different slots on the client should
still have two different representations in the DevTools. Also if the
same child Fibers update in place because the Server Component refreshed
we shouldn't lose the selection if you've selected a Server Component.
I'll implement this by creating Virtual Instances that only exist for
the purpose of the DevTools UI and so it'll be implemented in the
DevTools.
We could just make a Map from `id` to `Fiber | ReactComponentInfo` but
that requires a branching without a consistent hidden class. We also
need some more states on there. We also have some other Maps that tracks
extra states like that of component stacks, errors and warnings.
Constantly resizing and adding/removing from a Map isn't exactly fast.
It's faster to have a single Map with an object in it than one Map per
object. However, having extra fields that are usually just `null` can
instead mean more memory gets used. Since only a very small fraction of
instances will have errors/warnings or having initialized its component
stack, it made sense to store that in a separate Map that is usually
just empty.
However, with the addition of particularly the `parent` field and the
ability to do a fast hidden-class safe branching on the `kind` field I
think it's probably worth actually allocating an extra first class
object per Instance to store DevTools state into. That's why I converted
from just storing `Fiber` -> `id` to storing `Fiber` ->
`DevToolsInstance` which then keeps the warnings/errors/componentStack
as extra fields that are usually `null`. That is a lot of objects though
since it's one per Fiber pair basically.
Stacked on #30491.
When going from DOM Node to select a component or highlight a component
we find the nearest mounted ancestor. However, when multiple renderers
are nested there can be multiple ancestors. The original fix#24665 did
this by taking the inner renderer if it was an exact match but if it
wasn't it just took the first renderer.
Instead, we can track the inner most node we've found so far. Then get
the ID from that node (which will be fast since it's now a perfect
match). This is a better match.
However, the main reason I'm doing this is because the old mechanism
leaked the `Fiber` type outside the `RendererInterface` which is
supposed to abstract all of that. With the new algorithm this doesn't
leak.
I've tested this with a new build against the repro in the old issue
#24539 and it seems to work.
Stacked on #30490.
This is in the same spirit but to clarify the difference between what is
React Native vs part of any generic Host. We used to use "Native" to
mean three different concepts. Now "Native" just means React Native.
E.g. from the frontend's perspective the Host can be
Highlighted/Inspected. However, that in turn can then be implemented as
either direct DOM manipulation or commands to React Native. So frontend
-> backend is "Host" but backend -> React Native is "Native" while
backend -> DOM is "Web".
Rename NativeElementsPanel to BuiltinElementsPanel. This isn't a React
Native panel but one part of the surrounding DevTools. We refer to Host
more as the thing running React itself. I.e. where the backend lives.
The runtime you're inspecting. The DevTools itself needs a third term.
So I went with "Builtin".
I need to start clarifying where things are really actually Fibers and
where they're not since I'm adding Server Components as a separate type
of component instance which is not backed by a Fiber.
Nothing in the front end should really know anything about what kind of
renderer implementation we're inspecting and indeed it's already not
always a "Fiber" in the legacy renderer.
We typically refer to this as a "Component Instance" but the front end
currently refers to it as an Element as it historically grew from the
browser DevTools Elements tab.
I also moved the renderer.js implementation into the `backend/fiber`
folder. These are at the same level as `backend/legacy`. This clarifies
that anything outside of this folder ideally shouldn't refer to a
"Fiber".
console.js and profilingHooks.js unfortunately use Fibers a lot which
needs further refactoring. The profiler frontend also uses the term
alot.
