Skip to content

Commit

Permalink
Delay executing state updates
Browse files Browse the repository at this point in the history
  • Loading branch information
JoviDeCroock committed Nov 29, 2024
1 parent db47ab6 commit 340bd1a
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 89 deletions.
128 changes: 48 additions & 80 deletions hooks/src/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { options as _options } from 'preact';
import { SKIP_CHILDREN } from 'preact/src/constants';

/** @type {number} */
let currentIndex;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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];

Check failure on line 208 in hooks/src/index.js

View workflow job for this annotation

GitHub Actions / Build & Test / Build & Test

Property '_updated' does not exist on type 'ReducerHookState<any, any>'.
hookState._value = [hookState._value[0], hookState._value[1]];
hookState._didExecute = true;

Check failure on line 210 in hooks/src/index.js

View workflow job for this annotation

GitHub Actions / Build & Test / Build & Test

Property '_didExecute' does not exist on type 'ReducerHookState<any, any>'.
}

return hookState._nextValue || hookState._value;
return hookState._value;
}

const oldAfterRender = options._afterRender;

Check failure on line 216 in hooks/src/index.js

View workflow job for this annotation

GitHub Actions / Build & Test / Build & Test

Property '_afterRender' does not exist on type 'Options'.
options._afterRender = newVNode => {

Check failure on line 217 in hooks/src/index.js

View workflow job for this annotation

GitHub Actions / Build & Test / Build & Test

Property '_afterRender' does not exist on type 'Options'.
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

Check failure on line 222 in hooks/src/index.js

View workflow job for this annotation

GitHub Actions / Build & Test / Build & Test

Property '_didExecute' does not exist on type 'HookState'.
);

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
Expand Down
2 changes: 1 addition & 1 deletion hooks/src/internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export interface MemoHookState<T = unknown> extends BaseHookState {

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>;
Expand Down
13 changes: 8 additions & 5 deletions hooks/test/browser/useReducer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}>
Expand Down
12 changes: 9 additions & 3 deletions hooks/test/browser/useState.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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);
}
};
Expand All @@ -328,11 +332,13 @@ describe('useState', () => {
}

function Popover() {
console.log('POPOVER');
useModal();
return <div>Popover</div>;
}

function App() {
console.log('render app');
return (
<ModalProvider>
<Popover />
Expand All @@ -344,7 +350,7 @@ describe('useState', () => {
render(<App />, scratch);
});

expect(renderSpy).to.be.calledTwice;
expect(renderSpy).to.be.calledThrice;
});

// see preactjs/preact#3731
Expand Down
1 change: 1 addition & 0 deletions mangle.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
"$_diff": "__b",
"$_commit": "__c",
"$_addHookName": "__a",
"$_afterRender": "__a",
"$_render": "__r",
"$_hook": "__h",
"$_catchError": "__e",
Expand Down
2 changes: 2 additions & 0 deletions src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
20 changes: 20 additions & 0 deletions src/diff/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
MODE_HYDRATE,
MODE_SUSPENDED,
RESET_MODE,
SKIP_CHILDREN,
UNDEFINED
} from '../constants';
import { BaseComponent, getDomSibling } from '../component';
Expand Down Expand Up @@ -204,6 +205,7 @@ export function diff(
c._force = false;

let renderHook = options._render,
afterRender = options._afterRender,

Check failure on line 208 in src/diff/index.js

View workflow job for this annotation

GitHub Actions / Build & Test / Build & Test

Property '_afterRender' does not exist on type 'Options'.
count = 0;
if (isClassComponent) {
c.state = c._nextState;
Expand All @@ -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]);
}
Expand All @@ -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);
Expand Down

0 comments on commit 340bd1a

Please sign in to comment.