diff --git a/hooks/src/index.js b/hooks/src/index.js index d6c434e400..1be204f66f 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'; /** @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 diff --git a/hooks/src/internal.d.ts b/hooks/src/internal.d.ts index e828b198d7..4a13ff0a8e 100644 --- a/hooks/src/internal.d.ts +++ b/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,7 +70,6 @@ export interface EffectHookState extends BaseHookState { export interface MemoHookState extends BaseHookState { _value?: T; - _pendingValue?: T; _args?: unknown[]; _pendingArgs?: unknown[]; _factory?: () => T; @@ -80,10 +77,12 @@ export interface MemoHookState extends BaseHookState { export interface ReducerHookState extends BaseHookState { - _nextValue?: [S, StateUpdater]; + _actions: Array; _value?: [S, StateUpdater]; _component?: Component; _reducer?: Reducer; + _didExecute?: boolean; + _didUpdate?: boolean; } export interface ContextHookState extends BaseHookState { diff --git a/hooks/test/browser/useReducer.test.js b/hooks/test/browser/useReducer.test.js index 410805b766..00fb550136 100644 --- a/hooks/test/browser/useReducer.test.js +++ b/hooks/test/browser/useReducer.test.js @@ -194,11 +194,14 @@ describe('useReducer', () => { const badContextDefault = {}; const BadContext = createContext({}); - const Abstraction = ({ reducer, defaultState, children }) => ( - - {children} - - ); + const Abstraction = ({ reducer, defaultState, children }) => { + const state = useReducer(reducer, defaultState); + return ( + + {children} + + ); + }; const App = () => ( diff --git a/hooks/test/browser/useState.test.js b/hooks/test/browser/useState.test.js index ffa99e7902..d9c25523c7 100644 --- a/hooks/test/browser/useState.test.js +++ b/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 */ @@ -69,12 +76,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', () => { @@ -344,7 +351,7 @@ describe('useState', () => { render(, scratch); }); - expect(renderSpy).to.be.calledTwice; + expect(renderSpy).to.be.calledThrice; }); // see preactjs/preact#3731 @@ -413,4 +420,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 1d009545cb..fb1310e475 100644 --- a/mangle.json +++ b/mangle.json @@ -32,7 +32,8 @@ "$_list": "__", "$_pendingEffects": "__h", "$_value": "__", - "$_nextValue": "__N", + "$_didExecute": "__N", + "$_didUpdate": "__U", "$_original": "__v", "$_args": "__H", "$_factory": "__h", @@ -70,6 +71,7 @@ "$_diff": "__b", "$_commit": "__c", "$_addHookName": "__a", + "$_afterRender": "__d", "$_render": "__r", "$_hook": "__h", "$_catchError": "__e", diff --git a/src/constants.js b/src/constants.js index 197783b081..54b10517b8 100644 --- a/src/constants.js +++ b/src/constants.js @@ -7,6 +7,8 @@ export const INSERT_VNODE = 1 << 2; /** Indicates a VNode has been matched with another VNode in the diff */ export const MATCHED = 1 << 1; +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 6e139a6094..a61fd6d3b1 100644 --- a/src/diff/index.js +++ b/src/diff/index.js @@ -5,8 +5,9 @@ import { MODE_SUSPENDED, RESET_MODE, SVG_NAMESPACE, - UNDEFINED, - XHTML_NAMESPACE + XHTML_NAMESPACE, + SKIP_CHILDREN, + UNDEFINED } from '../constants'; import { BaseComponent, getDomSibling } from '../component'; import { Fragment } from '../create-element'; @@ -219,6 +220,7 @@ export function diff( c._force = false; let renderHook = options._render, + afterRender = options._afterRender, count = 0; if (isClassComponent) { c.state = c._nextState; @@ -228,6 +230,8 @@ export function diff( 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]); } @@ -239,6 +243,19 @@ export function diff( 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; } while (c._dirty && ++count < 25); diff --git a/src/index-5.d.ts b/src/index-5.d.ts index 7311010d71..b9a98e19b9 100644 --- a/src/index-5.d.ts +++ b/src/index-5.d.ts @@ -347,6 +347,7 @@ export interface Options { requestAnimationFrame?(callback: () => void): void; debounceRendering?(cb: () => void): void; useDebugValue?(value: string | number): void; + _afterRender?(newVNode: VNode, oldVNode: VNode): void; _addHookName?(name: string | number): 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 7733b0f279..28b0f6ff88 100644 --- a/src/internal.d.ts +++ b/src/internal.d.ts @@ -35,6 +35,8 @@ export interface Options extends preact.Options { _commit?(vnode: VNode, commitQueue: Component[]): void; /** Attach a hook that is invoked before a vnode has rendered. */ _render?(vnode: VNode): void; + /** Attach a hook that is invoked after a vnode has rendered. */ + _afterRender?(vnode: VNode, oldVNode: VNode): void; /** Attach a hook that is invoked before a hook's state is queried. */ _hook?(component: Component, index: number, type: HookType): void; /** Bypass effect execution. Currenty only used in devtools for hooks inspection */