backport useState optim

hooks/src/index.js

@@ -180,15 +180,49 @@ export function useReducer(reducer, initialState, init) {
 		];
 
 		hookState._internal = currentInternal;
-		currentInternal._component.shouldComponentUpdate = () => {
-			if (!hookState._nextValue) return true;
-
-			const currentValue = hookState._value[0];
-			hookState._value = hookState._nextValue;
-			hookState._nextValue = undefined;
+		if (!currentInternal.data._hasScuFromHooks) {
+			currentInternal.data._hasScuFromHooks = true;
+			const prevScu = currentInternal._component.shouldComponentUpdate;
+
+			// This SCU has the purpose of bailing out after repeated updates
+			// to stateful hooks.
+			// we store the next value in _nextValue[0] and keep doing that for all
+			// state setters, if we have next states and
+			// all next states within a component end up being equal to their original state
+			// we are safe to bail out for this specific component.
+			currentInternal._component.shouldComponentUpdate = function(p, s, c) {
+				if (!hookState._internal.data.__hooks) return true;
+
+				const stateHooks = hookState._internal.data.__hooks._list.filter(
+					x => x._internal
+				);
+				const allHooksEmpty = stateHooks.every(x => !x._nextValue);
+				// When we have no updated hooks in the component we invoke the previous SCU or
+				// traverse the VDOM tree further.
+				if (allHooksEmpty) {
+					return prevScu ? prevScu.call(this, p, s, c) : true;
+				}
 
-			return currentValue !== hookState._value[0];
-		};
+				// We check whether we have components with a nextValue set that
+				// have values that aren't equal to one another this pushes
+				// us to update further down the tree
+				let shouldUpdate = false;
+				stateHooks.forEach(hookItem => {
+					if (hookItem._nextValue) {
+						const currentValue = hookItem._value[0];
+						hookItem._value = hookItem._nextValue;
+						hookItem._nextValue = undefined;
+						if (currentValue !== hookItem._value[0]) shouldUpdate = true;
+					}
+				});
+
+				return shouldUpdate
+					? prevScu
+						? prevScu.call(this, p, s, c)
+						: true
+					: false;
+			};
+		}
 	}
 
 	return hookState._nextValue || hookState._value;

hooks/test/browser/useState.test.js

@@ -1,4 +1,4 @@
-import { act, setupRerender } from 'preact/test-utils';
+import { setupRerender, act } from 'preact/test-utils';
 import { createElement, render, createContext } from 'preact';
 import { useState, useContext, useEffect } from 'preact/hooks';
 import { setupScratch, teardown } from '../../../test/_util/helpers';
@@ -55,7 +55,7 @@ describe('useState', () => {
 		let lastState;
 		let doSetState;
 
-		const Comp = sinon.spy(function Comp() {
+		const Comp = sinon.spy(() => {
 			const [state, setState] = useState(0);
 			lastState = state;
 			doSetState = setState;
@@ -81,7 +81,7 @@ describe('useState', () => {
 		let lastState;
 		let doSetState;
 
-		const Comp = sinon.spy(function Comp() {
+		const Comp = sinon.spy(() => {
 			const [state, setState] = useState(0);
 			lastState = state;
 			doSetState = setState;
@@ -176,26 +176,6 @@ describe('useState', () => {
 		expect(scratch.innerHTML).to.equal('<p>hi</p>');
 	});
 
-	it('should render a second time when the render function updates state', () => {
-		const calls = [];
-		const App = () => {
-			const [greeting, setGreeting] = useState('bye');
-
-			if (greeting === 'bye') {
-				setGreeting('hi');
-			}
-
-			calls.push(greeting);
-
-			return <p>{greeting}</p>;
-		};
-
-		render(<App />, scratch);
-		expect(calls.length).to.equal(2);
-		expect(calls).to.deep.equal(['bye', 'hi']);
-		expect(scratch.textContent).to.equal('hi');
-	});
-
 	it('should handle queued useState', () => {
 		function Message({ message, onClose }) {
 			const [isVisible, setVisible] = useState(Boolean(message));
@@ -232,6 +212,89 @@ describe('useState', () => {
 		expect(scratch.innerHTML).to.equal('');
 	});
 
+	it('should render a second time when the render function updates state', () => {
+		const calls = [];
+		const App = () => {
+			const [greeting, setGreeting] = useState('bye');
+
+			if (greeting === 'bye') {
+				setGreeting('hi');
+			}
+
+			calls.push(greeting);
+
+			return <p>{greeting}</p>;
+		};
+
+		act(() => {
+			render(<App />, scratch);
+		});
+		expect(calls.length).to.equal(2);
+		expect(calls).to.deep.equal(['bye', 'hi']);
+		expect(scratch.textContent).to.equal('hi');
+	});
+
+	// https://github.com/preactjs/preact/issues/3669
+	it('correctly updates with multiple state updates', () => {
+		let simulateClick;
+		function TestWidget() {
+			const [saved, setSaved] = useState(false);
+			const [, setSaving] = useState(false);
+
+			simulateClick = () => {
+				setSaving(true);
+				setSaved(true);
+				setSaving(false);
+			};
+
+			return <div>{saved ? 'Saved!' : 'Unsaved!'}</div>;
+		}
+
+		render(<TestWidget />, scratch);
+		expect(scratch.innerHTML).to.equal('<div>Unsaved!</div>');
+
+		act(() => {
+			simulateClick();
+		});
+
+		expect(scratch.innerHTML).to.equal('<div>Saved!</div>');
+	});
+
+	// https://github.com/preactjs/preact/issues/3674
+	it('ensure we iterate over all hooks', () => {
+		let open, close;
+
+		function TestWidget() {
+			const [, setCounter] = useState(0);
+			const [isOpen, setOpen] = useState(false);
+
+			open = () => {
+				setCounter(42);
+				setOpen(true);
+			};
+
+			close = () => {
+				setOpen(false);
+			};
+
+			return <div>{isOpen ? 'open' : 'closed'}</div>;
+		}
+
+		render(<TestWidget />, scratch);
+		expect(scratch.innerHTML).to.equal('<div>closed</div>');
+
+		act(() => {
+			open();
+		});
+
+		expect(scratch.innerHTML).to.equal('<div>open</div>');
+
+		act(() => {
+			close();
+		});
+		expect(scratch.innerHTML).to.equal('<div>closed</div>');
+	});
+
 	it('does not loop when states are equal after batches', () => {
 		const renderSpy = sinon.spy();
 		const Context = createContext(null);