From 976a554fb50c92104d1512393b517423577e26de Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 5 Apr 2024 13:06:39 -0400 Subject: [PATCH] Make class prop resolution faster (#28766) `delete` causes an object (in V8, and maybe other engines) to deopt to a dictionary instead of a class. Instead of `assign` + `delete`, manually iterate over all the properties, like the JSX runtime does. To avoid copying the object twice I moved the `ref` prop removal to come before handling default props. If we already cloned the props to remove `ref`, then we can skip cloning again to handle default props. --- .../src/ReactFiberClassComponent.js | 31 +++++++++------ packages/react-server/src/ReactFizzServer.js | 38 ++++++++++++------- 2 files changed, 44 insertions(+), 25 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index e464670bfedb4..f802c261ba418 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -1251,7 +1251,19 @@ export function resolveClassComponentProps( ): Object { let newProps = baseProps; - // Resolve default props. Taken from old JSX runtime, where this used to live. + if (enableRefAsProp) { + // Remove ref from the props object, if it exists. + if ('ref' in baseProps) { + newProps = ({}: any); + for (const propName in baseProps) { + if (propName !== 'ref') { + newProps[propName] = baseProps[propName]; + } + } + } + } + + // Resolve default props. const defaultProps = Component.defaultProps; if ( defaultProps && @@ -1259,7 +1271,12 @@ export function resolveClassComponentProps( // default props here in the reconciler, rather than in the JSX runtime. (disableDefaultPropsExceptForClasses || !alreadyResolvedDefaultProps) ) { - newProps = assign({}, newProps, baseProps); + // We may have already copied the props object above to remove ref. If so, + // we can modify that. Otherwise, copy the props object with Object.assign. + if (newProps === baseProps) { + newProps = assign({}, newProps, baseProps); + } + // Taken from old JSX runtime, where this used to live. for (const propName in defaultProps) { if (newProps[propName] === undefined) { newProps[propName] = defaultProps[propName]; @@ -1267,16 +1284,6 @@ export function resolveClassComponentProps( } } - if (enableRefAsProp) { - // Remove ref from the props object, if it exists. - if ('ref' in newProps) { - if (newProps === baseProps) { - newProps = assign({}, newProps); - } - delete newProps.ref; - } - } - return newProps; } diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 327311d74fa31..a322d57df695f 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -1397,24 +1397,36 @@ export function resolveClassComponentProps( ): Object { let newProps = baseProps; - // Resolve default props. Taken from old JSX runtime, where this used to live. - const defaultProps = Component.defaultProps; - if (defaultProps && disableDefaultPropsExceptForClasses) { - newProps = assign({}, newProps, baseProps); - for (const propName in defaultProps) { - if (newProps[propName] === undefined) { - newProps[propName] = defaultProps[propName]; + if (enableRefAsProp) { + // Remove ref from the props object, if it exists. + if ('ref' in baseProps) { + newProps = ({}: any); + for (const propName in baseProps) { + if (propName !== 'ref') { + newProps[propName] = baseProps[propName]; + } } } } - if (enableRefAsProp) { - // Remove ref from the props object, if it exists. - if ('ref' in newProps) { - if (newProps === baseProps) { - newProps = assign({}, newProps); + // Resolve default props. + const defaultProps = Component.defaultProps; + if ( + defaultProps && + // If disableDefaultPropsExceptForClasses is true, we always resolve + // default props here, rather than in the JSX runtime. + disableDefaultPropsExceptForClasses + ) { + // We may have already copied the props object above to remove ref. If so, + // we can modify that. Otherwise, copy the props object with Object.assign. + if (newProps === baseProps) { + newProps = assign({}, newProps, baseProps); + } + // Taken from old JSX runtime, where this used to live. + for (const propName in defaultProps) { + if (newProps[propName] === undefined) { + newProps[propName] = defaultProps[propName]; } - delete newProps.ref; } }