Skip to content

Commit

Permalink
Let's schedule the passive effects even earlier (#16714)
Browse files Browse the repository at this point in the history
It turns out I needed to schedule mine in the mutation phase and there
are also clean up life-cycles there.
  • Loading branch information
sebmarkbage authored Sep 9, 2019
1 parent cc2492c commit 440cbf2
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 13 deletions.
25 changes: 13 additions & 12 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -1819,7 +1819,8 @@ function commitRootImpl(root, renderPriorityLevel) {

function commitBeforeMutationEffects() {
while (nextEffect !== null) {
if ((nextEffect.effectTag & Snapshot) !== NoEffect) {
const effectTag = nextEffect.effectTag;
if ((effectTag & Snapshot) !== NoEffect) {
setCurrentDebugFiberInDEV(nextEffect);
recordEffect();

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

resetCurrentDebugFiberInDEV();
}
if ((effectTag & Passive) !== NoEffect) {
// If there are passive effects, schedule a callback to flush at
// the earliest opportunity.
if (!rootDoesHavePassiveEffects) {
rootDoesHavePassiveEffects = true;
scheduleCallback(NormalPriority, () => {
flushPassiveEffects();
return null;
});
}
}
nextEffect = nextEffect.nextEffect;
}
}
Expand All @@ -1850,17 +1862,6 @@ function commitMutationEffects(root: FiberRoot, renderPriorityLevel) {
}
}

if (effectTag & Passive) {
// If there are passive effects, schedule a callback to flush them.
if (!rootDoesHavePassiveEffects) {
rootDoesHavePassiveEffects = true;
scheduleCallback(NormalPriority, () => {
flushPassiveEffects();
return null;
});
}
}

// The following switch statement is only concerned about placement,
// updates, and deletions. To avoid needing to add a case for every possible
// bitmap value, we remove the secondary effects from the effect tag and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,13 +281,30 @@ describe('ReactSchedulerIntegration', () => {
});
return null;
}
function CleanupEffect() {
useLayoutEffect(() => () => {
Scheduler.unstable_yieldValue('Cleanup Layout Effect');
Scheduler.unstable_scheduleCallback(NormalPriority, () =>
Scheduler.unstable_yieldValue(
'Scheduled Normal Callback from Cleanup Layout Effect',
),
);
});
return null;
}
await ReactNoop.act(async () => {
ReactNoop.render(<CleanupEffect />);
});
expect(Scheduler).toHaveYielded([]);
await ReactNoop.act(async () => {
ReactNoop.render(<Effects />);
});
expect(Scheduler).toHaveYielded([
'Cleanup Layout Effect',
'Layout Effect',
'Passive Effect',
// This callback should be scheduled after the passive effects.
// These callbacks should be scheduled after the passive effects.
'Scheduled Normal Callback from Cleanup Layout Effect',
'Scheduled Normal Callback from Layout Effect',
]);
});
Expand Down

0 comments on commit 440cbf2

Please sign in to comment.