Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fresh] Reset useMemoCache when size changes #30663

Closed
wants to merge 11 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 7 additions & 11 deletions packages/react-reconciler/src/ReactFiberHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -1226,21 +1226,17 @@ function useMemoCache(size: number): Array<any> {
updateQueue.memoCache = memoCache;

let data = memoCache.data[memoCache.index];
if (data === undefined) {
// We reset the cache on init, or indiscriminately even in the event that the cache size requested
// differs from the previous render. In dev environments this is typically due to FastRefresh,
// where editing the code can change the cache size and should reset entirely to prevent stale
// values from being reused. In prod environments this is never expected to happen. However, in
// the unlikely case that it does vary between renders, we reset the cache anyway so behavior is
// consistent in both environments.
if (data === undefined || data.length !== size) {
data = memoCache.data[memoCache.index] = new Array(size);
for (let i = 0; i < size; i++) {
data[i] = REACT_MEMO_CACHE_SENTINEL;
}
} else if (data.length !== size) {
// TODO: consider warning or throwing here
if (__DEV__) {
console.error(
'Expected a constant size argument for each invocation of useMemoCache. ' +
'The previous cache was allocated with size %s but size %s was requested.',
data.length,
size,
);
}
}
memoCache.index++;
return data;
Expand Down
33 changes: 33 additions & 0 deletions packages/react-reconciler/src/__tests__/useMemoCache-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -621,4 +621,37 @@ describe('useMemoCache()', () => {
</>,
);
});

// @gate enableUseMemoCacheHook
it('resets cache when cache size varies between renders', async () => {
const obj = {};
function Component(props) {
const $ = useMemoCache(props.cacheSize);
let cacheHit = 0;
if ($[0] === obj) {
cacheHit = 1;
} else {
$[0] = obj;
}
return cacheHit;
}
const root = ReactNoop.createRoot();
await act(() => {
root.render(<Component key="comp" cacheSize={1} />);
});
// Initial render, cache is empty
expect(root).toMatchRenderedOutput('0');

await act(() => {
root.render(<Component key="comp" cacheSize={1} />);
});
// Re-render, re-use cache since obj identity has not changed
expect(root).toMatchRenderedOutput('1');

await act(() => {
root.render(<Component key="comp" cacheSize={10} />);
});
// Cache size changed b/w renders, reset the whole cache
expect(root).toMatchRenderedOutput('0');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -1637,8 +1637,7 @@ describe('ReactFreshIntegration', () => {
}
});

// eslint-disable-next-line jest/no-disabled-tests
it.skip('resets useMemoCache cache slots', async () => {
it('resets useMemoCache cache slots', async () => {
if (__DEV__) {
await render(`
const useMemoCache = require('react/compiler-runtime').c;
Expand Down
Loading