diff --git a/hooks/src/index.js b/hooks/src/index.js index aeca5944da..76e501171d 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -64,9 +64,9 @@ options._render = vnode => { hookItem._pendingArgs = hookItem._nextValue = undefined; }); } else { - hooks._pendingEffects.some(invokeCleanup); - hooks._pendingEffects.some(invokeEffect); - hooks._pendingEffects = []; + if (hooks._pendingEffects.length) { + flushAfterPaintEffects(); + } currentIndex = 0; } } diff --git a/hooks/test/browser/useEffect.test.js b/hooks/test/browser/useEffect.test.js index 3bf8c938cf..af738d04a6 100644 --- a/hooks/test/browser/useEffect.test.js +++ b/hooks/test/browser/useEffect.test.js @@ -1,5 +1,5 @@ import { act, teardown as teardownAct } from 'preact/test-utils'; -import { createElement, render, Fragment, Component } from 'preact'; +import { createElement, render, Fragment, Component, options } from 'preact'; import { useEffect, useState, useRef } from 'preact/hooks'; import { setupScratch, teardown } from '../../../test/_util/helpers'; import { useEffectAssertions } from './useEffectAssertions'; @@ -665,4 +665,62 @@ describe('useEffect', () => { setVal(1); }); }); + + it('should flush all pending effects when one component re-renders before afterPaint', () => { + // Regression test: when only one component re-renders (via setState) + // before afterPaint has flushed effects, ALL pending effects from + // ALL components should be flushed as a batch before the re-render + // proceeds. This matches React's flushPendingEffects() behavior. + // + // Without the fix, only the re-rendering component's stale effects + // were flushed in _render, leaving sibling effects unflushed until + // they got their own re-render or afterPaint fired. + const order = []; + let setB; + + function A() { + useEffect(() => { + order.push('A effect'); + }, []); + return
A
; + } + + function B() { + const [val, _setB] = useState(0); + setB = _setB; + useEffect(() => { + order.push('B effect'); + }, []); + return
{val}
; + } + + function App() { + return ( +
+ + +
+ ); + } + + render(, scratch); + + // Effects are queued but not yet flushed (afterPaint hasn't fired). + expect(order).to.deep.equal([]); + + // Force B to re-render synchronously by overriding debounceRendering. + // This simulates what happens when a store subscription or + // useLayoutEffect triggers a re-render before afterPaint. + const prev = options.debounceRendering; + options.debounceRendering = cb => cb(); + setB(1); + options.debounceRendering = prev; + + // With the fix: B's _render detects stale effects and calls + // flushAfterPaintEffects(), which flushes BOTH A's and B's effects. + // Without the fix: only B's effects would flush here; A's effect + // would be deferred. + expect(order).to.include('A effect'); + expect(order).to.include('B effect'); + }); });