[Fabric] Use container node to toggle the visibility of Offscreen and Suspense trees #21960

Fixes an oversight from a previous refactor. The fiber that wraps a Suspense component's children used to be a Fragment but now it's on Offscreen fiber, so its props type has changed. There's a special hydration path where I forgot to update this. This isn't observable because we don't ever end up rendering this particular fiber (because the Suspense boundary is in its fallback state) but we should fix it anyway to avoid a potential regression in the future.

...into a new method called `createWorkInProgressOffscreenFiber`. Just for symmetry with `updateWorkInProgressOffscreenFiber`. Doesn't change any behavior.

This changes how we hide and show the contents of Offscreen boundaries in the React Fabric renderer (persistent mode), and also Suspense boundaries which use the same feature.= The way it used to work was that when a boundary is hidden, in the complete phase, instead of calling the normal `cloneInstance` method inside `appendAllChildren`, we would call a forked method called `cloneHiddenInstance` for each of the nearest host nodes within the subtree. This design was largely based on how it works in React DOM (mutation mode), where instead of cloning the nearest host nodes, we mutate their `style.display` property. The motivation for doing it this way in React DOM was because there's no built-in browser API for hiding a collection of DOM nodes without affecting their layout. In Fabric, however, there is no such limitation, so we can instead wrap in an extra host node and apply a hidden style. The immediate motivation for this change is that Fabric on Android has a view pooling mechanism for instances that relies on the assumption that a current Fiber that is cloned and replaced by a new Fiber will never appear in a future commit. When this assumption is broken, it may cause crashes. In the current implementation, that can indeed happen when a node that was previously hidden is toggled back to visible. Although this change sidesteps the issue, we may introduce in other features in the future that would benefit from being able to revert back to an older node without cloning it again, such as animations. The way I've implemented this is to insert an additional HostComponent fiber as the child of each OffscreenComponent. The extra fiber is not ideal — the way I'd prefer to do it is to attach the host instance to the OffscreenComponent. However, the native Fabric implementation currently expects a 1:1 correspondence between HostComponents and host instances, so I've deferred that optimization to a future PR to derisk fixing the Fabric pooling crash. I left a TODO in the host config with a description of the remaining steps, but this alone should be sufficient to unblock.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fabric] Use container node to toggle the visibility of Offscreen and Suspense trees #21960

[Fabric] Use container node to toggle the visibility of Offscreen and Suspense trees #21960

Commits on Jul 26, 2021