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

Fix: LegacyHidden should not toggle effects #21928

Merged
merged 1 commit into from
Jul 21, 2021

Commits on Jul 21, 2021

  1. Fix: LegacyHidden should not toggle effects

    LegacyHidden is a transitional API that we added to replace the old
    `<div hidden={true} />` API that we used to use for pre-rendering. The
    plan is to replace this with the Offscreen component, once it's ready.
    
    The idea is that LegacyHidden has identical behavior to Offscreen except
    that it doesn't change the behavior of effects. (Which is basically how
    `<div hidden={true} />` worked — it prerendered the hidden content in
    the background, but nothing else.) That way, while we're rolling this
    out, we could toggle the feature behind a feature flag either for
    performance testing or as a kill switch.
    
    It looks like we accidentally enabled the effects flag for both
    Offscreen _and_ LegacyHidden. I suppose it's a good thing that nobody
    has complained yet, since we eventually do want to ship this
    behavior everywhere?
    
    But I do think we should remove it from LegacyHidden, and roll it out by
    gating the component type in the downstream repo. That way if there's an
    issue related to the use of LegacyHidden, we can disable that without
    disabling the behavior for Suspense boundaries.
    
    In retrospect, I might have implemented this as an unstable prop on
    Offscreen instead of a completely separate type — though at the time,
    Offscreen didn't exist. I originally added LegacyHidden to unblock the
    Lanes refactor, so I could move the deprioritization logic out of the
    HostComponent implementation.
    
    Not a big deal since we're going to remove this soon. The implementation
    is almost the same regardless: before disconnecting or reconnecting
    the effects, check the fiber tag. The rest of the logic is the same.
    acdlite committed Jul 21, 2021
    Configuration menu
    Copy the full SHA
    93b9f9e View commit details
    Browse the repository at this point in the history