Delay executing state updates

hooks/src/index.js

@@ -1,4 +1,5 @@
 import { options as _options } from 'preact';
+import { SKIP_CHILDREN } from '../../src/constants';
 
 /** @type {number} */
 let currentIndex;
@@ -24,6 +25,7 @@ let oldAfterDiff = options.diffed;
 let oldCommit = options._commit;
 let oldBeforeUnmount = options.unmount;
 let oldRoot = options._root;
+let oldAfterRender = options._afterRender;
 
 const RAF_TIMEOUT = 100;
 let prevRaf;
@@ -55,10 +57,7 @@ options._render = vnode => {
 			hooks._pendingEffects = [];
 			currentComponent._renderCallbacks = [];
 			hooks._list.forEach(hookItem => {
-				if (hookItem._nextValue) {
-					hookItem._value = hookItem._nextValue;
-				}
-				hookItem._pendingArgs = hookItem._nextValue = undefined;
+				hookItem._pendingArgs = undefined;
 			});
 		} else {
 			hooks._pendingEffects.forEach(invokeCleanup);
@@ -181,96 +180,65 @@ export function useReducer(reducer, initialState, init) {
 	const hookState = getHookState(currentIndex++, 2);
 	hookState._reducer = reducer;
 	if (!hookState._component) {
+		hookState._actions = [];
 		hookState._value = [
 			!init ? invokeOrReturn(undefined, initialState) : init(initialState),
 
 			action => {
-				const currentValue = hookState._nextValue
-					? hookState._nextValue[0]
-					: hookState._value[0];
-				const nextValue = hookState._reducer(currentValue, action);
-
-				if (currentValue !== nextValue) {
-					hookState._nextValue = [nextValue, hookState._value[1]];
-					hookState._component.setState({});
-				}
+				hookState._actions.push(action);
+				hookState._component.setState({});
 			}
 		];
 
 		hookState._component = currentComponent;
+	}
 
-		if (!currentComponent._hasScuFromHooks) {
-			currentComponent._hasScuFromHooks = true;
-			let prevScu = currentComponent.shouldComponentUpdate;
-			const prevCWU = currentComponent.componentWillUpdate;
-
-			// If we're dealing with a forced update `shouldComponentUpdate` will
-			// not be called. But we use that to update the hook values, so we
-			// need to call it.
-			currentComponent.componentWillUpdate = function (p, s, c) {
-				if (this._force) {
-					let tmp = prevScu;
-					// Clear to avoid other sCU hooks from being called
-					prevScu = undefined;
-					updateHookState(p, s, c);
-					prevScu = tmp;
-				}
-
-				if (prevCWU) prevCWU.call(this, p, s, c);
-			};
-
-			// 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.
-			/**
-			 *
-			 * @type {import('./internal').Component["shouldComponentUpdate"]}
-			 */
-			// @ts-ignore - We don't use TS to downtranspile
-			// eslint-disable-next-line no-inner-declarations
-			function updateHookState(p, s, c) {
-				if (!hookState._component.__hooks) return true;
-
-				/** @type {(x: import('./internal').HookState) => x is import('./internal').ReducerHookState} */
-				const isStateHook = x => !!x._component;
-				const stateHooks =
-					hookState._component.__hooks._list.filter(isStateHook);
-
-				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;
-				}
-
-				// 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 = hookState._component.props !== p;
-				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 prevScu
-					? prevScu.call(this, p, s, c) || shouldUpdate
-					: shouldUpdate;
-			}
+	if (hookState._actions.length) {
+		const initialValue = hookState._value[0];
+		hookState._actions.some(action => {
+			hookState._value[0] = hookState._reducer(hookState._value[0], action);
+		});
 
-			currentComponent.shouldComponentUpdate = updateHookState;
-		}
+		// This is incorrect, we set this shallowly based on the last state
+		// which means that if 2 useReducer calls have actions and the last one
+		// settles to an equal state we bail the previous one as well. This should
+		// be tackled all together in a post-render type of hook, that hook should
+		// also give us the ability to set SKIP_CHILDREN
+		hookState._didUpdate = initialValue !== hookState._value[0];
+		hookState._value = [hookState._value[0], hookState._value[1]];
+		hookState._didExecute = true;
+		hookState._actions = [];
 	}
 
-	return hookState._nextValue || hookState._value;
+	return hookState._value;
 }
 
+options._afterRender = (newVNode, oldVNode) => {
+	if (newVNode._component && newVNode._component.__hooks) {
+		const hooks = newVNode._component.__hooks._list;
+		const stateHooksThatExecuted = hooks.filter(
+			/** @type {(x: import('./internal').HookState) => x is import('./internal').ReducerHookState} */
+			// @ts-expect-error
+			x => x._component && x._didExecute
+		);
+
+		if (
+			stateHooksThatExecuted.length &&
+			!stateHooksThatExecuted.some(x => x._didUpdate) &&
+			oldVNode.props === newVNode.props
+		) {
+			newVNode._component.__hooks._pendingEffects = [];
+			newVNode._flags |= SKIP_CHILDREN;
+		}
+
+		stateHooksThatExecuted.some(hook => {
+			hook._didExecute = hook._didUpdate = false;
+		});
+	}
+
+	if (oldAfterRender) oldAfterRender(newVNode, oldVNode);
+};
+
 /**
  * @param {import('./internal').Effect} callback
  * @param {unknown[]} args

hooks/src/internal.d.ts

@@ -2,7 +2,7 @@ import {
 	Options as PreactOptions,
 	Component as PreactComponent,
 	VNode as PreactVNode,
-	Context as PreactContext,
+	Context as PreactContext
 } from '../../src/internal';
 import { Reducer, StateUpdater } from '.';
 
@@ -52,8 +52,6 @@ export type HookState =
 
 interface BaseHookState {
 	_value?: unknown;
-	_nextValue?: undefined;
-	_pendingValue?: undefined;
 	_args?: undefined;
 	_pendingArgs?: undefined;
 	_component?: undefined;
@@ -72,18 +70,19 @@ export interface EffectHookState extends BaseHookState {
 
 export interface MemoHookState<T = unknown> extends BaseHookState {
 	_value?: T;
-	_pendingValue?: T;
 	_args?: unknown[];
 	_pendingArgs?: unknown[];
 	_factory?: () => T;
 }
 
 export interface ReducerHookState<S = unknown, A = unknown>
 	extends BaseHookState {
-	_nextValue?: [S, StateUpdater<S>];
+	_actions: Array<any>;
 	_value?: [S, StateUpdater<S>];
 	_component?: Component;
 	_reducer?: Reducer<S, A>;
+	_didExecute?: boolean;
+	_didUpdate?: boolean;
 }
 
 export interface ContextHookState extends BaseHookState {

hooks/test/browser/useReducer.test.js

@@ -194,11 +194,14 @@ describe('useReducer', () => {
 		const badContextDefault = {};
 		const BadContext = createContext({});
 
-		const Abstraction = ({ reducer, defaultState, children }) => (
-			<BadContext.Provider value={useReducer(reducer, defaultState)}>
-				<Wrapper>{children}</Wrapper>
-			</BadContext.Provider>
-		);
+		const Abstraction = ({ reducer, defaultState, children }) => {
+			const state = useReducer(reducer, defaultState);
+			return (
+				<BadContext.Provider value={state}>
+					<Wrapper>{children}</Wrapper>
+				</BadContext.Provider>
+			);
+		};
 
 		const App = () => (
 			<Abstraction reducer={reducer} defaultState={badContextDefault}>

hooks/test/browser/useState.test.js

@@ -1,6 +1,13 @@
 import { setupRerender, act } from 'preact/test-utils';
 import { createElement, render, createContext, Component } from 'preact';
-import { useState, useContext, useEffect } from 'preact/hooks';
+import {
+	useState,
+	useContext,
+	useEffect,
+	useLayoutEffect,
+	useReducer,
+	useRef
+} from 'preact/hooks';
 import { setupScratch, teardown } from '../../../test/_util/helpers';
 
 /** @jsx createElement */
@@ -51,6 +58,8 @@ describe('useState', () => {
 		expect(initState).to.be.calledOnce;
 	});
 
+	// TODO: check whether the delayed execution of
+	// useReducer should also be in useState
 	it('does not rerender on equal state', () => {
 		let lastState;
 		let doSetState;
@@ -69,12 +78,12 @@ describe('useState', () => {
 		doSetState(0);
 		rerender();
 		expect(lastState).to.equal(0);
-		expect(Comp).to.be.calledOnce;
+		expect(Comp).to.be.calledTwice;
 
 		doSetState(() => 0);
 		rerender();
 		expect(lastState).to.equal(0);
-		expect(Comp).to.be.calledOnce;
+		expect(Comp).to.be.calledThrice;
 	});
 
 	it('rerenders when setting the state', () => {
@@ -305,9 +314,11 @@ describe('useState', () => {
 			let context = {
 				modalCount,
 				addModal() {
+					console.log('adding');
 					setModalCount(count => count + 1);
 				},
 				removeModal() {
+					console.log('removing');
 					setModalCount(count => count - 1);
 				}
 			};
@@ -328,11 +339,13 @@ describe('useState', () => {
 		}
 
 		function Popover() {
+			console.log('POPOVER');
 			useModal();
 			return <div>Popover</div>;
 		}
 
 		function App() {
+			console.log('render app');
 			return (
 				<ModalProvider>
 					<Popover />
@@ -344,7 +357,7 @@ describe('useState', () => {
 			render(<App />, scratch);
 		});
 
-		expect(renderSpy).to.be.calledTwice;
+		expect(renderSpy).to.be.calledThrice;
 	});
 
 	// see preactjs/preact#3731
@@ -413,4 +426,94 @@ describe('useState', () => {
 			expect(renders).to.equal(2);
 		});
 	});
+
+	it('Should capture the closure in the reducer', () => {
+		function createContext2() {
+			const context = createContext();
+
+			const ProviderOrig = context.Provider;
+			context.Provider = ({ value, children }) => {
+				const valueRef = useRef(value);
+				const contextValue = useRef();
+
+				if (!contextValue.current) {
+					contextValue.current = {
+						value: valueRef,
+						listener: null
+					};
+				}
+
+				useLayoutEffect(() => {
+					valueRef.current = value;
+					if (contextValue.current.listener) {
+						contextValue.current.listener([value]);
+					}
+				}, [value]);
+				return (
+					<ProviderOrig value={contextValue.current}>{children}</ProviderOrig>
+				);
+			};
+
+			return context;
+		}
+
+		function useContextSelector(context) {
+			const contextValue = useContext(context);
+			const {
+				value: { current: value }
+			} = contextValue;
+			const [state, dispatch] = useReducer(
+				() => {
+					return {
+						value
+					};
+				},
+				{
+					value
+				}
+			);
+			useLayoutEffect(() => {
+				contextValue.listener = dispatch;
+			}, []);
+			return state.value;
+		}
+
+		const context = createContext2();
+		let set;
+
+		function Child() {
+			const [count, setState] = useContextSelector(context);
+			const [c, setC] = useState(0);
+			set = () => {
+				setC(s => s + 1);
+				setState(s => s + 1);
+			};
+			return (
+				<div>
+					<div>Context count: {count}</div>
+					<div>Local count: {c}</div>
+				</div>
+			);
+		}
+
+		// Render this
+		function App() {
+			const [state, setState] = useState(0);
+			return (
+				<context.Provider value={[state, setState]}>
+					<Child />
+				</context.Provider>
+			);
+		}
+
+		act(() => {
+			render(<App />, scratch);
+		});
+		expect(scratch.textContent).to.equal('Context count: 0Local count: 0');
+
+		act(() => {
+			set();
+		});
+		expect(scratch.textContent).to.equal('Context count: 1Local count: 1');
+	});
 });