diff --git a/hooks/src/index.js b/hooks/src/index.js index d6e66dfe7f..86b407f302 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -1,4 +1,5 @@ import { options as _options } from 'preact'; +import { SKIP_CHILDREN } from '../../src/constants'; const ObjectIs = Object.is; @@ -26,6 +27,7 @@ let oldAfterDiff = options.diffed; let oldCommit = options._commit; let oldBeforeUnmount = options.unmount; let oldRoot = options._root; +let oldAfterRender = options._afterRender; // We take the minimum timeout for requestAnimationFrame to ensure that // the callback is invoked after the next frame. 35ms is based on a 30hz @@ -60,10 +62,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); @@ -186,19 +185,13 @@ 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 (!ObjectIs(currentValue, nextValue)) { - hookState._nextValue = [nextValue, hookState._value[1]]; - hookState._component.setState({}); - } + hookState._actions.push(action); + hookState._component.setState({}); } ]; @@ -207,75 +200,55 @@ export function useReducer(reducer, initialState, init) { 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); + + currentComponent.shouldComponentUpdate = (p, s, c) => { + return prevScu + ? prevScu.call(this, p, s, c) || hookState._actions.length + : hookState._actions.length; }; + } + } - // 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 (!ObjectIs(currentValue, hookItem._value[0])) - shouldUpdate = true; - } - }); + if (hookState._actions.length) { + const initialValue = hookState._value[0]; + hookState._actions.some(action => { + hookState._value[0] = hookState._reducer(hookState._value[0], action); + }); - return prevScu - ? prevScu.call(this, p, s, c) || shouldUpdate - : shouldUpdate; - } + hookState._didUpdate = !ObjectIs(initialValue, hookState._value[0]); + hookState._value = [hookState._value[0], hookState._value[1]]; + hookState._didExecute = true; + hookState._actions = []; + } - currentComponent.shouldComponentUpdate = updateHookState; + 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; + }); } - return hookState._nextValue || hookState._value; -} + if (oldAfterRender) oldAfterRender(newVNode, oldVNode); +}; /** * @param {import('./internal').Effect} callback diff --git a/hooks/src/internal.d.ts b/hooks/src/internal.d.ts index 76cd97812b..b5b6d66c99 100644 --- a/hooks/src/internal.d.ts +++ b/hooks/src/internal.d.ts @@ -4,7 +4,7 @@ import { VNode as PreactVNode, PreactContext, HookType, - ErrorInfo, + ErrorInfo } from '../../src/internal'; import { Reducer, StateUpdater } from '.'; @@ -32,7 +32,8 @@ export interface ComponentHooks { _pendingEffects: EffectHookState[]; } -export interface Component extends Omit, '_renderCallbacks'> { +export interface Component + extends Omit, '_renderCallbacks'> { __hooks?: ComponentHooks; // Extend to include HookStates _renderCallbacks?: Array void)>; @@ -54,8 +55,6 @@ export type HookState = interface BaseHookState { _value?: unknown; - _nextValue?: unknown; - _pendingValue?: unknown; _args?: unknown; _pendingArgs?: unknown; _component?: unknown; @@ -74,7 +73,6 @@ export interface EffectHookState extends BaseHookState { export interface MemoHookState extends BaseHookState { _value?: T; - _pendingValue?: T; _args?: unknown[]; _pendingArgs?: unknown[]; _factory?: () => T; @@ -82,10 +80,12 @@ export interface MemoHookState extends BaseHookState { export interface ReducerHookState extends BaseHookState { - _nextValue?: [S, StateUpdater]; _value?: [S, StateUpdater]; + _actions?: any[]; _component?: Component; _reducer?: Reducer; + _didExecute?: boolean; + _didUpdate?: boolean; } export interface ContextHookState extends BaseHookState { diff --git a/hooks/test/browser/useState.test.js b/hooks/test/browser/useState.test.js index 5b020c43aa..29e722c4c2 100644 --- a/hooks/test/browser/useState.test.js +++ b/hooks/test/browser/useState.test.js @@ -1,7 +1,14 @@ import { setupRerender, act } from 'preact/test-utils'; import { createElement, render, createContext, Component } from 'preact'; -import { vi } from 'vitest'; -import { useState, useContext, useEffect } from 'preact/hooks'; +import { afterAll, beforeAll, expect, vi } from 'vitest'; +import { + useState, + useContext, + useEffect, + useLayoutEffect, + useReducer, + useRef +} from 'preact/hooks'; import { setupScratch, teardown } from '../../../test/_util/helpers'; /** @jsx createElement */ @@ -70,12 +77,12 @@ describe('useState', () => { doSetState(0); rerender(); expect(lastState).to.equal(0); - expect(Comp).toHaveBeenCalledOnce(); + expect(Comp).toHaveBeenCalledTimes(2); doSetState(() => 0); rerender(); expect(lastState).to.equal(0); - expect(Comp).toHaveBeenCalledOnce(); + expect(Comp).toHaveBeenCalledTimes(3); }); it('rerenders when setting the state', () => { @@ -345,7 +352,7 @@ describe('useState', () => { render(, scratch); }); - expect(renderSpy).to.be.calledTwice; + expect(renderSpy).to.be.calledThrice; }); // see preactjs/preact#3731 @@ -373,7 +380,7 @@ describe('useState', () => { expect(scratch.innerHTML).to.equal('

hello world!!!

'); }); - it('should limit rerenders when setting state to NaN', () => { + it('should exhaust renders when NaN state is set as a result of a props update', () => { const calls = []; const App = ({ i }) => { calls.push('rendering' + i); @@ -395,10 +402,57 @@ describe('useState', () => { act(() => { render(, scratch); }); - expect(calls.length).to.equal(3); + expect(calls.length).to.equal(27); expect(calls.slice(1).every(c => c === 'rendering2')).to.equal(true); }); + it('should bail correctly when setting NaN twice', () => { + const calls = []; + let set; + const Greeting = ({ greeting }) => { + calls.push('rendering ' + greeting); + return

{greeting}

; + }; + const App = () => { + const [greeting, setGreeting] = useState(0); + set = setGreeting; + + return ; + }; + + act(() => { + render(, scratch); + }); + expect(calls.length).to.equal(1); + expect(calls).to.deep.equal(['rendering 0']); + + act(() => { + set(1); + }); + expect(calls.length).to.equal(2); + expect(calls).to.deep.equal(['rendering 0', 'rendering 1']); + + act(() => { + set(NaN); + }); + expect(calls.length).to.equal(3); + expect(calls).to.deep.equal([ + 'rendering 0', + 'rendering 1', + 'rendering NaN' + ]); + + act(() => { + set(NaN); + }); + expect(calls.length).to.equal(3); + expect(calls).to.deep.equal([ + 'rendering 0', + 'rendering 1', + 'rendering NaN' + ]); + }); + describe('Global sCU', () => { let prevScu; beforeAll(() => { @@ -440,4 +494,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 ( + {children} + ); + }; + + 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 ( +
+
Context count: {count}
+
Local count: {c}
+
+ ); + } + + // Render this + function App() { + const [state, setState] = useState(0); + return ( + + + + ); + } + + act(() => { + render(, scratch); + }); + expect(scratch.textContent).to.equal('Context count: 0Local count: 0'); + + act(() => { + set(); + }); + expect(scratch.textContent).to.equal('Context count: 1Local count: 1'); + }); }); diff --git a/mangle.json b/mangle.json index d999d593f9..bfe7a0b10b 100644 --- a/mangle.json +++ b/mangle.json @@ -33,12 +33,14 @@ "$_list": "__", "$_pendingEffects": "__h", "$_value": "__", - "$_nextValue": "__N", + "$_didExecute": "__N", + "$_didUpdate": "__U", "$_original": "__v", "$_args": "__H", "$_factory": "__h", "$_depth": "__b", "$_dirty": "__d", + "$_afterRender": "__d", "$_mask": "__m", "$_detachOnNextRender": "__b", "$_force": "__e", diff --git a/src/constants.js b/src/constants.js index 2fe00830be..57d56d98c6 100644 --- a/src/constants.js +++ b/src/constants.js @@ -6,6 +6,8 @@ export const MODE_SUSPENDED = 1 << 7; export const INSERT_VNODE = 1 << 2; /** Indicates a VNode has been matched with another VNode in the diff */ export const MATCHED = 1 << 1; +/** Indicates that children should not be diffed */ +export const SKIP_CHILDREN = 1 << 3; /** Reset all mode flags */ export const RESET_MODE = ~(MODE_HYDRATE | MODE_SUSPENDED); diff --git a/src/diff/index.js b/src/diff/index.js index 30eed07d62..d9a62cbae5 100644 --- a/src/diff/index.js +++ b/src/diff/index.js @@ -5,6 +5,7 @@ import { MODE_SUSPENDED, NULL, RESET_MODE, + SKIP_CHILDREN, SVG_NAMESPACE, UNDEFINED, XHTML_NAMESPACE @@ -223,6 +224,7 @@ export function diff( c._force = false; let renderHook = options._render, + afterRender = options._afterRender, count = 0; if (isClassComponent) { c.state = c._nextState; @@ -231,6 +233,7 @@ export function diff( if (renderHook) renderHook(newVNode); tmp = c.render(c.props, c.state, c.context); + if (afterRender) afterRender(newVNode, oldVNode); for (let i = 0; i < c._stateCallbacks.length; i++) { c._renderCallbacks.push(c._stateCallbacks[i]); @@ -242,6 +245,18 @@ export function diff( if (renderHook) renderHook(newVNode); tmp = c.render(c.props, c.state, c.context); + if (afterRender) afterRender(newVNode, oldVNode); + + if (newVNode._flags & SKIP_CHILDREN) { + c._dirty = false; + c._renderCallbacks = []; + newVNode._dom = oldVNode._dom; + newVNode._children = oldVNode._children; + newVNode._children.some(vnode => { + if (vnode) vnode._parent = newVNode; + }); + break outer; + } // Handle setState called in render, see #2553 c.state = c._nextState; diff --git a/src/index-5.d.ts b/src/index-5.d.ts index be90714274..1431b01f30 100644 --- a/src/index-5.d.ts +++ b/src/index-5.d.ts @@ -344,6 +344,7 @@ export interface Options { debounceRendering?(cb: () => void): void; useDebugValue?(value: string | number): void; _addHookName?(name: string | number): void; + _afterRender?(newVNode: VNode, oldVNode: VNode): void; __suspenseDidResolve?(vnode: VNode, cb: () => void): void; // __canSuspenseResolve?(vnode: VNode, cb: () => void): void; diff --git a/src/internal.d.ts b/src/internal.d.ts index c32303ee0f..403fe37dbb 100644 --- a/src/internal.d.ts +++ b/src/internal.d.ts @@ -27,6 +27,8 @@ export interface ErrorInfo { } export interface Options extends preact.Options { + /** Attach a hook that is invoked after a vnode has rendered. */ + _afterRender?(vnode: VNode, oldVNode: VNode): void; /** Attach a hook that is invoked before render, mainly to check the arguments. */ _root?(vnode: ComponentChild, parent: preact.ContainerNode): void; /** Attach a hook that is invoked before a vnode is diffed. */ @@ -156,7 +158,8 @@ export interface VNode

extends preact.VNode

{ _flags: number; } -export interface Component

extends Omit, 'base'> { +export interface Component

+ extends Omit, 'base'> { // When component is functional component, this is reset to functional component constructor: ComponentType

; state: S; // Override Component["state"] to not be readonly for internal use, specifically Hooks