diff --git a/hooks/src/index.js b/hooks/src/index.js index 92a05f0a54..7af41c1183 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 'preact/src/constants'; /** @type {number} */ let currentIndex; @@ -55,10 +56,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 +179,66 @@ 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.forEach(action => { + const value = hookState._value[0]; + hookState._value[0] = hookState._reducer(value, action); + }); - currentComponent.shouldComponentUpdate = updateHookState; - } + hookState._actions = []; + // 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._updated = initialValue !== hookState._value[0]; + hookState._value = [hookState._value[0], hookState._value[1]]; + hookState._didExecute = true; } - return hookState._nextValue || hookState._value; + return hookState._value; } +const oldAfterRender = options._afterRender; +options._afterRender = newVNode => { + 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} */ + x => !!x._component && x._didExecute + ); + + if ( + stateHooksThatExecuted.length && + !stateHooksThatExecuted.some(x => x._updated) + ) { + console.log('SKIPPING'); + newVNode._flags |= SKIP_CHILDREN; + } + + stateHooksThatExecuted.forEach(hook => { + hook._updated = undefined; + hook._didExecute = false; + }); + } + + if (oldAfterRender) oldAfterRender(newVNode); +}; + /** * @param {import('./internal').Effect} callback * @param {unknown[]} args diff --git a/hooks/src/internal.d.ts b/hooks/src/internal.d.ts index 5b612dbda6..2b6b7917a7 100644 --- a/hooks/src/internal.d.ts +++ b/hooks/src/internal.d.ts @@ -74,7 +74,7 @@ export interface MemoHookState extends BaseHookState { export interface ReducerHookState extends BaseHookState { - _nextValue?: [S, StateUpdater]; + _actions: Array; _value?: [S, StateUpdater]; _component?: Component; _reducer?: Reducer; 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..8aa0be44e0 100644 --- a/hooks/test/browser/useState.test.js +++ b/hooks/test/browser/useState.test.js @@ -51,6 +51,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 +71,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 +307,11 @@ describe('useState', () => { let context = { modalCount, addModal() { + console.log('adding'); setModalCount(count => count + 1); }, removeModal() { + console.log('removing'); setModalCount(count => count - 1); } }; @@ -328,11 +332,13 @@ describe('useState', () => { } function Popover() { + console.log('POPOVER'); useModal(); return
Popover
; } function App() { + console.log('render app'); return ( @@ -344,7 +350,7 @@ describe('useState', () => { render(, scratch); }); - expect(renderSpy).to.be.calledTwice; + expect(renderSpy).to.be.calledThrice; }); // see preactjs/preact#3731 diff --git a/mangle.json b/mangle.json index 42b128bf62..4d8b3355f2 100644 --- a/mangle.json +++ b/mangle.json @@ -71,6 +71,7 @@ "$_diff": "__b", "$_commit": "__c", "$_addHookName": "__a", + "$_afterRender": "__a", "$_render": "__r", "$_hook": "__h", "$_catchError": "__e", diff --git a/src/constants.js b/src/constants.js index 283c2b39e6..620b095cfa 100644 --- a/src/constants.js +++ b/src/constants.js @@ -7,6 +7,8 @@ export const INSERT_VNODE = 1 << 16; /** Indicates a VNode has been matched with another VNode in the diff */ export const MATCHED = 1 << 17; +export const SKIP_CHILDREN = 1 << 18; + /** 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 7e7ee8343f..9b924039ef 100644 --- a/src/diff/index.js +++ b/src/diff/index.js @@ -3,6 +3,7 @@ import { MODE_HYDRATE, MODE_SUSPENDED, RESET_MODE, + SKIP_CHILDREN, UNDEFINED } from '../constants'; import { BaseComponent, getDomSibling } from '../component'; @@ -204,6 +205,7 @@ export function diff( c._force = false; let renderHook = options._render, + afterRender = options._afterRender, count = 0; if (isClassComponent) { c.state = c._nextState; @@ -213,6 +215,8 @@ export function diff( tmp = c.render(c.props, c.state, c.context); + if (afterRender) afterRender(newVNode); + for (let i = 0; i < c._stateCallbacks.length; i++) { c._renderCallbacks.push(c._stateCallbacks[i]); } @@ -224,6 +228,22 @@ export function diff( tmp = c.render(c.props, c.state, c.context); + if (afterRender) afterRender(newVNode); + + if (newVNode._flags & SKIP_CHILDREN) { + c._dirty = false; + c._renderCallbacks = []; + // TODO: we'd need to clear pending effects + // from hooks + c.__hooks && (c.__hooks._pendingEffects = []); + 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);