The Lost Effect, chapter 2
Previously, flushPassiveEffects (called by scheduling work) would overwrite rootWithPendingPassiveEffects before we had a chance to schedule the work.
Sophie Alpert committed
Oct 10, 2018 at 15:41 UTC
30aa4ad5541c6e82de465a5e6234e5bb1329dec1
2 files changed
+39
-5
packages/react-reconciler/src/ReactFiberScheduler.js
+6
-5
@@ -599,12 +599,13 @@ function flushPassiveEffectsBeforeSchedulingUpdateOnFiber(fiber: Fiber) {
599
}
600
601
function flushPassiveEffects(root: FiberRoot) {
602
- if (root === rootWithPendingPassiveEffects) {
602
+ if (
603
+ passiveEffectCallbackHandle !== null &&
604
+ root === rootWithPendingPassiveEffects
605
+ ) {
606
+ Schedule_cancelCallback(passiveEffectCallbackHandle);
607
+ passiveEffectCallbackHandle = null;
608
rootWithPendingPassiveEffects = null;
604
- if (passiveEffectCallbackHandle !== null) {
605
- Schedule_cancelCallback(passiveEffectCallbackHandle);
606
- passiveEffectCallbackHandle = null;
607
- }
609
if (firstPassiveEffect !== null) {
610
commitPassiveEffects(root, firstPassiveEffect);
611
}
packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
+33
@@ -657,6 +657,39 @@ describe('ReactHooks', () => {
657
expect(ReactNoop.clearYields()).toEqual(null);
658
});
659
660
+ it('flushes passive effects even if siblings schedule an update', () => {
661
+ function PassiveEffect(props) {
662
+ useEffect(() => {
663
+ ReactNoop.yield('Passive effect');
664
+ });
665
+ return <Text text="Passive" />;
666
+ }
667
+ function LayoutEffect(props) {
668
+ let [count, setCount] = useState(0);
669
+ useLayoutEffect(() => {
670
+ // Scheduling work shouldn't interfere with the queued passive effect
671
+ if (count === 0) {
672
+ setCount(1);
673
+ }
674
+ ReactNoop.yield('Layout effect ' + count);
675
+ });
676
+ return <Text text="Layout" />;
677
+ }
678
+ ReactNoop.render([<PassiveEffect key="p" />, <LayoutEffect key="l" />]);
679
+ expect(ReactNoop.flush()).toEqual([
680
+ 'Passive',
681
+ 'Layout',
682
+ 'Layout effect 0',
683
+ 'Passive effect',
684
+ 'Layout',
685
+ 'Layout effect 1',
686
+ ]);
687
+ expect(ReactNoop.getChildren()).toEqual([
688
+ span('Passive'),
689
+ span('Layout'),
690
+ ]);
691
+ });
692
+
693
it(
694
'flushes effects serially by flushing old effects before flushing ' +
695
"new ones, if they haven't already fired",