From fba364a9067fe28c3b0478c011df58610df493b1 Mon Sep 17 00:00:00 2001
From: Robert Knight <robertknight@gmail.com>
Date: Sat, 10 Aug 2019 22:09:33 +0100
Subject: [PATCH] Schedule effect/update queue flushes if hooks change

When scheduling a state update or effect, check whether the
`options.{debounceRendering, requestAnimationFrame}` hooks changed since
an update/effect queue flush was last scheduled and schedule a flush
using the new hooks if so.

This fixes an issue where a call to `act` would fail to synchronously flush
state updates and effects if a flush was already scheduled but not yet executed
before `act` was called. This happened because when a state update or effect was
queued during the `act` callback, if the global update/effect queues were
non-empty then the temporary hooks installed by `act` to facilitate
synchronously flushing the queue would not be called.

More generally, this fixes an issue where any changes to the scheduling hooks
do not take effect until any already-scheduled flushes have happened.

Fixes #1794
---
 hooks/src/index.js                 |  6 +++-
 src/component.js                   |  6 +++-
 test-utils/test/shared/act.test.js | 48 ++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/hooks/src/index.js b/hooks/src/index.js
index e8249342d6..b593ae27ad 100644
--- a/hooks/src/index.js
+++ b/hooks/src/index.js
@@ -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);
 		}
diff --git a/src/component.js b/src/component.js
index 5355c2fdd2..a383fb2bed 100644
--- a/src/component.js
+++ b/src/component.js
@@ -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);
 	}
 }
diff --git a/test-utils/test/shared/act.test.js b/test-utils/test/shared/act.test.js
index 71323261bb..f1d3750e9b 100644
--- a/test-utils/test/shared/act.test.js
+++ b/test-utils/test/shared/act.test.js
@@ -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');
+	});
 });