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.

Sign up for GitHub

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

[flags] Cleanup enableUseMemoCacheHook #31767

Merged
merged 6 commits into from
Dec 14, 2024
Merged

[flags] Cleanup enableUseMemoCacheHook #31767

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d96eecb
Remove enableComponentStackLocations
rickhanlonii Dec 13, 2024
995beaf
Remove enableFilterEmptyStringAttributesDOM
rickhanlonii Dec 13, 2024
ad3968d
Merge branch 'main' into rh/rm-ff-filter-strings
rickhanlonii Dec 13, 2024
dbcb514
Remove enableFlightReadableStream
rickhanlonii Dec 13, 2024
fb464f5
Remove enableUseMemoCacheHook
rickhanlonii Dec 13, 2024
8cc0a1b
Merge branch 'main' into rh/rm-ff-uMC
rickhanlonii Dec 13, 2024
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
2 changes: 0 additions & 2 deletions packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1573,7 +1573,6 @@ describe('ReactHooksInspectionIntegration', () => {
});

describe('useMemoCache', () => {
// @gate enableUseMemoCacheHook
it('should not be inspectable', async () => {
function Foo() {
const $ = useMemoCache(1);
Expand Down Expand Up @@ -1601,7 +1600,6 @@ describe('ReactHooksInspectionIntegration', () => {
expect(tree.length).toEqual(0);
});

// @gate enableUseMemoCacheHook
it('should work in combination with other hooks', async () => {
function useSomething() {
const [something] = React.useState(null);
Expand Down
107 changes: 32 additions & 75 deletions packages/react-reconciler/src/ReactFiberHooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import {
enableCache,
enableLazyContextPropagation,
enableTransitionTracing,
enableUseMemoCacheHook,
enableUseEffectEventHook,
enableLegacyCache,
debugRenderPhaseSideEffectsForStrictMode,
Expand Down Expand Up @@ -277,8 +276,7 @@ export type FunctionComponentUpdateQueue = {
lastEffect: Effect | null,
events: Array<EventFunctionPayload<any, any, any>> | null,
stores: Array<StoreConsistencyCheck<any>> | null,
// NOTE: optional, only set when enableUseMemoCacheHook is enabled
memoCache?: MemoCache | null,
memoCache: MemoCache | null,
};

type BasicStateAction<S> = (S => S) | S;
Expand Down Expand Up @@ -1127,25 +1125,12 @@ function unstable_useContextWithBailout<T>(
return readContextAndCompare(context, select);
}

// NOTE: defining two versions of this function to avoid size impact when this feature is disabled.
// Previously this function was inlined, the additional `memoCache` property makes it not inlined.
let createFunctionComponentUpdateQueue: () => FunctionComponentUpdateQueue;
if (enableUseMemoCacheHook) {
createFunctionComponentUpdateQueue = () => {
return {
lastEffect: null,
events: null,
stores: null,
memoCache: null,
};
};
} else {
createFunctionComponentUpdateQueue = () => {
return {
lastEffect: null,
events: null,
stores: null,
};
function createFunctionComponentUpdateQueue(): FunctionComponentUpdateQueue {
return {
lastEffect: null,
events: null,
stores: null,
memoCache: null,
};
}

Expand All @@ -1155,13 +1140,11 @@ function resetFunctionComponentUpdateQueue(
updateQueue.lastEffect = null;
updateQueue.events = null;
updateQueue.stores = null;
if (enableUseMemoCacheHook) {
if (updateQueue.memoCache != null) {
// NOTE: this function intentionally does not reset memoCache data. We reuse updateQueue for the memo
// cache to avoid increasing the size of fibers that don't need a cache, but we don't want to reset
// the cache when other properties are reset.
updateQueue.memoCache.index = 0;
}
if (updateQueue.memoCache != null) {
// NOTE: this function intentionally does not reset memoCache data. We reuse updateQueue for the memo
// cache to avoid increasing the size of fibers that don't need a cache, but we don't want to reset
// the cache when other properties are reset.
updateQueue.memoCache.index = 0;
}
}

Expand Down Expand Up @@ -3982,13 +3965,11 @@ export const ContextOnlyDispatcher: Dispatcher = {
useFormState: throwInvalidHookError,
useActionState: throwInvalidHookError,
useOptimistic: throwInvalidHookError,
useMemoCache: throwInvalidHookError,
};
if (enableCache) {
(ContextOnlyDispatcher: Dispatcher).useCacheRefresh = throwInvalidHookError;
}
if (enableUseMemoCacheHook) {
(ContextOnlyDispatcher: Dispatcher).useMemoCache = throwInvalidHookError;
}
if (enableUseEffectEventHook) {
(ContextOnlyDispatcher: Dispatcher).useEffectEvent = throwInvalidHookError;
}
Expand Down Expand Up @@ -4023,13 +4004,11 @@ const HooksDispatcherOnMount: Dispatcher = {
useFormState: mountActionState,
useActionState: mountActionState,
useOptimistic: mountOptimistic,
useMemoCache,
};
if (enableCache) {
(HooksDispatcherOnMount: Dispatcher).useCacheRefresh = mountRefresh;
}
if (enableUseMemoCacheHook) {
(HooksDispatcherOnMount: Dispatcher).useMemoCache = useMemoCache;
}
if (enableUseEffectEventHook) {
(HooksDispatcherOnMount: Dispatcher).useEffectEvent = mountEvent;
}
Expand Down Expand Up @@ -4064,13 +4043,11 @@ const HooksDispatcherOnUpdate: Dispatcher = {
useFormState: updateActionState,
useActionState: updateActionState,
useOptimistic: updateOptimistic,
useMemoCache,
};
if (enableCache) {
(HooksDispatcherOnUpdate: Dispatcher).useCacheRefresh = updateRefresh;
}
if (enableUseMemoCacheHook) {
(HooksDispatcherOnUpdate: Dispatcher).useMemoCache = useMemoCache;
}
if (enableUseEffectEventHook) {
(HooksDispatcherOnUpdate: Dispatcher).useEffectEvent = updateEvent;
}
Expand Down Expand Up @@ -4106,13 +4083,11 @@ const HooksDispatcherOnRerender: Dispatcher = {
useFormState: rerenderActionState,
useActionState: rerenderActionState,
useOptimistic: rerenderOptimistic,
useMemoCache,
};
if (enableCache) {
(HooksDispatcherOnRerender: Dispatcher).useCacheRefresh = updateRefresh;
}
if (enableUseMemoCacheHook) {
(HooksDispatcherOnRerender: Dispatcher).useMemoCache = useMemoCache;
}
if (enableUseEffectEventHook) {
(HooksDispatcherOnRerender: Dispatcher).useEffectEvent = updateEvent;
}
Expand Down Expand Up @@ -4307,6 +4282,7 @@ if (__DEV__) {
return mountOptimistic(passthrough, reducer);
},
useHostTransitionStatus,
useMemoCache,
};
if (enableCache) {
(HooksDispatcherOnMountInDEV: Dispatcher).useCacheRefresh =
Expand All @@ -4316,9 +4292,6 @@ if (__DEV__) {
return mountRefresh();
};
}
if (enableUseMemoCacheHook) {
(HooksDispatcherOnMountInDEV: Dispatcher).useMemoCache = useMemoCache;
}
if (enableUseEffectEventHook) {
(HooksDispatcherOnMountInDEV: Dispatcher).useEffectEvent =
function useEffectEvent<Args, Return, F: (...Array<Args>) => Return>(
Expand Down Expand Up @@ -4511,6 +4484,7 @@ if (__DEV__) {
return mountOptimistic(passthrough, reducer);
},
useHostTransitionStatus,
useMemoCache,
};
if (enableCache) {
(HooksDispatcherOnMountWithHookTypesInDEV: Dispatcher).useCacheRefresh =
Expand All @@ -4520,10 +4494,6 @@ if (__DEV__) {
return mountRefresh();
};
}
if (enableUseMemoCacheHook) {
(HooksDispatcherOnMountWithHookTypesInDEV: Dispatcher).useMemoCache =
useMemoCache;
}
if (enableUseEffectEventHook) {
(HooksDispatcherOnMountWithHookTypesInDEV: Dispatcher).useEffectEvent =
function useEffectEvent<Args, Return, F: (...Array<Args>) => Return>(
Expand Down Expand Up @@ -4715,6 +4685,7 @@ if (__DEV__) {
return updateOptimistic(passthrough, reducer);
},
useHostTransitionStatus,
useMemoCache,
};
if (enableCache) {
(HooksDispatcherOnUpdateInDEV: Dispatcher).useCacheRefresh =
Expand All @@ -4724,9 +4695,6 @@ if (__DEV__) {
return updateRefresh();
};
}
if (enableUseMemoCacheHook) {
(HooksDispatcherOnUpdateInDEV: Dispatcher).useMemoCache = useMemoCache;
}
if (enableUseEffectEventHook) {
(HooksDispatcherOnUpdateInDEV: Dispatcher).useEffectEvent =
function useEffectEvent<Args, Return, F: (...Array<Args>) => Return>(
Expand Down Expand Up @@ -4918,6 +4886,7 @@ if (__DEV__) {
return rerenderOptimistic(passthrough, reducer);
},
useHostTransitionStatus,
useMemoCache,
};
if (enableCache) {
(HooksDispatcherOnRerenderInDEV: Dispatcher).useCacheRefresh =
Expand All @@ -4927,9 +4896,6 @@ if (__DEV__) {
return updateRefresh();
};
}
if (enableUseMemoCacheHook) {
(HooksDispatcherOnRerenderInDEV: Dispatcher).useMemoCache = useMemoCache;
}
if (enableUseEffectEventHook) {
(HooksDispatcherOnRerenderInDEV: Dispatcher).useEffectEvent =
function useEffectEvent<Args, Return, F: (...Array<Args>) => Return>(
Expand Down Expand Up @@ -5141,6 +5107,10 @@ if (__DEV__) {
mountHookTypesDev();
return mountOptimistic(passthrough, reducer);
},
useMemoCache(size: number): Array<any> {
warnInvalidHookAccess();
return useMemoCache(size);
},
useHostTransitionStatus,
};
if (enableCache) {
Expand All @@ -5151,13 +5121,6 @@ if (__DEV__) {
return mountRefresh();
};
}
if (enableUseMemoCacheHook) {
(InvalidNestedHooksDispatcherOnMountInDEV: Dispatcher).useMemoCache =
function (size: number): Array<any> {
warnInvalidHookAccess();
return useMemoCache(size);
};
}
if (enableUseEffectEventHook) {
(InvalidNestedHooksDispatcherOnMountInDEV: Dispatcher).useEffectEvent =
function useEffectEvent<Args, Return, F: (...Array<Args>) => Return>(
Expand Down Expand Up @@ -5372,6 +5335,10 @@ if (__DEV__) {
updateHookTypesDev();
return updateOptimistic(passthrough, reducer);
},
useMemoCache(size: number): Array<any> {
warnInvalidHookAccess();
return useMemoCache(size);
},
useHostTransitionStatus,
};
if (enableCache) {
Expand All @@ -5382,13 +5349,6 @@ if (__DEV__) {
return updateRefresh();
};
}
if (enableUseMemoCacheHook) {
(InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher).useMemoCache =
function (size: number): Array<any> {
warnInvalidHookAccess();
return useMemoCache(size);
};
}
if (enableUseEffectEventHook) {
(InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher).useEffectEvent =
function useEffectEvent<Args, Return, F: (...Array<Args>) => Return>(
Expand Down Expand Up @@ -5603,6 +5563,10 @@ if (__DEV__) {
updateHookTypesDev();
return rerenderOptimistic(passthrough, reducer);
},
useMemoCache(size: number): Array<any> {
warnInvalidHookAccess();
return useMemoCache(size);
},
useHostTransitionStatus,
};
if (enableCache) {
Expand All @@ -5613,13 +5577,6 @@ if (__DEV__) {
return updateRefresh();
};
}
if (enableUseMemoCacheHook) {
(InvalidNestedHooksDispatcherOnRerenderInDEV: Dispatcher).useMemoCache =
function (size: number): Array<any> {
warnInvalidHookAccess();
return useMemoCache(size);
};
}
if (enableUseEffectEventHook) {
(InvalidNestedHooksDispatcherOnRerenderInDEV: Dispatcher).useEffectEvent =
function useEffectEvent<Args, Return, F: (...Array<Args>) => Return>(
Expand Down
7 changes: 0 additions & 7 deletions packages/react-reconciler/src/__tests__/useMemoCache-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ describe('useMemoCache()', () => {
ErrorBoundary = _ErrorBoundary;
});

// @gate enableUseMemoCacheHook
it('render component using cache', async () => {
function Component(props) {
const cache = useMemoCache(1);
Expand All @@ -75,7 +74,6 @@ describe('useMemoCache()', () => {
expect(root).toMatchRenderedOutput('Ok');
});

// @gate enableUseMemoCacheHook
it('update component using cache', async () => {
let setX;
let forceUpdate;
Expand Down Expand Up @@ -145,7 +143,6 @@ describe('useMemoCache()', () => {
expect(data).toBe(data1); // confirm that the cache persisted across renders
});

// @gate enableUseMemoCacheHook
it('update component using cache with setstate during render', async () => {
let setN;
function Component(props) {
Expand Down Expand Up @@ -210,7 +207,6 @@ describe('useMemoCache()', () => {
expect(data).toBe(data0);
});

// @gate enableUseMemoCacheHook
it('update component using cache with throw during render', async () => {
let setN;
let shouldFail = true;
Expand Down Expand Up @@ -293,7 +289,6 @@ describe('useMemoCache()', () => {
expect(data).toBe(data1); // confirm that the cache persisted across renders
});

// @gate enableUseMemoCacheHook
it('update component and custom hook with caches', async () => {
let setX;
let forceUpdate;
Expand Down Expand Up @@ -370,7 +365,6 @@ describe('useMemoCache()', () => {
expect(data).toBe(data1); // confirm that the cache persisted across renders
});

// @gate enableUseMemoCacheHook
it('reuses computations from suspended/interrupted render attempts during an update', async () => {
// This test demonstrates the benefit of a shared memo cache. By "shared" I
// mean multiple concurrent render attempts of the same component/hook use
Expand Down Expand Up @@ -624,7 +618,6 @@ describe('useMemoCache()', () => {
);
});

// @gate enableUseMemoCacheHook
it('(repro) infinite renders when used with setState during render', async () => {
// Output of react compiler on `useUserMemo`
function useCompilerMemo(value) {
Expand Down
5 changes: 1 addition & 4 deletions packages/react-server/src/ReactFizzHooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import {createFastHash} from './ReactServerStreamConfig';
import {
enableCache,
enableUseEffectEventHook,
enableUseMemoCacheHook,
enableUseResourceEffectHook,
} from 'shared/ReactFeatureFlags';
import is from 'shared/objectIs';
Expand Down Expand Up @@ -859,6 +858,7 @@ export const HooksDispatcher: Dispatcher = supportsClientAPIs
useActionState,
useFormState: useActionState,
useHostTransitionStatus,
useMemoCache,
Copy link
Contributor

Choose a reason for hiding this comment

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

@rickhanlonii You seem to have forgotten to add this after L837. ☝️

};

if (enableCache) {
Expand All @@ -867,9 +867,6 @@ if (enableCache) {
if (enableUseEffectEventHook) {
HooksDispatcher.useEffectEvent = useEffectEvent;
}
if (enableUseMemoCacheHook) {
HooksDispatcher.useMemoCache = useMemoCache;
}
if (enableUseResourceEffectHook) {
HooksDispatcher.useResourceEffect = supportsClientAPIs
? noop
Expand Down
3 changes: 0 additions & 3 deletions packages/shared/ReactFeatureFlags.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,6 @@ export const enableCPUSuspense = __EXPERIMENTAL__;

export const enableHydrationLaneScheduling = true;

// Enables useMemoCache hook, intended as a compilation target for
// auto-memoization.
export const enableUseMemoCacheHook = true;
// Test this at Meta before enabling.
export const enableNoCloningMemoCache = false;

Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-fb.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ export const enableTransitionTracing = false;
export const enableTrustedTypesIntegration = false;
export const enableUpdaterTracking = __PROFILE__;
export const enableUseEffectEventHook = false;
export const enableUseMemoCacheHook = true;
export const favorSafetyOverHydrationPerf = true;
export const renameElementSymbol = false;
export const retryLaneExpirationMs = 5000;
Expand Down
Loading
Loading