Skip to content
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

[Fizz] improve Hoistable handling for Elements and Resources inside Suspense Boundaries #28069

Merged
merged 3 commits into from
Jan 30, 2024

Commits on Jan 23, 2024

  1. Refactor change to convey boundary state is not specific to resources

    Hoistable elements and resources in fallbacks should not flush unless the fallback itself flushes.
    
    Previously we used a renderState-local binding to track the resources for the currently rendering boundary. Instead we now track this in the task itself and pass it to the functions that depend on it. This cleans up an unfortunate factoring that I put in before where during flushing we had to mimic the currently rendering Boundary to hoist correctly. This new factoring does the same thing but in a much clearer way.
    
    I do track the Boundary state separately from the Boundary itself on the task as a hot path optimization to avoid having to existence check the boundary in `pushStartInstance`. Conceptually tracking the boundary itself is sufficient but this eliminates an extra condition check.
    
    The implementation ended up not merging the boundary resource concept with hoistable state due to the very different nature in which these things need to hoist (one during task completion and the other during flush but only when flushing late boundaries). Given that I've gone back to resource specific naming rather than calling it BoundaryState.
    gnoff committed Jan 23, 2024
    Configuration menu
    Copy the full SHA
    e443eb1 View commit details
    Browse the repository at this point in the history

Commits on Jan 24, 2024

  1. Uses a simpler approach to trackign hoistables and only writes them e…

    …ither during the preamble or when a boundary complets. The cost is that during the preamble flush there is an extra traversal of the segments that will write in the preamble. While this is not ideal for perf it does make the tracking significantly easier to follow and overall probalby strikes a better balance of practicality and maintainability over raw performance.
    gnoff committed Jan 24, 2024
    Configuration menu
    Copy the full SHA
    852f521 View commit details
    Browse the repository at this point in the history

Commits on Jan 25, 2024

  1. Updates the approach to emit hoistables eagerly but only if the hoist…

    …able is not in a fallback tree. Effectively it makes hoistable elements deep in any fallback noops. The logic here is that fallbacks aren't hydrated, they're always either replaced by the server or client rendered. In either case the primary content should be replacing the fallback hositables and since hoistable elements are really just metadata the imperative need to transiently render them is not as well justified. This avoids a host of complexities around deleting hoistables from fallbacks when the boundary reveals it's content.
    
    Along with this change I made it so primary content hositables flush eagerly and do not wait for their containing boundary to complete first. This is a progmatic solution to the problem of prerendering. when prerendering we assume that all transent state is entirely flushed in the static prelude however by holding back hoistables until the boundary is complete this is violated. We would either need to conditionally emit hoistables for incomplete boundaries when prerending or we would need to serialize the state of the hoistables to recover them on the resuem path. The arguments for holding them back are that it aligns with client hoistable semantics better and if the boundary never hydrates the hoistables will get orphaned. Both of these problems are not insignificant but are also not necessarily blockers and so this approach attempts to balance complexity over impact by emitting them eagerly.
    gnoff committed Jan 25, 2024
    Configuration menu
    Copy the full SHA
    eed492e View commit details
    Browse the repository at this point in the history