Skip to content

Commit 21820cc

Browse files
committed
Let's schedule the passive effects even earlier
It turns out I needed to schedule mine in the mutation phase and there are also clean up life-cycles there.
1 parent cc2492c commit 21820cc

File tree

2 files changed

+31
-13
lines changed

2 files changed

+31
-13
lines changed

packages/react-reconciler/src/ReactFiberWorkLoop.js

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1819,7 +1819,8 @@ function commitRootImpl(root, renderPriorityLevel) {
18191819

18201820
function commitBeforeMutationEffects() {
18211821
while (nextEffect !== null) {
1822-
if ((nextEffect.effectTag & Snapshot) !== NoEffect) {
1822+
const effectTag = nextEffect.effectTag;
1823+
if ((effectTag & Snapshot) !== NoEffect) {
18231824
setCurrentDebugFiberInDEV(nextEffect);
18241825
recordEffect();
18251826

@@ -1828,6 +1829,17 @@ function commitBeforeMutationEffects() {
18281829

18291830
resetCurrentDebugFiberInDEV();
18301831
}
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+
}
18311843
nextEffect = nextEffect.nextEffect;
18321844
}
18331845
}
@@ -1850,17 +1862,6 @@ function commitMutationEffects(root: FiberRoot, renderPriorityLevel) {
18501862
}
18511863
}
18521864

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-
18641865
// The following switch statement is only concerned about placement,
18651866
// updates, and deletions. To avoid needing to add a case for every possible
18661867
// bitmap value, we remove the secondary effects from the effect tag and

packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.internal.js

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,13 +281,30 @@ describe('ReactSchedulerIntegration', () => {
281281
});
282282
return null;
283283
}
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([]);
284299
await ReactNoop.act(async () => {
285300
ReactNoop.render(<Effects />);
286301
});
287302
expect(Scheduler).toHaveYielded([
303+
'Cleanup Layout Effect',
288304
'Layout Effect',
289305
'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',
291308
'Scheduled Normal Callback from Layout Effect',
292309
]);
293310
});

0 commit comments

Comments
 (0)