Ensure we reset renderCount (#5017)

* Golf some bytes and ensure we reset renderCount

* Apply suggestion from @JoviDeCroock

* Add test

Author: JoviDeCroock

src/component.js

@@ -217,28 +217,31 @@ const depthSort = (a, b) => a._vnode._depth - b._vnode._depth;
 
 /** Flush the render queue by rerendering all queued components */
 function process() {
-	let c,
-		l = 1;
-
-	// Don't update `renderCount` yet. Keep its value non-zero to prevent unnecessary
-	// process() calls from getting scheduled while `queue` is still being consumed.
-	while (rerenderQueue.length) {
-		// Keep the rerender queue sorted by (depth, insertion order). The queue
-		// will initially be sorted on the first iteration only if it has more than 1 item.
-		//
-		// New items can be added to the queue e.g. when rerendering a provider, so we want to
-		// keep the order from top to bottom with those new items so we can handle them in a
-		// single pass
-		if (rerenderQueue.length > l) {
-			rerenderQueue.sort(depthSort);
-		}
+	try {
+		let c,
+			l = 1;
+
+		// Don't update `renderCount` yet. Keep its value non-zero to prevent unnecessary
+		// process() calls from getting scheduled while `queue` is still being consumed.
+		while (rerenderQueue.length) {
+			// Keep the rerender queue sorted by (depth, insertion order). The queue
+			// will initially be sorted on the first iteration only if it has more than 1 item.
+			//
+			// New items can be added to the queue e.g. when rerendering a provider, so we want to
+			// keep the order from top to bottom with those new items so we can handle them in a
+			// single pass
+			if (rerenderQueue.length > l) {
+				rerenderQueue.sort(depthSort);
+			}
 
-		c = rerenderQueue.shift();
-		l = rerenderQueue.length;
+			c = rerenderQueue.shift();
+			l = rerenderQueue.length;
 
-		renderComponent(c);
+			renderComponent(c);
+		}
+	} finally {
+		rerenderQueue.length = process._rerenderCount = 0;
 	}
-	process._rerenderCount = 0;
 }
 
 process._rerenderCount = 0;

test/browser/components.test.js

@@ -1,4 +1,4 @@
-import { createElement, render, Component, Fragment } from 'preact';
+import { createElement, render, Component, Fragment, options } from 'preact';
 import { setupRerender } from 'preact/test-utils';
 import {
 	setupScratch,
@@ -2731,4 +2731,61 @@ describe('Components', () => {
 			expect(scratch.innerHTML).to.equal('<div>Updated: yes</div>');
 		});
 	});
+
+	it('should reset the rerender queue if rendering throws', () => {
+		let shouldThrow = false;
+		let increment;
+		let debounceCount = 0;
+		let flush;
+
+		// Use a custom debounceRendering so we can track whether
+		// enqueueRender actually schedules a new process() call.
+		const prevDebounce = options.debounceRendering;
+		options.debounceRendering = cb => {
+			debounceCount++;
+			flush = cb;
+		};
+
+		class App extends Component {
+			constructor(props) {
+				super(props);
+				this.state = { count: 0 };
+				increment = () => this.setState(s => ({ count: s.count + 1 }));
+			}
+			render(props, state) {
+				if (shouldThrow) throw new Error('test error');
+				return <div>{state.count}</div>;
+			}
+		}
+
+		render(<App />, scratch);
+		expect(scratch.innerHTML).to.equal('<div>0</div>');
+
+		// First setState to sync prevDebounce inside enqueueRender
+		increment();
+		expect(debounceCount).to.equal(1);
+		flush();
+		expect(scratch.innerHTML).to.equal('<div>1</div>');
+
+		// Trigger a rerender that will throw
+		debounceCount = 0;
+		shouldThrow = true;
+		increment();
+		expect(debounceCount).to.equal(1);
+		expect(() => flush()).to.throw();
+
+		// After the error, a subsequent setState must schedule a new
+		// process() via debounceRendering. Without the fix,
+		// _rerenderCount stays non-zero and enqueueRender won't
+		// call debounceRendering, leaving the component stuck.
+		debounceCount = 0;
+		shouldThrow = false;
+		increment();
+		expect(debounceCount).to.equal(1);
+
+		flush();
+		expect(scratch.innerHTML).to.equal('<div>3</div>');
+
+		options.debounceRendering = prevDebounce;
+	});
 });