fix[devtools/useMemoCache]: add stub for useMemoCache in ReactDebugHook #27472

hoxyq · 2023-10-06T14:18:29Z

Currently, we have this error in our logs of the internal version of React DevTools:

TypeError: Cannot read properties of undefined (reading 'memoCache')
    at Proxy.useMemoCache (chrome-extension://dnjnjgbfilfphmojnmhliehogmojhclc/build/react_devtools_backend_compact.js:151:71)

Looking at the build files of the extension, it fails here:

react/packages/react-debug-tools/src/ReactDebugHooks.js

Lines 333 to 337 in dddfe68

    
           if ( 
        
             hook !== null && 
        
             hook.updateQueue !== null && 
        
             hook.updateQueue.memoCache != null 
        
           ) {

Looks like updateQueue can be undefined, as it is not defined in hook object here:

react/packages/react-reconciler/src/ReactFiberHooks.js

Lines 180 to 186 in dddfe68

    
           export type Hook = { 
        
             memoizedState: any, 
        
             baseState: any, 
        
             baseQueue: Update<any, any> | null, 
        
             queue: any, 
        
             next: Hook | null, 
        
           };

~~Also, it looks like useMemoCache implementation doesn't expect this, so it should also result into TypeError here, line 1114:~~

react/packages/react-reconciler/src/ReactFiberHooks.js

Lines 1108 to 1115 in dddfe68

    
           function useMemoCache(size: number): Array<any> { 
        
             let memoCache = null; 
        
             // Fast-path, load memo cache from wip fiber if already prepared 
        
             let updateQueue: FunctionComponentUpdateQueue | null = 
        
               (currentlyRenderingFiber.updateQueue: any); 
        
             if (updateQueue !== null) { 
        
               memoCache = updateQueue.memoCache; 
        
             }

~~Should this also be updated?~~

react-sizebot · 2023-10-06T14:22:18Z

Comparing: 16619f1...1217280

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.min.js	=	174.46 kB	174.46 kB	=	54.27 kB	54.27 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	176.31 kB	176.31 kB	=	54.88 kB	54.88 kB
facebook-www/ReactDOM-prod.classic.js	=	564.48 kB	564.48 kB	=	99.37 kB	99.37 kB
facebook-www/ReactDOM-prod.modern.js	=	548.21 kB	548.21 kB	=	96.44 kB	96.44 kB
oss-experimental/react-debug-tools/cjs/react-debug-tools.production.min.js	=	7.13 kB	6.83 kB	=	2.66 kB	2.54 kB
oss-stable-semver/react-debug-tools/cjs/react-debug-tools.production.min.js	=	7.13 kB	6.83 kB	=	2.66 kB	2.54 kB
oss-stable/react-debug-tools/cjs/react-debug-tools.production.min.js	=	7.13 kB	6.83 kB	=	2.66 kB	2.54 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-experimental/react-debug-tools/cjs/react-debug-tools.development.js	=	23.11 kB	22.67 kB	=	6.27 kB	6.18 kB
oss-stable-semver/react-debug-tools/cjs/react-debug-tools.development.js	=	23.11 kB	22.67 kB	=	6.27 kB	6.18 kB
oss-stable/react-debug-tools/cjs/react-debug-tools.development.js	=	23.11 kB	22.67 kB	=	6.27 kB	6.18 kB
oss-experimental/react-debug-tools/cjs/react-debug-tools.production.min.js	=	7.13 kB	6.83 kB	=	2.66 kB	2.54 kB
oss-stable-semver/react-debug-tools/cjs/react-debug-tools.production.min.js	=	7.13 kB	6.83 kB	=	2.66 kB	2.54 kB
oss-stable/react-debug-tools/cjs/react-debug-tools.production.min.js	=	7.13 kB	6.83 kB	=	2.66 kB	2.54 kB

Generated by 🚫 dangerJS against 1217280

sophiebits · 2023-10-06T17:07:28Z

I think you are mixing up fiber.updateQueue and hook.updateQueue?

hoxyq · 2023-10-06T17:32:16Z

I think you are mixing up fiber.updateQueue and hook.updateQueue?

Oh, probably, but its only for the last point that I've mentioned about useMemoCache implementation, right?

sophiebits · 2023-10-06T17:35:31Z

I don't think the hook object ever has .updateQueue?

hoxyq · 2023-10-06T18:06:57Z

I don't think the hook object ever has .updateQueue?

Looks like it. For context, this field was added in #26696.

@josephsavona why aren't we using hook.memoizedState in useMemoCache implementation? Is it the only way to share value between hook's implementation and its shim in ReactDebugHooks?

josephsavona · 2023-10-09T16:47:55Z

Discussed in our sync and the conclusion was that we don't think useMemoCache should be exposed in devtools. It's an implementation detail of Forget's memoization and not a hook that users should ever be calling directly. The fact that it's a hook is also an implementation detail.

So for now we'll remove from devtools and then we'll revisit whether we should show higher-level information from Forget in devtools. For example we could in theory show information about which sets of values Forget is memoizing, and what the inputs/ouputs of each of those units are.

hoxyq · 2023-10-10T17:06:35Z

Discussed in our sync and the conclusion was that we don't think useMemoCache should be exposed in devtools. It's an implementation detail of Forget's memoization and not a hook that users should ever be calling directly. The fact that it's a hook is also an implementation detail.

So for now we'll remove from devtools and then we'll revisit whether we should show higher-level information from Forget in devtools. For example we could in theory show information about which sets of values Forget is memoizing, and what the inputs/ouputs of each of those units are.

I've updated this PR accordingly.

Now, when user inspects component, which uses useMemoCache, it won't be listed under hooks section:

Previously, we would list this hook here, because of updating hookLog object once this hook is called by dispatcher. With these changes it is just a stub now for ReactDebugHooks.

josephsavona

nice, thanks for flagging this and for updating!

…r DevTools

Update React from from 09fbee89d to a41957507. ### React upstream changes - facebook/react#27472 - facebook/react#27512 - facebook/react#27509 - facebook/react#27517 - facebook/react#27523 - facebook/react#27516 - facebook/react#27505

Changes: * fix[devtools/useMemoCache]: add stub for useMemoCache in ReactDebugHook ([hoxyq](https://github.com/hoxyq) in [#27472](#27472)) * useDeferredValue should skip initialValue if it suspends ([acdlite](https://github.com/acdlite) in [#27509](#27509)) * feat[react-devtools-extensions/logging]: initialize session id on the client for logging ([hoxyq](https://github.com/hoxyq) in [#27517](#27517)) * refactor[react-devtools-extensions]: use globals to eliminate dead code ([hoxyq](https://github.com/hoxyq) in [#27516](#27516)) * fix[devtools/inspectElement]: dont pause initial inspectElement call when user switches tabs ([hoxyq](https://github.com/hoxyq) in [#27488](#27488))

…27659) In #27472 I've removed broken `useMemoCache` implementation and replaced it with a stub. It actually produces errors when trying to inspect components, which are compiled with Forget. The main difference from the implementation in #26696 is that we are using corresponding `Fiber` here, which has patched `updateQueue` with `memoCache`. Previously we would check it on a hook object, which doesn't have `updateQueue`. Tested on pages, which are using Forget and by inspecting elements, which are transpiled with Forget.

…ok (facebook#27472) Currently, we have this error in our logs of the internal version of React DevTools: ``` TypeError: Cannot read properties of undefined (reading 'memoCache') at Proxy.useMemoCache (chrome-extension://dnjnjgbfilfphmojnmhliehogmojhclc/build/react_devtools_backend_compact.js:151:71) ``` Looking at the build files of the extension, it fails here: https://github.com/facebook/react/blob/dddfe688206dafa5646550d351eb9a8e9c53654a/packages/react-debug-tools/src/ReactDebugHooks.js#L333-L337 Looks like `updateQueue` can be `undefined`, as it is not defined in hook object here: https://github.com/facebook/react/blob/dddfe688206dafa5646550d351eb9a8e9c53654a/packages/react-reconciler/src/ReactFiberHooks.js#L180-L186 ~~Also, it looks like `useMemoCache` implementation doesn't expect this, so it should also result into TypeError here, line 1114:~~ https://github.com/facebook/react/blob/dddfe688206dafa5646550d351eb9a8e9c53654a/packages/react-reconciler/src/ReactFiberHooks.js#L1108-L1115 ~~Should this also be updated?~~

Changes: * fix[devtools/useMemoCache]: add stub for useMemoCache in ReactDebugHook ([hoxyq](https://github.com/hoxyq) in [facebook#27472](facebook#27472)) * useDeferredValue should skip initialValue if it suspends ([acdlite](https://github.com/acdlite) in [facebook#27509](facebook#27509)) * feat[react-devtools-extensions/logging]: initialize session id on the client for logging ([hoxyq](https://github.com/hoxyq) in [facebook#27517](facebook#27517)) * refactor[react-devtools-extensions]: use globals to eliminate dead code ([hoxyq](https://github.com/hoxyq) in [facebook#27516](facebook#27516)) * fix[devtools/inspectElement]: dont pause initial inspectElement call when user switches tabs ([hoxyq](https://github.com/hoxyq) in [facebook#27488](facebook#27488))

…acebook#27659) In facebook#27472 I've removed broken `useMemoCache` implementation and replaced it with a stub. It actually produces errors when trying to inspect components, which are compiled with Forget. The main difference from the implementation in facebook#26696 is that we are using corresponding `Fiber` here, which has patched `updateQueue` with `memoCache`. Previously we would check it on a hook object, which doesn't have `updateQueue`. Tested on pages, which are using Forget and by inspecting elements, which are transpiled with Forget.

…ok (#27472) Currently, we have this error in our logs of the internal version of React DevTools: ``` TypeError: Cannot read properties of undefined (reading 'memoCache') at Proxy.useMemoCache (chrome-extension://dnjnjgbfilfphmojnmhliehogmojhclc/build/react_devtools_backend_compact.js:151:71) ``` Looking at the build files of the extension, it fails here: https://github.com/facebook/react/blob/dddfe688206dafa5646550d351eb9a8e9c53654a/packages/react-debug-tools/src/ReactDebugHooks.js#L333-L337 Looks like `updateQueue` can be `undefined`, as it is not defined in hook object here: https://github.com/facebook/react/blob/dddfe688206dafa5646550d351eb9a8e9c53654a/packages/react-reconciler/src/ReactFiberHooks.js#L180-L186 ~~Also, it looks like `useMemoCache` implementation doesn't expect this, so it should also result into TypeError here, line 1114:~~ https://github.com/facebook/react/blob/dddfe688206dafa5646550d351eb9a8e9c53654a/packages/react-reconciler/src/ReactFiberHooks.js#L1108-L1115 ~~Should this also be updated?~~ DiffTrain build for commit a419575.

hoxyq requested review from poteto and josephsavona October 6, 2023 14:18

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Oct 6, 2023

hoxyq force-pushed the devtools/fix-use-memo-cache-undefined-type-error branch from 85429c7 to 7dadf27 Compare October 10, 2023 16:59

hoxyq changed the title ~~fix[devtools/useMemoCache]: updateQueue can be undefined~~ fix[devtools/useMemoCache]: add stub for useMemoCache in ReactDebugHook Oct 10, 2023

hoxyq force-pushed the devtools/fix-use-memo-cache-undefined-type-error branch from 7dadf27 to a5c6ea1 Compare October 10, 2023 17:06

josephsavona approved these changes Oct 16, 2023

View reviewed changes

fix[devtools/useMemoCache]: useMemoCache as a non-inspectable stub fo…

1217280

…r DevTools

hoxyq force-pushed the devtools/fix-use-memo-cache-undefined-type-error branch from a5c6ea1 to 1217280 Compare October 17, 2023 17:31

hoxyq merged commit a419575 into facebook:main Oct 17, 2023
2 checks passed

hoxyq deleted the devtools/fix-use-memo-cache-undefined-type-error branch October 17, 2023 17:39

ztanner mentioned this pull request Oct 17, 2023

Update React from 09fbee89d to a41957507 vercel/next.js#56970

Merged

hoxyq mentioned this pull request Oct 18, 2023

React DevTools 4.28.4 -> 4.28.5 #27538

Merged

hoxyq mentioned this pull request Nov 6, 2023

fix[devtools/useMemoCache]: implement a working copy of useMemoCache #27659

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix[devtools/useMemoCache]: add stub for useMemoCache in ReactDebugHook #27472

fix[devtools/useMemoCache]: add stub for useMemoCache in ReactDebugHook #27472

hoxyq commented Oct 6, 2023 •

edited

Loading

react-sizebot commented Oct 6, 2023 •

edited

Loading

sophiebits commented Oct 6, 2023

hoxyq commented Oct 6, 2023

sophiebits commented Oct 6, 2023

hoxyq commented Oct 6, 2023 •

edited

Loading

josephsavona commented Oct 9, 2023

hoxyq commented Oct 10, 2023

josephsavona left a comment

	if (
	hook !== null &&
	hook.updateQueue !== null &&
	hook.updateQueue.memoCache != null
	) {

	export type Hook = {
	memoizedState: any,
	baseState: any,
	baseQueue: Update<any, any> \| null,
	queue: any,
	next: Hook \| null,
	};

	function useMemoCache(size: number): Array<any> {
	let memoCache = null;
	// Fast-path, load memo cache from wip fiber if already prepared
	let updateQueue: FunctionComponentUpdateQueue \| null =
	(currentlyRenderingFiber.updateQueue: any);
	if (updateQueue !== null) {
	memoCache = updateQueue.memoCache;
	}

fix[devtools/useMemoCache]: add stub for useMemoCache in ReactDebugHook #27472

fix[devtools/useMemoCache]: add stub for useMemoCache in ReactDebugHook #27472

Conversation

hoxyq commented Oct 6, 2023 • edited Loading

react-sizebot commented Oct 6, 2023 • edited Loading

Critical size changes

Significant size changes

sophiebits commented Oct 6, 2023

hoxyq commented Oct 6, 2023

sophiebits commented Oct 6, 2023

hoxyq commented Oct 6, 2023 • edited Loading

josephsavona commented Oct 9, 2023

hoxyq commented Oct 10, 2023

josephsavona left a comment

Choose a reason for hiding this comment

hoxyq commented Oct 6, 2023 •

edited

Loading

react-sizebot commented Oct 6, 2023 •

edited

Loading

hoxyq commented Oct 6, 2023 •

edited

Loading