Let's schedule the passive effects even earlier (#16714)
It turns out I needed to schedule mine in the mutation phase and there are also clean up life-cycles there.
Sebastian Markbåge committed
Sep 9, 2019 at 15:06 UTC
440cbf2ee57420bd36ef90aaae19e6542614677a
2 files changed
+31
-13
packages/react-reconciler/src/ReactFiberWorkLoop.js
+13
-12
@@ -1819,7 +1819,8 @@ function commitRootImpl(root, renderPriorityLevel) {
1819
1820
function commitBeforeMutationEffects() {
1821
while (nextEffect !== null) {
1822
- if ((nextEffect.effectTag & Snapshot) !== NoEffect) {
1822
+ const effectTag = nextEffect.effectTag;
1823
+ if ((effectTag & Snapshot) !== NoEffect) {
1824
setCurrentDebugFiberInDEV(nextEffect);
1825
recordEffect();
1826
@@ -1828,6 +1829,17 @@ function commitBeforeMutationEffects() {
1829
1830
resetCurrentDebugFiberInDEV();
1831
}
1832
+ if ((effectTag & Passive) !== NoEffect) {
1833
+ // If there are passive effects, schedule a callback to flush at
1834
+ // the earliest opportunity.
1835
+ if (!rootDoesHavePassiveEffects) {
1836
+ rootDoesHavePassiveEffects = true;
1837
+ scheduleCallback(NormalPriority, () => {
1838
+ flushPassiveEffects();
1839
+ return null;
1840
+ });
1841
+ }
1842
+ }
1843
nextEffect = nextEffect.nextEffect;
1844
}
1845
}
@@ -1850,17 +1862,6 @@ function commitMutationEffects(root: FiberRoot, renderPriorityLevel) {
1862
}
1863
}
1864
1853
- if (effectTag & Passive) {
1854
- // If there are passive effects, schedule a callback to flush them.
1855
- if (!rootDoesHavePassiveEffects) {
1856
- rootDoesHavePassiveEffects = true;
1857
- scheduleCallback(NormalPriority, () => {
1858
- flushPassiveEffects();
1859
- return null;
1860
- });
1861
- }
1862
- }
1863
-
1865
// The following switch statement is only concerned about placement,
1866
// updates, and deletions. To avoid needing to add a case for every possible
1867
// bitmap value, we remove the secondary effects from the effect tag and
packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.internal.js
+18
-1
@@ -281,13 +281,30 @@ describe('ReactSchedulerIntegration', () => {
281
});
282
return null;
283
}
284
+ function CleanupEffect() {
285
+ useLayoutEffect(() => () => {
286
+ Scheduler.unstable_yieldValue('Cleanup Layout Effect');
287
+ Scheduler.unstable_scheduleCallback(NormalPriority, () =>
288
+ Scheduler.unstable_yieldValue(
289
+ 'Scheduled Normal Callback from Cleanup Layout Effect',
290
+ ),
291
+ );
292
+ });
293
+ return null;
294
+ }
295
+ await ReactNoop.act(async () => {
296
+ ReactNoop.render(<CleanupEffect />);
297
+ });
298
+ expect(Scheduler).toHaveYielded([]);
299
await ReactNoop.act(async () => {
300
ReactNoop.render(<Effects />);
301
});
302
expect(Scheduler).toHaveYielded([
303
+ 'Cleanup Layout Effect',
304
'Layout Effect',
305
'Passive Effect',
290
- // This callback should be scheduled after the passive effects.
306
+ // These callbacks should be scheduled after the passive effects.
307
+ 'Scheduled Normal Callback from Cleanup Layout Effect',
308
'Scheduled Normal Callback from Layout Effect',
309
]);
310
});