Skip to content

Commit

Permalink
Make class prop resolution faster (#28766)
Browse files Browse the repository at this point in the history
`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.
  • Loading branch information
acdlite committed Apr 5, 2024
1 parent cbb6f2b commit bfd8da8
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 25 deletions.
31 changes: 19 additions & 12 deletions packages/react-reconciler/src/ReactFiberClassComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -1251,32 +1251,39 @@ 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 &&
// If disableDefaultPropsExceptForClasses is true, we always resolve
// 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);

This comment has been minimized.

Copy link
@yungsters

yungsters Apr 6, 2024

Contributor

Why not just assign({}, newProps)?

This comment has been minimized.

Copy link
@eps1lon

eps1lon Apr 12, 2024

Collaborator

Testing in #28829

}
// Taken from old JSX runtime, where this used to live.
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 newProps) {
if (newProps === baseProps) {
newProps = assign({}, newProps);
}
delete newProps.ref;
}
}

return newProps;
}

Expand Down
38 changes: 25 additions & 13 deletions packages/react-server/src/ReactFizzServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down

0 comments on commit bfd8da8

Please sign in to comment.