Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Regression test: SuspenseList causes lost unmount (facebook#20433)
@sebmarkbage reminded me that the complete phase of SuspenseList will sometimes enter the begin phase of the children without calling `createWorkInProgress` again, instead calling `resetWorkInProgress`. This was raised in the context of considering whether facebook#20398 might have accidentally caused a SuspenseList bug. (I did look at this code at the time, but considering how complicated SuspenseList is it's not hard to imagine that I made a mistake.) Anyway, I think that PR is fine; however, reviewing it again did lead me to find a different bug. This new bug is actually a variant of the bug fixed by facebook#20398. `resetWorkInProgress` clears a fiber's static flags. That's wrong, since static flags -- like PassiveStatic -- are meant to last the lifetime of the component. In more practical terms, what happens is that if a direct child of SuspenseList contains a `useEffect`, then SuspenseList will cause the child to "forget" that it contains passive effects. When the child is deleted, its unmount effects are not fired :O This is the second of this type of bug I've found, which indicates to me that it's too easy to accidentally clear static flags. Maybe we should only update the `flags` field using helper functions, like we do with `lanes`. Or perhaps we add an internal warning somewhere that detects when a fiber has different static flags than its alternate.
- Loading branch information