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.

DiffTrain build for [bfd8da8](bfd8da8)
  • Loading branch information
acdlite committed Apr 5, 2024
1 parent 1d8cde0 commit c7d55de
Show file tree
Hide file tree
Showing 23 changed files with 1,715 additions and 1,623 deletions.
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
cbb6f2b5461cdce282c7e47b9c68a0897d393383
bfd8da807c75a2d123627415f9eaf2d36ac3ed6a
40 changes: 23 additions & 17 deletions compiled/facebook-www/ReactART-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ if (__DEV__) {
return self;
}

var ReactVersion = "19.0.0-www-classic-da2c086d";
var ReactVersion = "19.0.0-www-classic-fcdce517";

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down Expand Up @@ -14287,7 +14287,20 @@ if (__DEV__) {
// remove this extra check.
alreadyResolvedDefaultProps
) {
var newProps = baseProps; // Resolve default props. Taken from old JSX runtime, where this used to live.
var newProps = baseProps;

if (enableRefAsProp) {
// Remove ref from the props object, if it exists.
if ("ref" in baseProps) {
newProps = {};

for (var propName in baseProps) {
if (propName !== "ref") {
newProps[propName] = baseProps[propName];
}
}
}
} // Resolve default props.

var defaultProps = Component.defaultProps;

Expand All @@ -14296,26 +14309,19 @@ if (__DEV__) {
// 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 (var propName in defaultProps) {
if (newProps[propName] === undefined) {
newProps[propName] = defaultProps[propName];
for (var _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
40 changes: 23 additions & 17 deletions compiled/facebook-www/ReactART-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ if (__DEV__) {
return self;
}

var ReactVersion = "19.0.0-www-modern-a4bdb396";
var ReactVersion = "19.0.0-www-modern-f3bc72db";

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down Expand Up @@ -13997,7 +13997,20 @@ if (__DEV__) {
// remove this extra check.
alreadyResolvedDefaultProps
) {
var newProps = baseProps; // Resolve default props. Taken from old JSX runtime, where this used to live.
var newProps = baseProps;

if (enableRefAsProp) {
// Remove ref from the props object, if it exists.
if ("ref" in baseProps) {
newProps = {};

for (var propName in baseProps) {
if (propName !== "ref") {
newProps[propName] = baseProps[propName];
}
}
}
} // Resolve default props.

var defaultProps = Component.defaultProps;

Expand All @@ -14006,26 +14019,19 @@ if (__DEV__) {
// 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 (var propName in defaultProps) {
if (newProps[propName] === undefined) {
newProps[propName] = defaultProps[propName];
for (var _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
Loading

0 comments on commit c7d55de

Please sign in to comment.