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.

Sign up for GitHub

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

Jump to bottom

[Bugfix] Dropped updates inside a suspended tree #18384

Merged
merged 6 commits into from Mar 26, 2020
Merged

[Bugfix] Dropped updates inside a suspended tree #18384

merged 6 commits into from Mar 26, 2020

Commits on Mar 26, 2020

  1. Minor test refactor: resolveText 

    Adds a `resolveText` method as an alternative to using timers. Also
removes dependency on react-cache (for just this one test file; can do
the others later).

Timer option is still there if you provide a `ms` prop.
    @acdlite
    acdlite committed Mar 26, 2020
    Copy the full SHA
    6e312cf View commit details
    Browse the repository at this point in the history

  2. Bugfix: Dropped updates in suspended tree 

    When there are multiple updates at different priority levels inside
a suspended subtree, all but the highest priority one is dropped after
the highest one suspends.

We do have tests that cover this for updates that originate outside of
the Suspense boundary, but not for updates that originate inside.

I'm surprised it's taken us this long to find this issue, but it makes
sense in that transition updates usually originate outside the boundary
or "seam" of the part of the UI that is transitioning.
    @acdlite
    acdlite committed Mar 26, 2020
    Copy the full SHA
    2eab95a View commit details
    Browse the repository at this point in the history

  3. Bugfix: Suspense fragment skipped by setState 

    Fixes a bug where updates inside a suspended tree are dropped because
the fragment fiber we insert to wrap the hidden children is not part of
the return path, so it doesn't get marked during setState.

As a workaround, I recompute `childExpirationTime` right before deciding
to bail out by bubbling it up from the next level of children.

This is something we should consider addressing when we refactor the
Fiber data structure.
    @acdlite
    acdlite committed Mar 26, 2020
    Copy the full SHA
    5a0f1d5 View commit details
    Browse the repository at this point in the history

  4. Add back lastPendingTime field 

    This reverts commit 9a54113.

I want to use this so we can check if there might be any lower priority
updates in a suspended tree.

We can remove it again during the expiration times refactor.
    @acdlite
    acdlite committed Mar 26, 2020
    Copy the full SHA
    9ebadfe View commit details
    Browse the repository at this point in the history

  5. Use lastPendingTime instead of Idle 

    We don't currently have an mechanism to check if there are lower
priority updates in a subtree, but we can check if there are any in the
whole root. This still isn't perfect but it's better than using Idle,
which frequently leads to redundant re-renders.

When we refactor `expirationTime` to be a bitmask, this will no longer
be necessary because we'll know exactly which "task bits" remain.
    @acdlite
    acdlite committed Mar 26, 2020
    Copy the full SHA
    12d275e View commit details
    Browse the repository at this point in the history

  6. Add a test for updating the fallback

    @acdlite
    acdlite committed Mar 26, 2020
    Copy the full SHA
    ae27d3a View commit details
    Browse the repository at this point in the history