From 0f83a64eda73e93135b2d30531b10427fccc4c05 Mon Sep 17 00:00:00 2001
From: Andrew Clark <git@andrewclark.io>
Date: Wed, 18 Nov 2020 10:33:26 -0600
Subject: [PATCH] Regression test: Missing unmount after re-order (#20285)

Adds a regression test for a bug I found in the effects refactor.

The bug was that reordering a child that contains passive effects would
cause the child to "forget" that it contains passive effects. This is
because when a Placement effect is scheduled by the reconciler, it would
override all of the fiber's flags, including its "static" ones:

```
child.flags = Placement;
```

The problem is that we use a static flag to use a "static" flag to track
that a fiber contains passive effects.

So what happens is that when the tree is deleted, the unmount effect is
never fired.

In the new implementation, the fix is to add the Placement flag without
overriding the rest of the bitmask:

```
child.flags |= Placement;
```

(The old implementation doesn't need to be changed because it does not
use static flags for this purpose.)
---
 .../ReactHooksWithNoopRenderer-test.js        | 46 +++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
index 1a7e3d1f5fe9..43ec803f1155 100644
--- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
@@ -3370,4 +3370,50 @@ describe('ReactHooksWithNoopRenderer', () => {
     });
     expect(Scheduler).toHaveYielded(['Unmount layout B', 'Unmount passive B']);
   });
+
+  it('regression: deleting a tree and unmounting its effects after a reorder', async () => {
+    const root = ReactNoop.createRoot();
+
+    function Child({label}) {
+      useEffect(() => {
+        Scheduler.unstable_yieldValue('Mount ' + label);
+        return () => {
+          Scheduler.unstable_yieldValue('Unmount ' + label);
+        };
+      }, [label]);
+      return label;
+    }
+
+    await act(async () => {
+      root.render(
+        <>
+          <Child key="A" label="A" />
+          <Child key="B" label="B" />
+        </>,
+      );
+    });
+    expect(Scheduler).toHaveYielded(['Mount A', 'Mount B']);
+
+    await act(async () => {
+      root.render(
+        <>
+          <Child key="B" label="B" />
+          <Child key="A" label="A" />
+        </>,
+      );
+    });
+    expect(Scheduler).toHaveYielded([]);
+
+    await act(async () => {
+      root.render(null);
+    });
+
+    expect(Scheduler).toHaveYielded([
+      'Unmount B',
+      // In the regression, the reorder would cause Child A to "forget" that it
+      // contains passive effects. Then when we deleted the tree, A's unmount
+      // effect would not fire.
+      'Unmount A',
+    ]);
+  });
 });