From d4469c19900e15e417ac78e4a653c7b70108f3a5 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Tue, 11 Oct 2022 16:18:30 -0700 Subject: [PATCH] try/finally simplification --- .../src/ReactFiberHooks.new.js | 25 ++-- .../src/ReactFiberHooks.old.js | 25 ++-- .../src/ReactInternalTypes.js | 5 +- .../react-reconciler/src/ReactMemoCache.js | 59 ---------- .../src/__tests__/useMemoCache-test.js | 110 +++++++----------- packages/react-server/src/ReactFizzHooks.js | 10 +- packages/react-server/src/ReactFlightHooks.js | 9 +- packages/react/src/ReactHooks.js | 3 +- packages/shared/ReactTypes.js | 35 +----- scripts/error-codes/codes.json | 5 +- 10 files changed, 88 insertions(+), 198 deletions(-) delete mode 100644 packages/react-reconciler/src/ReactMemoCache.js diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index b662c31eb3035..9c40300b68ffa 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -8,7 +8,6 @@ */ import type { - MemoCache, MutableSource, MutableSourceGetSnapshotFn, MutableSourceSubscribeFn, @@ -21,7 +20,7 @@ import type { Fiber, Dispatcher, HookType, - MemoCacheStorage, + MemoCache, EventFunctionWrapper, } from './ReactInternalTypes'; import type {Lanes, Lane} from './ReactFiberLane.new'; @@ -49,7 +48,6 @@ import { REACT_CONTEXT_TYPE, REACT_SERVER_CONTEXT_TYPE, } from 'shared/ReactSymbols'; -import {createMemoCache, MEMO_CACHE_UNSET_SENTINEL} from './ReactMemoCache'; import { NoMode, @@ -197,7 +195,7 @@ export type FunctionComponentUpdateQueue = { events: Array<() => mixed> | null, stores: Array> | null, // NOTE: optional, only set when enableUseMemoCacheHook is enabled - memoCache?: MemoCacheStorage | null, + memoCache?: MemoCache | null, }; type BasicStateAction = (S => S) | S; @@ -840,7 +838,10 @@ function use(usable: Usable): T { throw new Error('An unsupported type was passed to use(): ' + String(usable)); } -function useMemoCache(size: number): MemoCache { +const MEMO_CACHE_UNSET_SENTINEL: symbol = Symbol.for( + 'react.usememocache_sentinel', +); +function useMemoCache(size: number): Array { let memoCache = null; // Fast-path, load memo cache from wip fiber if already prepared let updateQueue: FunctionComponentUpdateQueue | null = (currentlyRenderingFiber.updateQueue: any); @@ -853,8 +854,7 @@ function useMemoCache(size: number): MemoCache { if (current !== null) { const currentUpdateQueue: FunctionComponentUpdateQueue | null = (current.updateQueue: any); if (currentUpdateQueue !== null) { - const currentMemoCache: ?MemoCacheStorage = - currentUpdateQueue.memoCache; + const currentMemoCache: ?MemoCache = currentUpdateQueue.memoCache; if (currentMemoCache != null) { memoCache = { data: currentMemoCache.data.map(array => array.slice()), @@ -882,6 +882,9 @@ function useMemoCache(size: number): MemoCache { data = memoCache.data[memoCache.index] = new Array(size).fill( MEMO_CACHE_UNSET_SENTINEL, ); + // Attach sentinel to the array, eventually we can make the sentinel a public + // export from React and avoid modifying the array + (data: any)._ = MEMO_CACHE_UNSET_SENTINEL; } else if (data.length !== size) { // TODO: consider warning or throwing here if (__DEV__) { @@ -894,7 +897,7 @@ function useMemoCache(size: number): MemoCache { } } memoCache.index++; - return createMemoCache(data); + return data; } function basicStateReducer(state: S, action: BasicStateAction): S { @@ -3611,7 +3614,7 @@ if (__DEV__) { if (enableUseMemoCacheHook) { (InvalidNestedHooksDispatcherOnMountInDEV: Dispatcher).useMemoCache = function( size: number, - ): MemoCache { + ): Array { warnInvalidHookAccess(); return useMemoCache(size); }; @@ -3798,7 +3801,7 @@ if (__DEV__) { if (enableUseMemoCacheHook) { (InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher).useMemoCache = function( size: number, - ): MemoCache { + ): Array { warnInvalidHookAccess(); return useMemoCache(size); }; @@ -3986,7 +3989,7 @@ if (__DEV__) { if (enableUseMemoCacheHook) { (InvalidNestedHooksDispatcherOnRerenderInDEV: Dispatcher).useMemoCache = function( size: number, - ): MemoCache { + ): Array { warnInvalidHookAccess(); return useMemoCache(size); }; diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index 5afa27651f453..e1de9e1c49f96 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -8,7 +8,6 @@ */ import type { - MemoCache, MutableSource, MutableSourceGetSnapshotFn, MutableSourceSubscribeFn, @@ -21,7 +20,7 @@ import type { Fiber, Dispatcher, HookType, - MemoCacheStorage, + MemoCache, EventFunctionWrapper, } from './ReactInternalTypes'; import type {Lanes, Lane} from './ReactFiberLane.old'; @@ -49,7 +48,6 @@ import { REACT_CONTEXT_TYPE, REACT_SERVER_CONTEXT_TYPE, } from 'shared/ReactSymbols'; -import {createMemoCache, MEMO_CACHE_UNSET_SENTINEL} from './ReactMemoCache'; import { NoMode, @@ -197,7 +195,7 @@ export type FunctionComponentUpdateQueue = { events: Array<() => mixed> | null, stores: Array> | null, // NOTE: optional, only set when enableUseMemoCacheHook is enabled - memoCache?: MemoCacheStorage | null, + memoCache?: MemoCache | null, }; type BasicStateAction = (S => S) | S; @@ -840,7 +838,10 @@ function use(usable: Usable): T { throw new Error('An unsupported type was passed to use(): ' + String(usable)); } -function useMemoCache(size: number): MemoCache { +const MEMO_CACHE_UNSET_SENTINEL: symbol = Symbol.for( + 'react.usememocache_sentinel', +); +function useMemoCache(size: number): Array { let memoCache = null; // Fast-path, load memo cache from wip fiber if already prepared let updateQueue: FunctionComponentUpdateQueue | null = (currentlyRenderingFiber.updateQueue: any); @@ -853,8 +854,7 @@ function useMemoCache(size: number): MemoCache { if (current !== null) { const currentUpdateQueue: FunctionComponentUpdateQueue | null = (current.updateQueue: any); if (currentUpdateQueue !== null) { - const currentMemoCache: ?MemoCacheStorage = - currentUpdateQueue.memoCache; + const currentMemoCache: ?MemoCache = currentUpdateQueue.memoCache; if (currentMemoCache != null) { memoCache = { data: currentMemoCache.data.map(array => array.slice()), @@ -882,6 +882,9 @@ function useMemoCache(size: number): MemoCache { data = memoCache.data[memoCache.index] = new Array(size).fill( MEMO_CACHE_UNSET_SENTINEL, ); + // Attach sentinel to the array, eventually we can make the sentinel a public + // export from React and avoid modifying the array + (data: any)._ = MEMO_CACHE_UNSET_SENTINEL; } else if (data.length !== size) { // TODO: consider warning or throwing here if (__DEV__) { @@ -894,7 +897,7 @@ function useMemoCache(size: number): MemoCache { } } memoCache.index++; - return createMemoCache(data); + return data; } function basicStateReducer(state: S, action: BasicStateAction): S { @@ -3611,7 +3614,7 @@ if (__DEV__) { if (enableUseMemoCacheHook) { (InvalidNestedHooksDispatcherOnMountInDEV: Dispatcher).useMemoCache = function( size: number, - ): MemoCache { + ): Array { warnInvalidHookAccess(); return useMemoCache(size); }; @@ -3798,7 +3801,7 @@ if (__DEV__) { if (enableUseMemoCacheHook) { (InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher).useMemoCache = function( size: number, - ): MemoCache { + ): Array { warnInvalidHookAccess(); return useMemoCache(size); }; @@ -3986,7 +3989,7 @@ if (__DEV__) { if (enableUseMemoCacheHook) { (InvalidNestedHooksDispatcherOnRerenderInDEV: Dispatcher).useMemoCache = function( size: number, - ): MemoCache { + ): Array { warnInvalidHookAccess(); return useMemoCache(size); }; diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index 5dc55ef01edb6..2ee501d2a433f 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -9,7 +9,6 @@ import type {Source} from 'shared/ReactElementType'; import type { - MemoCache, RefObject, ReactContext, MutableSourceSubscribeFn, @@ -74,7 +73,7 @@ export type Dependencies = { ... }; -export type MemoCacheStorage = { +export type MemoCache = { data: Array>, index: number, }; @@ -429,7 +428,7 @@ export type Dispatcher = { ): T, useId(): string, useCacheRefresh?: () => (?() => T, ?T) => void, - useMemoCache?: (size: number) => MemoCache, + useMemoCache?: (size: number) => Array, unstable_isNewReconciler?: boolean, }; diff --git a/packages/react-reconciler/src/ReactMemoCache.js b/packages/react-reconciler/src/ReactMemoCache.js deleted file mode 100644 index 651c64668993a..0000000000000 --- a/packages/react-reconciler/src/ReactMemoCache.js +++ /dev/null @@ -1,59 +0,0 @@ -/** - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @flow - */ - -import type {MemoCache} from 'shared/ReactTypes'; -import type {Dispatcher} from './ReactInternalTypes'; - -import ReactSharedInternals from 'shared/ReactSharedInternals'; -const {ReactCurrentDispatcher} = ReactSharedInternals; - -export const MEMO_CACHE_UNSET_SENTINEL: symbol = Symbol.for( - 'react.usememocache_sentinel', -); - -const MemoCachePrototype: MemoCache = { - has(index: number): boolean { - // $FlowFixMe[object-this-reference] - return this.data[index] !== MEMO_CACHE_UNSET_SENTINEL; - }, - - get(index: number): any { - // $FlowFixMe[object-this-reference] - return this.data[index]; - }, - - update(index: number, value: any): boolean { - // $FlowFixMe[object-this-reference] - const previous = this.data[index]; - // $FlowFixMe[object-this-reference] - this.data[index] = value; - return previous === MEMO_CACHE_UNSET_SENTINEL || previous !== value; - }, - - withDispatcher_TEMPORARY_DEBUG_API_DO_NOT_USE( - dispatcher: Dispatcher, - fn: () => T, - ): T | void { - const previousDispatcher = ReactCurrentDispatcher.current; - ReactCurrentDispatcher.current = dispatcher; - let value; - try { - value = fn(); - } finally { - ReactCurrentDispatcher.current = previousDispatcher; - } - return value; - }, -}; - -export function createMemoCache(data: Array): MemoCache { - const cache: {data: Array} = (Object.create(MemoCachePrototype): any); - cache.data = data; - return (cache: any); -} diff --git a/packages/react-reconciler/src/__tests__/useMemoCache-test.js b/packages/react-reconciler/src/__tests__/useMemoCache-test.js index 4e749acbb52c5..22edf2cbf5fcd 100644 --- a/packages/react-reconciler/src/__tests__/useMemoCache-test.js +++ b/packages/react-reconciler/src/__tests__/useMemoCache-test.js @@ -44,10 +44,10 @@ describe('useMemoCache()', () => { test('render component using cache', async () => { function Component(props) { const cache = useMemoCache(1); - expect(cache.has(0)).toBe(false); - expect(cache.update(0, undefined)).toBe(true); // changed - expect(cache.has(0)).toBe(true); - expect(cache.update(0, undefined)).toBe(false); // unchanged + expect(Array.isArray(cache)).toBe(true); + expect(cache.length).toBe(1); + expect(cache[0]).toBe(cache._); + expect(typeof cache._).toBe('symbol'); return 'Ok'; } const root = ReactNoop.createRoot(); @@ -67,26 +67,25 @@ describe('useMemoCache()', () => { // x is used to produce a `data` object passed to the child const [x, _setX] = useState(0); setX = _setX; - const c_x = cache.update(0, x); + const c_x = x !== cache[0]; + cache[0] = x; // n is passed as-is to the child as a cache breaker const [n, setN] = useState(0); forceUpdate = () => setN(a => a + 1); - const c_n = cache.update(1, n); + const c_n = n !== cache[1]; + cache[1] = n; let data; if (c_x) { - data = {text: `Count ${x}`}; - cache.update(2, data); + data = cache[2] = {text: `Count ${x}`}; } else { - data = cache.get(2); + data = cache[2]; } if (c_x || c_n) { - const ret = ; - cache.update(3, ret); - return ret; + return (cache[3] = ); } else { - return cache.get(3); + return cache[3]; } } let data; @@ -132,12 +131,14 @@ describe('useMemoCache()', () => { // x is used to produce a `data` object passed to the child const [x, _setX] = useState(0); setX = _setX; - const c_x = cache.update(0, x); + const c_x = x !== cache[0]; + cache[0] = x; // n is passed as-is to the child as a cache breaker const [n, _setN] = useState(0); setN = _setN; - const c_n = cache.update(1, n); + const c_n = n !== cache[1]; + cache[1] = n; // NOTE: setstate and early return here means that x will update // without the data value being updated. Subsequent renders could @@ -153,17 +154,14 @@ describe('useMemoCache()', () => { let data; if (c_x) { - data = {text: `Count ${x}`}; - cache.update(2, data); + data = cache[2] = {text: `Count ${x}`}; } else { - data = cache.get(2); + data = cache[2]; } if (c_x || c_n) { - const ret = ; - cache.update(3, ret); - return ret; + return (cache[3] = ); } else { - return cache.get(3); + return cache[3]; } } let data; @@ -213,12 +211,14 @@ describe('useMemoCache()', () => { // x is used to produce a `data` object passed to the child const [x, _setX] = useState(0); setX = _setX; - const c_x = cache.update(0, x); + const c_x = x !== cache[0]; + cache[0] = x; // n is passed as-is to the child as a cache breaker const [n, _setN] = useState(0); setN = _setN; - const c_n = cache.update(1, n); + const c_n = n !== cache[1]; + cache[1] = n; // NOTE the initial failure will trigger a re-render, after which the function // will early return. This validates that the runtime resets the cache on error: @@ -235,17 +235,14 @@ describe('useMemoCache()', () => { let data; if (c_x) { - data = {text: `Count ${x}`}; - cache.update(2, data); + data = cache[2] = {text: `Count ${x}`}; } else { - data = cache.get(2); + data = cache[2]; } if (c_x || c_n) { - const ret = ; - cache.update(3, ret); - return ret; + return (cache[3] = ); } else { - return cache.get(3); + return cache[3]; } } let data; @@ -302,38 +299,37 @@ describe('useMemoCache()', () => { // x is used to produce a `data` object passed to the child const [x, _setX] = useState(0); setX = _setX; - const c_x = cache.update(0, x); + const c_x = x !== cache[0]; + cache[0] = x; // n is passed as-is to the child as a cache breaker const [n, setN] = useState(0); forceUpdate = () => setN(a => a + 1); - const c_n = cache.update(1, n); + const c_n = n !== cache[1]; + cache[1] = n; let _data; if (c_x) { - _data = {text: `Count ${x}`}; - cache.update(2, _data); + _data = cache[2] = {text: `Count ${x}`}; } else { - _data = cache.get(2); + _data = cache[2]; } const data = useData(_data); if (c_x || c_n) { - const ret = ; - cache.update(3, ret); - return ret; + return (cache[3] = ); } else { - return cache.get(3); + return cache[3]; } } function useData(data) { const cache = useMemoCache(2); - const c_data = cache.update(0, data); + const c_data = data !== cache[0]; + cache[0] = data; let nextData; if (c_data) { - nextData = {text: data.text.toLowerCase()}; - cache.update(1, nextData); + nextData = cache[1] = {text: data.text.toLowerCase()}; } else { - nextData = cache.get(1); + nextData = cache[1]; } return nextData; } @@ -369,30 +365,4 @@ describe('useMemoCache()', () => { expect(Text).toBeCalledTimes(3); expect(data).toBe(data1); // confirm that the cache persisted across renders }); - - // @gate enableUseMemoCacheHook - test('withDispatcher temporary debugging utility', async () => { - const dispatcher = { - useMemoCache: jest.fn(), - }; - function Component() { - const cache = useMemoCache(0); - return cache.withDispatcher_TEMPORARY_DEBUG_API_DO_NOT_USE( - dispatcher, - () => { - // oops, calling a component without jsx - return InnerComponent(); - }, - ); - } - function InnerComponent() { - useMemoCache(); - } - const root = ReactNoop.createRoot(); - expect(dispatcher.useMemoCache).toBeCalledTimes(0); - await act(async () => { - root.render(); - }); - expect(dispatcher.useMemoCache).toBeCalledTimes(1); - }); }); diff --git a/packages/react-server/src/ReactFizzHooks.js b/packages/react-server/src/ReactFizzHooks.js index 965f18acd9e71..9b688fb1058ee 100644 --- a/packages/react-server/src/ReactFizzHooks.js +++ b/packages/react-server/src/ReactFizzHooks.js @@ -13,7 +13,6 @@ import type { } from 'react-reconciler/src/ReactInternalTypes'; import type { - MemoCache, MutableSource, MutableSourceGetSnapshotFn, MutableSourceSubscribeFn, @@ -672,8 +671,13 @@ function useCacheRefresh(): (?() => T, ?T) => void { return unsupportedRefresh; } -function useMemoCache(size: number): MemoCache { - throw new Error('useMemoCache is not yet supported in Fizz'); +const MEMO_CACHE_UNSET_SENTINEL: symbol = Symbol.for( + 'react.usememocache_sentinel', +); +function useMemoCache(size: number): Array { + const data = new Array(size).fill(MEMO_CACHE_UNSET_SENTINEL); + (data: any)._ = MEMO_CACHE_UNSET_SENTINEL; + return data; } function noop(): void {} diff --git a/packages/react-server/src/ReactFlightHooks.js b/packages/react-server/src/ReactFlightHooks.js index dff654ba06ab9..0c56591bd9097 100644 --- a/packages/react-server/src/ReactFlightHooks.js +++ b/packages/react-server/src/ReactFlightHooks.js @@ -64,6 +64,9 @@ function readContext(context: ReactServerContext): T { return readContextImpl(context); } +const MEMO_CACHE_UNSET_SENTINEL: symbol = Symbol.for( + 'react.usememocache_sentinel', +); export const Dispatcher: DispatcherType = { useMemo(nextCreate: () => T): T { return nextCreate(); @@ -103,7 +106,11 @@ export const Dispatcher: DispatcherType = { useCacheRefresh(): (?() => T, ?T) => void { return unsupportedRefresh; }, - useMemoCache: (unsupportedHook: any), + useMemoCache(size: number): Array { + const data = new Array(size).fill(MEMO_CACHE_UNSET_SENTINEL); + (data: any)._ = MEMO_CACHE_UNSET_SENTINEL; + return data; + }, use: enableUseHook ? use : (unsupportedHook: any), }; diff --git a/packages/react/src/ReactHooks.js b/packages/react/src/ReactHooks.js index 7d380e23f3768..271a3bae9e680 100644 --- a/packages/react/src/ReactHooks.js +++ b/packages/react/src/ReactHooks.js @@ -9,7 +9,6 @@ import type {Dispatcher} from 'react-reconciler/src/ReactInternalTypes'; import type { - MemoCache, MutableSource, MutableSourceGetSnapshotFn, MutableSourceSubscribeFn, @@ -211,7 +210,7 @@ export function use(usable: Usable): T { return dispatcher.use(usable); } -export function useMemoCache(size: number): MemoCache { +export function useMemoCache(size: number): Array { const dispatcher = resolveDispatcher(); // $FlowFixMe This is unstable, thus optional return dispatcher.useMemoCache(size); diff --git a/packages/shared/ReactTypes.js b/packages/shared/ReactTypes.js index c903ec5c499cd..48b4ce11c1b6e 100644 --- a/packages/shared/ReactTypes.js +++ b/packages/shared/ReactTypes.js @@ -7,7 +7,7 @@ * @flow */ -import type {Dispatcher, Fiber} from 'react-reconciler/src/ReactInternalTypes'; +import type {Fiber} from 'react-reconciler/src/ReactInternalTypes'; export type ReactNode = | React$Element @@ -217,36 +217,3 @@ export type StartTransitionOptions = { }; export type Usable = Thenable | ReactContext; - -/** - * Describes the result of `useMemoCache()` - */ -export interface MemoCache { - /** - * Returns true if the value at @param index is initialized, - * false if not. - */ - has(index: number): boolean; - - /** - * Returns the value at @param index, regardless of whether it is - * initialized or not. - */ - get(index: number): any; - - /** - * Sets the value at @param index to @param value, and returns true - * if the value was newly initialized or changed, false if the value - * was previously initialized and did not change. - */ - update(index: number, value: any): boolean; - - /** - * Temporary API used during debugging of auto-memoizing compiler to - * guard some sections of code by swapping the dispatcher. - */ - withDispatcher_TEMPORARY_DEBUG_API_DO_NOT_USE( - dispatcher: Dispatcher, - fn: () => T, - ): T | void; -} diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 1205f2ae1a904..db795a516fe49 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -439,8 +439,5 @@ "451": "resolveSingletonInstance was called with an element type that is not supported. This is a bug in React.", "452": "React expected an element (document.documentElement) to exist in the Document but one was not found. React never removes the documentElement for any Document it renders into so the cause is likely in some other script running on this page.", "453": "React expected a element (document.head) to exist in the Document but one was not found. React never removes the head for any Document it renders into so the cause is likely in some other script running on this page.", - "454": "React expected a element (document.body) to exist in the Document but one was not found. React never removes the body for any Document it renders into so the cause is likely in some other script running on this page.", - "455": "Memo cache index of out bounds", - "456": "Memo cache index accessed before being initialized", - "457": "useMemoCache is not yet supported in Fizz" + "454": "React expected a element (document.body) to exist in the Document but one was not found. React never removes the body for any Document it renders into so the cause is likely in some other script running on this page." }