Skip to content

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Aug 14, 2024

Stacked on #30662.

Alternative to #30663 and #30677.

During a Fast Refresh, we always want to evict the memo cache, same as we do with normal useMemo. The mechanism used by useMemo and other Hooks is this module-level variable:

// In DEV, this tracks whether currently rendering component needs to ignore
// the dependencies for Hooks that need them (e.g. useEffect or useMemo).
// When true, such Hooks will always be "remounted". Only used during hot reload.
let ignorePreviousDependencies: boolean = false;

which has DEV-only behavior as if the dependencies are always different:

function areHookInputsEqual(
nextDeps: Array<mixed>,
prevDeps: Array<mixed> | null,
): boolean {
if (__DEV__) {
if (ignorePreviousDependencies) {
// Only true when this component is being hot reloaded.
return false;
}
}

The useMemoCache Hook doesn't use a dependency array but conceptually I think we want the same behavior.

Test Plan

The test passes.

poteto and others added 10 commits August 12, 2024 11:39
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
Copy link

vercel bot commented Aug 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 14, 2024 9:51pm

@react-sizebot
Copy link

react-sizebot commented Aug 14, 2024

Comparing: d9eb154...60ed96b

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.68 kB 6.68 kB = 1.82 kB 1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 500.37 kB 500.37 kB = 89.80 kB 89.80 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 507.50 kB 507.50 kB = 90.96 kB 90.96 kB
facebook-www/ReactDOM-prod.classic.js = 595.24 kB 595.24 kB = 105.55 kB 105.55 kB
facebook-www/ReactDOM-prod.modern.js = 571.54 kB 571.54 kB = 101.75 kB 101.75 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 4d86c1b

Copy link
Member

@josephsavona josephsavona left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

amazing!

Copy link
Member

@poteto poteto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh nice, yeah this makes sense if we already had this infra from before

@gaearon gaearon merged commit 7e8a06c into main Aug 15, 2024
@gaearon gaearon deleted the fixmemocache-fr branch August 15, 2024 00:03
github-actions bot pushed a commit that referenced this pull request Aug 15, 2024
Stacked on #30662.

Alternative to #30663 and
#30677.

During a Fast Refresh, we always want to evict the memo cache, same as
we do with normal `useMemo`. The mechanism used by `useMemo` and other
Hooks is this module-level variable:

https://github.com/facebook/react/blob/fca5d655d78917400a2722287351c20938166669/packages/react-reconciler/src/ReactFiberHooks.js#L304-L307

which has DEV-only behavior as if the dependencies are always different:

https://github.com/facebook/react/blob/fca5d655d78917400a2722287351c20938166669/packages/react-reconciler/src/ReactFiberHooks.js#L451-L460

The `useMemoCache` Hook doesn't use a dependency array but conceptually
I think we want the same behavior.

## Test Plan

The test passes.

---------

Co-authored-by: Lauren Tan <poteto@users.noreply.github.com>

DiffTrain build for [7e8a06c](7e8a06c)
github-actions bot pushed a commit that referenced this pull request Aug 15, 2024
Stacked on #30662.

Alternative to #30663 and
#30677.

During a Fast Refresh, we always want to evict the memo cache, same as
we do with normal `useMemo`. The mechanism used by `useMemo` and other
Hooks is this module-level variable:

https://github.com/facebook/react/blob/fca5d655d78917400a2722287351c20938166669/packages/react-reconciler/src/ReactFiberHooks.js#L304-L307

which has DEV-only behavior as if the dependencies are always different:

https://github.com/facebook/react/blob/fca5d655d78917400a2722287351c20938166669/packages/react-reconciler/src/ReactFiberHooks.js#L451-L460

The `useMemoCache` Hook doesn't use a dependency array but conceptually
I think we want the same behavior.

## Test Plan

The test passes.

---------

Co-authored-by: Lauren Tan <poteto@users.noreply.github.com>

DiffTrain build for commit 7e8a06c.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants