Skip to content

Commit 165ffdd

Browse files
committed
Flush subtree effects
1 parent 6ef4405 commit 165ffdd

File tree

2 files changed

+62
-4
lines changed

2 files changed

+62
-4
lines changed

hooks/src/index.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,9 @@ options._render = vnode => {
6464
hookItem._pendingArgs = hookItem._nextValue = undefined;
6565
});
6666
} else {
67-
hooks._pendingEffects.some(invokeCleanup);
68-
hooks._pendingEffects.some(invokeEffect);
69-
hooks._pendingEffects = [];
67+
if (hooks._pendingEffects.length) {
68+
flushAfterPaintEffects();
69+
}
7070
currentIndex = 0;
7171
}
7272
}

hooks/test/browser/useEffect.test.js

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { act, teardown as teardownAct } from 'preact/test-utils';
2-
import { createElement, render, Fragment, Component } from 'preact';
2+
import { createElement, render, Fragment, Component, options } from 'preact';
33
import { useEffect, useState, useRef } from 'preact/hooks';
44
import { setupScratch, teardown } from '../../../test/_util/helpers';
55
import { useEffectAssertions } from './useEffectAssertions';
@@ -665,4 +665,62 @@ describe('useEffect', () => {
665665
setVal(1);
666666
});
667667
});
668+
669+
it('should flush all pending effects when one component re-renders before afterPaint', () => {
670+
// Regression test: when only one component re-renders (via setState)
671+
// before afterPaint has flushed effects, ALL pending effects from
672+
// ALL components should be flushed as a batch before the re-render
673+
// proceeds. This matches React's flushPendingEffects() behavior.
674+
//
675+
// Without the fix, only the re-rendering component's stale effects
676+
// were flushed in _render, leaving sibling effects unflushed until
677+
// they got their own re-render or afterPaint fired.
678+
const order = [];
679+
let setB;
680+
681+
function A() {
682+
useEffect(() => {
683+
order.push('A effect');
684+
}, []);
685+
return <div>A</div>;
686+
}
687+
688+
function B() {
689+
const [val, _setB] = useState(0);
690+
setB = _setB;
691+
useEffect(() => {
692+
order.push('B effect');
693+
}, []);
694+
return <div>{val}</div>;
695+
}
696+
697+
function App() {
698+
return (
699+
<div>
700+
<A />
701+
<B />
702+
</div>
703+
);
704+
}
705+
706+
render(<App />, scratch);
707+
708+
// Effects are queued but not yet flushed (afterPaint hasn't fired).
709+
expect(order).to.deep.equal([]);
710+
711+
// Force B to re-render synchronously by overriding debounceRendering.
712+
// This simulates what happens when a store subscription or
713+
// useLayoutEffect triggers a re-render before afterPaint.
714+
const prev = options.debounceRendering;
715+
options.debounceRendering = cb => cb();
716+
setB(1);
717+
options.debounceRendering = prev;
718+
719+
// With the fix: B's _render detects stale effects and calls
720+
// flushAfterPaintEffects(), which flushes BOTH A's and B's effects.
721+
// Without the fix: only B's effects would flush here; A's effect
722+
// would be deferred.
723+
expect(order).to.include('A effect');
724+
expect(order).to.include('B effect');
725+
});
668726
});

0 commit comments

Comments
 (0)