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: Detach deleted fiber's alternate, too #20587

Merged
merged 1 commit into from
Jan 14, 2021

Commits on Jan 14, 2021

  1. Fix: Detach deleted fiber's alternate, too

    We need to call `detachFiberAfterEffects` on the fiber's alternate to
    clean it up. We're currently not, because the `alternate` field is
    cleared during `detachFiberMutation`. So I deferred detaching the
    `alternate` until the passive phase. Only the `return` pointer needs to
    be detached for semantic purposes.
    
    I don't think there's any good way to test this without using
    reflection. It's not even observable using out our "supported"
    reflection APIs (`findDOMNode`), or at least not that I can think of.
    Which is a good thing, in a way.
    
    It's not really a memory leak, either, because the only reference to the
    alternate fiber is from the parent's alternate. Which will be
    disconnected the next time the parent is updated or deleted.
    
    It's really only observable if you mess around with internals in ways
    you're not supposed to — I found it because a product test in www that
    uses Enzyme was doing just that.
    
    In lieu of a new unit test, I confirmed this patch fixes the broken
    product test.
    acdlite committed Jan 14, 2021
    Configuration menu
    Copy the full SHA
    e522991 View commit details
    Browse the repository at this point in the history