Schedule passive callbacks before layout effects are invoked (#16713)
Sebastian Markbåge committed
Sep 9, 2019 at 13:34 UTC
cc2492ccf14ebc20b2a05ff6806a42b6107e2154
2 files changed
+38
-12
packages/react-reconciler/src/ReactFiberWorkLoop.js
+11
-12
@@ -1497,14 +1497,6 @@ function commitRoot(root) {
1497
ImmediatePriority,
1498
commitRootImpl.bind(null, root, renderPriorityLevel),
1499
);
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
- }
1500
return null;
1501
}
1502
@@ -1858,6 +1850,17 @@ function commitMutationEffects(root: FiberRoot, renderPriorityLevel) {
1850
}
1851
}
1852
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
+
1864
// The following switch statement is only concerned about placement,
1865
// updates, and deletions. To avoid needing to add a case for every possible
1866
// bitmap value, we remove the secondary effects from the effect tag and
@@ -1943,10 +1946,6 @@ function commitLayoutEffects(
1946
commitAttachRef(nextEffect);
1947
}
1948
1946
- if (effectTag & Passive) {
1947
- rootDoesHavePassiveEffects = true;
1948
- }
1949
-
1949
resetCurrentDebugFiberInDEV();
1950
nextEffect = nextEffect.nextEffect;
1951
}
packages/react-reconciler/src/__tests__/ReactSchedulerIntegration-test.internal.js
+27
@@ -265,6 +265,33 @@ describe('ReactSchedulerIntegration', () => {
265
expect(Scheduler).toHaveYielded(['Effect clean-up priority: Idle']);
266
});
267
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
+
295
it('after completing a level of work, infers priority of the next batch based on its expiration time', () => {
296
function App({label}) {
297
Scheduler.unstable_yieldValue(