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

Add built-in Suspense cache with support for invalidation (refreshing) #20456

Merged
merged 30 commits into from
Dec 18, 2020

Commits on Dec 18, 2020

  1. Initial scaffolding for <Cache />

    - Add Cache component type to React exports
    - Add internal work tag
    - Test that it can render children
    
    No functionality yet.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    2d2a65b View commit details
    Browse the repository at this point in the history
  2. Implement getCacheForType

    Makes Cache act like a context provider. Along with `getCacheForType`,
    this unlocks basic data fetching.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    1874591 View commit details
    Browse the repository at this point in the history
  3. Use same cache for all new data in a single update

    If multiple Cache boundaries mount at the same time, they should use the
    same cache, even if they are in totally separate trees.
    
    The plan is to extend this further so that we keep reusing the same
    cache for all incoming updates until one of them finishes. (Not yet
    implemented.)
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    f9b829d View commit details
    Browse the repository at this point in the history
  4. Retain in-progress caches on the root, per lane

    All the data that loaded as a result of a single transition/update
    should share the same cache. This includes nested content that gets
    progressively "filled in" after the initial shell is displayed.
    
    If the shell itself were wrapped in a Cache boundary, such that the
    cache can commit with suspending, then this is easy: once the boundary
    mounts, the cache is attached the React tree.
    
    The tricky part is when the shell does not include a cache boundary. In
    the naive approach, since the cache is not part of the initial tree, it
    does not get retained; during the retry, a fresh cache is created,
    leading to duplicate requests and possibly an infinite loop as requests
    are endlessly created then discarded.
    
    This is the essential problem we faced several years ago when building
    Simple Cache Provider (later the react-cache package).
    
    Our solution is to retain in-flight caches on the root, associated by
    lane. The cache cleared from the root once all of the lanes that depend
    on it finish rendering.
    
    Because progressively rendering nested boundaries ("retry" updates) uses
    a different lane from the update that spawned it, we must take extra
    care to transfer the cache to the new lane when scheduling the retry.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    57d6c1d View commit details
    Browse the repository at this point in the history
  5. Cache refreshing

    Implements useRefresh, a method to invalidate the cache and request
    new data.
    
    It will find the nearest <Cache /> boundary, clear its cache, and
    schedule an update to re-render with fresh data.
    
    We had discussed calling this method `useCacheInvalidation`. The problem
    I have with that name is that it is bad. I went with `useRefresh`
    because it, by contrast, is good.
    
    One might object is that it clashes with the name for "Fast Refresh" but
    I disagree.
    
    It's experimental anyway so we can bikeshed the name before release.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    893a831 View commit details
    Browse the repository at this point in the history
  6. Refresh with seeded data

    Usually, when performing a server mutation, the response includes an
    updated version of the mutated data.
    
    This avoids an extra roundtrip, and because of eventual consistency, it
    also guarantees that we reload with the freshest possible data. If we
    didn't seed with the mutation response, and instead refetched with a
    separate GET request, we might receive stale data as the mutation
    propagates through the data layer.
    
    Not all refreshes are the result of a mutation, though, so the seed is
    not required.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    eabf255 View commit details
    Browse the repository at this point in the history
  7. Refreshes should not affect "sibling" boundaries

    I had thought we decided that refreshing a boundary would also refresh
    all the content that is currently consistent (i.e. shared the same
    underlying cache) with it, but I was wrong. Refreshing should only
    affect the nearest tree and its descendents. "Sibling" content will
    intentionally be inconsistent after the refresh.
    
    This allows me to drop the subscription stuff, which is nice.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    c40683a View commit details
    Browse the repository at this point in the history
  8. Add implicit root-level cache

    If `getCacheForType` or `useRefresh` cannot find a parent <Cache />,
    they will access a top-level cache associated with the root. The
    behavior is effectively the same as if you wrapped the entire tree in a
    <Cache /> boundary.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    220a9d7 View commit details
    Browse the repository at this point in the history
  9. Make CacheContext type non-nullable

    Now that the root is a cache provider, there will always be a cache.
    Don't need the null check.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    da9dc4c View commit details
    Browse the repository at this point in the history
  10. Use an update queue for refreshes

    Refreshes are easier than initial mounts because we have a mounted fiber
    that we can attach the cache to. We don't need to rely on clever pooling
    tricks; they're just normal updates.
    
    More importantly, we're not at risk of dropping requests/data if we run
    out of lanes, which is especially important for refreshes because they
    can contain data seeded from a server mutation response; we cannot
    afford to accidentally evict it.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    bc43607 View commit details
    Browse the repository at this point in the history
  11. Add fast path for nested mounting Caches

    Only the top most Cache boundary in a newly mounting tree needs to
    call `requestFreshCache`. Nested caches can inherit the parent cache
    by reading from context.
    
    This is strictly a performance optimization, since `requestFreshCache`
    would return the parent cache, anyway.
    
    After our planned lanes refactor, `requestFreshCache` might be fast
    enough that we don't need this special fast path.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    900af34 View commit details
    Browse the repository at this point in the history
  12. Every request in initial render shares same cache

    I noticed this bug when writing some new tests.
    
    The HostRoot fiber is special because it's always mounted. The moment
    you call `createRoot()`, you have a "current" (albeit empty) tree. So
    the "initial render" of an app is actually implemented internally as an
    update. This model has some nice advantages because we can use a regular
    Fiber and regular update queue to manage in-progress work even before
    the initial commit.
    
    However, for the purposes of the cache, we want the initial render to be
    treated like an initial render: all requests should share the same
    cache, including nested boundaries. My trick of checking if the provider
    fiber has an alternate won't work, because the root fiber always has an
    alternate.
    
    So I use another trick: if the provider fiber is a host root, check if
    `memoizedState.element` is null. We also check the alternate. The
    work-in-progress fiber's `element` will never be null because we're
    inside a work-in-progress tree. So if either fiber's element is null,
    that fiber must be the current one, which most likely means it's the
    initial mount.
    
    (I say "most likely" because, technically, you could pass `null` to
    `root.render()`. But, meh, good enough.)
    
    Fixing this revealed a related bug in one of my tests. When you render
    the initial app, all the caches on the entire page share the same
    provider: the root. So a refresh anywhere in the UI will refresh the
    entire screen... until you navigate or reveal more content. The more you
    interact with the UI, the more granular the consistency boundaries get.
    
    I also found another bug where caches were not transfered across retries
    if the original update was spawned by a cache refresh. That's because
    refresh caches are stored in the provider's update queue; we don't track
    these on these on the root, because they're already part of the tree.
    
    So for these types of retries, we can go back to the original trick I
    attempted at the beginning of this exploration: when mounting a new
    tree, consult the render lanes to see if it's a retry. If it is, and
    there's no cache associated with those lanes, then the retry must have
    been the result of a cache refresh. Which means the nearest cache
    provider must be the one that we want: the one that refreshed.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    3cb4936 View commit details
    Browse the repository at this point in the history
  13. pushCacheProvider/popCacheProvider

    Extracted these into functions so I can put more stuff there.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    7485268 View commit details
    Browse the repository at this point in the history
  14. Add warnings if cache context is in invalid state

    A parent cache refresh always overrides any nested cache. So there will
    only ever be a single fresh cache on the context stack.
    
    We can use this knowledge to detect stack mismatch bugs.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    23d46bf View commit details
    Browse the repository at this point in the history
  15. Explicitly check if the parent provider is fresh

    We can track this cheaply because there's only ever a single fresh
    provider. We don't need to store the fresh caches on the stack, just one
    if it exists. Then when we unwind the Fiber stack, we reset it.
    
    Bonus: this replaces the fast path I added to detect fresh roots, too.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    0d605b1 View commit details
    Browse the repository at this point in the history
  16. Restore retry cache from Suspense/Offscreen fiber

    When a Suspense or Offscreen boundary resumes rendering, the inner tree
    should use the same cache that the outer one did during the original
    render. This is important not just for UI consistency reasons, but
    because dropping the original cache means dropping all the in-
    flight requests.
    
    This is arguably an edge case, because it only applies to the first
    Cache boundary in the new tree is not part of its "shell" — that is,
    if it's inside the first Suspense boundary, and isn't committed in the
    first render. But we should still try to get it right.
    
    Previously I was using an array on the root (the one we use for
    tracking caches that aren't yet rooted to the tree) but with that
    approach you quickly run out of lanes.
    
    The new approach is to store the cache on the Offscreen fiber. Suspense
    uses an Offscreen fiber internally to wrap its children, so the code is
    almost entirely shared.
    
    A neat property is that we only have to store a single cache per
    fallback/hidden tree. I had previously expected that we'd need to store
    a cache per retry lane per tree. But, when unhiding a hidden tree, the
    retry lanes must be entangled — that was the discovery we made when
    fixing the "flickering" bug earlier in the year. So we actually only
    need a single retry cache per hidden Suspense/Offscreen boundary. Even
    setting aside entanglement, the only reason you'd have multiple is if
    there were multiple parent refreshes, in which case the last one should
    win regardless. The important thing is that each separate tree can have
    their own, which this approach achieves.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    65458e4 View commit details
    Browse the repository at this point in the history
  17. Code size optimizations

    - Wraped more things in the feature flag.
    - Removed CacheComponent cases from commit phase.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    c599f98 View commit details
    Browse the repository at this point in the history
  18. Remove useRefresh from unstable-shared-subset

    From @sebmarkbage's comment
    
    > This should not be included in this file. Which means that the error
    > the dispatcher throws should never be reachable, but worth keep in
    > case something is misconfigured or tries use internals.
    >
    > This ensures that statically, we can say that a shared/server component
    > can't import this at all so there's no risk of accidentally using it and
    > it's a signal that a client component is needed.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    27c30e1 View commit details
    Browse the repository at this point in the history
  19. Previous retry cache takes precedence over pool

    When committing a fallback, if there's no cache on the stack, check if
    there's a cache from the previous render. This is what we would have
    used for new content during the first pass when we attempted to unhide.
    
    If there's no previous cache, then we can check the pool. If a nested
    cache accessed the pool, it would have been assigned
    to `root.pooledCache`.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    ddaa1f1 View commit details
    Browse the repository at this point in the history
  20. To resume pooled cache, override root.pooledCache

    When retrying with a pooled cache resumed from the first render, we
    can't put the cache on the regular Suspense context, because it will
    override nested refreshes. We have to track it on a different conceptual
    context stack. Currently that's `root.pooledCache`. So my solution is to
    overwrite that field when we enter the nested subtree. (This might be too
    clever and I should put it on a stack cursor instead. Regardless, it
    doesn't change much about the structure of code.)
    
    I originally noticed this issue because the type of `root.pooledCache`
    was `{cache: Cache, provider: Fiber}` — pooled caches do not have
    providers!
    
    This fix partially depends on a planned change to get rid of the
    lane-indexed cache pool and always read from `root.pooledCache`. I'll do
    that in the next commit.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    79c65dc View commit details
    Browse the repository at this point in the history
  21. Use only a single pooled cache at a time

    The cache-per-lane approach makes conceptual sense but it's probably not
    worth it until we complete the Lanes entanglement refactor. In the
    current implementation we have to do lots of looping to maintain the
    pool. And most transitions get batched together, anyway.
    
    We'll re-evaluate later.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    52fd1eb View commit details
    Browse the repository at this point in the history
  22. useRefresh -> useCacheRefresh

    useRefresh is probably too general. We may also add additional APIs
    related to the cache, and including the word "cache" in all of them
    signals they are related.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    f4e2124 View commit details
    Browse the repository at this point in the history
  23. More tests

    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    e451117 View commit details
    Browse the repository at this point in the history
  24. Wrap more things in flag

    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    8e0b756 View commit details
    Browse the repository at this point in the history
  25. Remove default transition priority for refreshes

    The flaw here is that if another update in the same event is not wrapped
    in `startTransition`, then it won't be batched with the refresh. The
    solution is to wrap both in the same `startTransition`. It's worse for
    them not to be batched then for the batch to have too high a priority.
    
    We'll consider adding a warning.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    eed9285 View commit details
    Browse the repository at this point in the history
  26. Fork ReactFiberCacheComponent

    Split into `new` and `old` reconciler files
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    9894eb2 View commit details
    Browse the repository at this point in the history
  27. Only mutate root.pooledCache in complete/unwind

    Less indirection when accessing during the render phase and less hard
    to make a mutation mistake.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    ea6700c View commit details
    Browse the repository at this point in the history
  28. Remove provider fiber from cache context

    I originally put the provider fiber on the cache context because I
    expected the semantics to be that a refresh finds the root of the
    "freshness" boundary; that is, all the data that refreshed or appeared
    as part of the same transition.
    
    Refreshing is a tricky problem space that we're not done exploring; the
    better default behavior is to refresh the most local provider, without
    considering freshness.
    
    This also makes the implementation simpler because `refresh` no longer
    needs to be bound to the provider fiber. Instead I traverse up the
    fiber return path.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    3cfe44e View commit details
    Browse the repository at this point in the history
  29. Detect refreshes by comparing to previous parent

    Removes the fresh/stale distinction from the context stack and instead
    detects refreshes by comparing the previous and next parent cache.
    
    This is closer to one of the earlier implementation drafts, and it's
    essentially how you'd implement this in userspace using context. I had
    moved away from this when I got off on a tangent thinking about how the
    cache pool should work; once that fell into place, it became more clear
    what the relationship is between the context stack, which you use for
    updates ("Here"), and the cache pool, which you use for newly mounted
    content ("There").
    
    The only thing we're doing internally that can't really be achieved in
    userspace is transfering the cache across Suspense retries. Kinda neat.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    4bb19da View commit details
    Browse the repository at this point in the history
  30. Fix cross-fork discrepancy

    I missed a few lines when syncing an earlier step. Usually I would find
    which one and patch it but I'm about to squash and merge so meh.
    acdlite committed Dec 18, 2020
    Configuration menu
    Copy the full SHA
    7c7d2ba View commit details
    Browse the repository at this point in the history