Skip to content

Commit cc2492c

Browse files
authored
Schedule passive callbacks before layout effects are invoked (#16713)
1 parent 031eba7 commit cc2492c

File tree

2 files changed

+38
-12
lines changed

2 files changed

+38
-12
lines changed

packages/react-reconciler/src/ReactFiberWorkLoop.js

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1497,14 +1497,6 @@ function commitRoot(root) {
14971497
ImmediatePriority,
14981498
commitRootImpl.bind(null, root, renderPriorityLevel),
14991499
);
1500-
// If there are passive effects, schedule a callback to flush them. This goes
1501-
// outside commitRootImpl so that it inherits the priority of the render.
1502-
if (rootWithPendingPassiveEffects !== null) {
1503-
scheduleCallback(NormalPriority, () => {
1504-
flushPassiveEffects();
1505-
return null;
1506-
});
1507-
}
15081500
return null;
15091501
}
15101502

@@ -1858,6 +1850,17 @@ function commitMutationEffects(root: FiberRoot, renderPriorityLevel) {
18581850
}
18591851
}
18601852

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+
18611864
// The following switch statement is only concerned about placement,
18621865
// updates, and deletions. To avoid needing to add a case for every possible
18631866
// bitmap value, we remove the secondary effects from the effect tag and
@@ -1943,10 +1946,6 @@ function commitLayoutEffects(
19431946
commitAttachRef(nextEffect);
19441947
}
19451948

1946-
if (effectTag & Passive) {
1947-
rootDoesHavePassiveEffects = true;
1948-
}
1949-
19501949
resetCurrentDebugFiberInDEV();
19511950
nextEffect = nextEffect.nextEffect;
19521951
}

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,33 @@ describe('ReactSchedulerIntegration', () => {
265265
expect(Scheduler).toHaveYielded(['Effect clean-up priority: Idle']);
266266
});
267267

268+
it('passive effects are called before Normal-pri scheduled in layout effects', async () => {
269+
const {useEffect, useLayoutEffect} = React;
270+
function Effects({step}) {
271+
useLayoutEffect(() => {
272+
Scheduler.unstable_yieldValue('Layout Effect');
273+
Scheduler.unstable_scheduleCallback(NormalPriority, () =>
274+
Scheduler.unstable_yieldValue(
275+
'Scheduled Normal Callback from Layout Effect',
276+
),
277+
);
278+
});
279+
useEffect(() => {
280+
Scheduler.unstable_yieldValue('Passive Effect');
281+
});
282+
return null;
283+
}
284+
await ReactNoop.act(async () => {
285+
ReactNoop.render(<Effects />);
286+
});
287+
expect(Scheduler).toHaveYielded([
288+
'Layout Effect',
289+
'Passive Effect',
290+
// This callback should be scheduled after the passive effects.
291+
'Scheduled Normal Callback from Layout Effect',
292+
]);
293+
});
294+
268295
it('after completing a level of work, infers priority of the next batch based on its expiration time', () => {
269296
function App({label}) {
270297
Scheduler.unstable_yieldValue(

0 commit comments

Comments
 (0)