Skip to content

Commit

Permalink
fix memo doing the wrong thing
Browse files Browse the repository at this point in the history
  • Loading branch information
JoviDeCroock committed Jun 26, 2024
1 parent eb37677 commit d0daefa
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 6 deletions.
7 changes: 3 additions & 4 deletions hooks/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ options._render = vnode => {
hookItem._value = hookItem._nextValue;
}
hookItem._pendingValue = EMPTY;
hookItem._nextValue = hookItem._pendingArgs = undefined;
hookItem._pendingArgs = hookItem._nextValue = undefined;
});
} else {
hooks._pendingEffects.forEach(invokeCleanup);
Expand Down Expand Up @@ -350,10 +350,9 @@ export function useMemo(factory, args) {
/** @type {import('./internal').MemoHookState<T>} */
const state = getHookState(currentIndex++, 7);
if (argsChanged(state._args, args)) {
state._pendingValue = factory();
state._pendingArgs = args;
state._value = factory();
state._args = args;
state._factory = factory;
return state._pendingValue;
}

return state._value;
Expand Down
40 changes: 40 additions & 0 deletions hooks/test/browser/combinations.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -477,4 +477,44 @@ describe('combinations', () => {
'anchor effect'
]);
});

it('should not loop infinitely', () => {
const actions = [];
let toggle;
function App() {
const [value, setValue] = useState(false);

const data = useMemo(() => {
actions.push('memo');
return {};
}, [value]);

const [prevData, setPreviousData] = useState(data);
if (prevData !== data) {
setPreviousData(data);
}

actions.push('render');
toggle = () => setValue(!value);
return <div>Value: {JSON.stringify(value)}</div>;
}

act(() => {
render(<App />, scratch);
});
expect(actions).to.deep.equal(['memo', 'render']);
expect(scratch.innerHTML).to.deep.equal('<div>Value: false</div>');

act(() => {
toggle();
});
expect(actions).to.deep.equal([
'memo',
'render',
'memo',
'render',
'render'
]);
expect(scratch.innerHTML).to.deep.equal('<div>Value: true</div>');
});
});
7 changes: 5 additions & 2 deletions hooks/test/browser/useMemo.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,12 +151,13 @@ describe('useMemo', () => {
act(() => {
set('bye');
});
expect(calls.length).to.equal(5);
expect(calls.length).to.equal(6);
expect(calls).to.deep.equal([
'doing memo',
'render hi',
'doing memo',
'render bye', // We expect a missing "doing memo" here because we return to the previous args value
'render bye',
'doing memo',
'render hi'
]);
});
Expand All @@ -172,6 +173,8 @@ describe('useMemo', () => {
if (v === 0) {
set(v + 1);
}

console.log('hi', res);
return <p>{res}</p>;
}

Expand Down

0 comments on commit d0daefa

Please sign in to comment.