Skip to content

Commit

Permalink
Merge pull request #1851 from preactjs/handle-flush-hook-changes
Browse files Browse the repository at this point in the history
Fix `act` not flushing effects/updates if global effect/update queues are non-empty before `act` call
  • Loading branch information
marvinhagemeister authored Aug 11, 2019
2 parents 52ab905 + 8f4fb12 commit 42aff16
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 2 deletions.
6 changes: 5 additions & 1 deletion hooks/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,12 @@ function safeRaf(callback) {

/* istanbul ignore else */
if (typeof window !== 'undefined') {
let prevRaf = options.requestAnimationFrame;
afterPaint = (component) => {
if (!component._afterPaintQueued && (component._afterPaintQueued = true) && afterPaintEffects.push(component) === 1) {
if ((!component._afterPaintQueued && (component._afterPaintQueued = true) && afterPaintEffects.push(component) === 1) ||
prevRaf !== options.requestAnimationFrame) {
prevRaf = options.requestAnimationFrame;

/* istanbul ignore next */
(options.requestAnimationFrame || safeRaf)(flushAfterPaintEffects);
}
Expand Down
6 changes: 5 additions & 1 deletion src/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,16 @@ const defer = typeof Promise=='function' ? Promise.prototype.then.bind(Promise.r
* * [Callbacks synchronous and asynchronous](https://blog.ometer.com/2011/07/24/callbacks-synchronous-and-asynchronous/)
*/

let prevDebounce = options.debounceRendering;

/**
* Enqueue a rerender of a component
* @param {import('./internal').Component} c The component to rerender
*/
export function enqueueRender(c) {
if (!c._dirty && (c._dirty = true) && q.push(c) === 1) {
if ((!c._dirty && (c._dirty = true) && q.push(c) === 1) ||
(prevDebounce !== options.debounceRendering)) {
prevDebounce = options.debounceRendering;
(options.debounceRendering || defer)(process);
}
}
Expand Down
48 changes: 48 additions & 0 deletions test-utils/test/shared/act.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,4 +143,52 @@ describe('act', () => {
act(() => null);
expect(options.debounceRendering).to.equal(undefined);
});

it('should flush state updates if there are pending state updates before `act` call', () => {
function CounterButton() {
const [count, setCount] = useState(0);
const increment = () => setCount(count => count + 1);
return <button onClick={increment}>{count}</button>;
}

render(<CounterButton />, scratch);
const button = scratch.querySelector('button');

// Click button. This will schedule an update which is deferred, as is
// normal for Preact, since it happens outside an `act` call.
button.dispatchEvent(new Event('click'));

expect(button.textContent).to.equal('0');

act(() => {
// Click button a second time. This will schedule a second update.
button.dispatchEvent(new Event('click'));
});
// All state updates should be applied synchronously after the `act`
// callback has run but before `act` returns.
expect(button.textContent).to.equal('2');
});

it('should flush effects if there are pending effects before `act` call', () => {
function Counter() {
const [count, setCount] = useState(0);
useEffect(() => {
setCount(count => count + 1);
}, []);
return <div>{count}</div>;
}

// Render a component which schedules an effect outside of an `act`
// call. This will be scheduled to execute after the next paint as usual.
render(<Counter />, scratch);
expect(scratch.firstChild.textContent).to.equal('0');

// Render a component inside an `act` call, this effect should be
// executed synchronously before `act` returns.
act(() => {
render(<div />, scratch);
render(<Counter />, scratch);
});
expect(scratch.firstChild.textContent).to.equal('1');
});
});

0 comments on commit 42aff16

Please sign in to comment.