Skip to content

Commit

Permalink
Fast JSX: Don't clone props object (#28768)
Browse files Browse the repository at this point in the history 
(Unless "key" is spread onto the element.)

Historically, the JSX runtime clones the props object that is passed in.
We've done this for two reasons.

One reason is that there are certain prop names that are reserved by
React, like `key` and (before React 19) `ref`. These are not actual
props and are not observable by the target component; React uses them
internally but removes them from the props object before passing them to
userspace.

The second reason is that the classic JSX runtime, `createElement`, is
both a compiler target _and_ a public API that can be called manually.
Therefore, we can't assume that the props object that is passed into
`createElement` won't be mutated by userspace code after it is passed
in.

However, the new JSX runtime, `jsx`, is not a public API — it's solely a
compiler target, and the compiler _will_ always pass a fresh, inline
object. So the only reason to clone the props is if a reserved prop name
is used.

In React 19, `ref` is no longer a reserved prop name, and `key` will
only appear in the props object if it is spread onto the element.
(Because if `key` is statically defined, the compiler will pass it as a
separate argument to the `jsx` function.) So the only remaining reason
to clone the props object is if `key` is spread onto the element, which
is a rare case, and also triggers a warning in development.

In a future release, we will not remove a spread key from the props
object. (But we'll still warn.) We'll always pass the object straight
through.

The expected impact is much faster JSX element creation, which in many
apps is a significant slice of the overall runtime cost of rendering.

DiffTrain build for [d1547de](d1547de)
  • Loading branch information
@acdlite
acdlite committed Apr 5, 2024
1 parent c7d55de commit 2cfe8d3
Show file tree
Hide file tree
Showing 11 changed files with 259 additions and 164 deletions.
59 changes: 38 additions & 21 deletions compiled/facebook-www/JSXDEVRuntime-dev.classic.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1352,9 +1352,6 @@ if (__DEV__) {
}
}

var propName; // Reserved names are extracted

var props = {};
var key = null;
var ref = null; // Currently, key can be spread in as a prop. This causes a potential
// issue if key is also explicitly declared (ie. <div {...props} key="Hi" />
Expand Down Expand Up @@ -1391,22 +1388,42 @@ if (__DEV__) {
{
warnIfStringRefCannotBeAutoConverted(config, self);
}
} // Remaining properties are added to a new props object
}

for (propName in config) {
if (
hasOwnProperty.call(config, propName) && // Skip over reserved prop names
propName !== "key" &&
(enableRefAsProp || propName !== "ref")
) {
if (enableRefAsProp && !disableStringRefs && propName === "ref") {
props.ref = coerceStringRef(
config[propName],
ReactCurrentOwner.current,
type
);
} else {
props[propName] = config[propName];
var props;

if (enableRefAsProp && disableStringRefs && !("key" in config)) {
// If key was not spread in, we can reuse the original props object. This
// only works for `jsx`, not `createElement`, because `jsx` is a compiler
// target and the compiler always passes a new object. For `createElement`,
// we can't assume a new object is passed every time because it can be
// called manually.
//
// Spreading key is a warning in dev. In a future release, we will not
// remove a spread key from the props object. (But we'll still warn.) We'll
// always pass the object straight through.
props = config;
} else {
// We need to remove reserved props (key, prop, ref). Create a fresh props
// object and copy over all the non-reserved props. We don't use `delete`
// because in V8 it will deopt the object to dictionary mode.
props = {};

for (var propName in config) {
if (
hasOwnProperty.call(config, propName) && // Skip over reserved prop names
propName !== "key" &&
(enableRefAsProp || propName !== "ref")
) {
if (enableRefAsProp && !disableStringRefs && propName === "ref") {
props.ref = coerceStringRef(
config[propName],
ReactCurrentOwner.current,
type
);
} else {
props[propName] = config[propName];
}
}
}
}
Expand All @@ -1416,9 +1433,9 @@ if (__DEV__) {
if (type && type.defaultProps) {
var defaultProps = type.defaultProps;

for (propName in defaultProps) {
if (props[propName] === undefined) {
props[propName] = defaultProps[propName];
for (var _propName2 in defaultProps) {
if (props[_propName2] === undefined) {
props[_propName2] = defaultProps[_propName2];
}
}
}
Expand Down
59 changes: 38 additions & 21 deletions compiled/facebook-www/JSXDEVRuntime-dev.modern.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1354,9 +1354,6 @@ if (__DEV__) {
}
}

var propName; // Reserved names are extracted

var props = {};
var key = null;
var ref = null; // Currently, key can be spread in as a prop. This causes a potential
// issue if key is also explicitly declared (ie. <div {...props} key="Hi" />
Expand Down Expand Up @@ -1393,22 +1390,42 @@ if (__DEV__) {
{
warnIfStringRefCannotBeAutoConverted(config, self);
}
} // Remaining properties are added to a new props object
}

for (propName in config) {
if (
hasOwnProperty.call(config, propName) && // Skip over reserved prop names
propName !== "key" &&
(enableRefAsProp || propName !== "ref")
) {
if (enableRefAsProp && !disableStringRefs && propName === "ref") {
props.ref = coerceStringRef(
config[propName],
ReactCurrentOwner.current,
type
);
} else {
props[propName] = config[propName];
var props;

if (enableRefAsProp && disableStringRefs && !("key" in config)) {
// If key was not spread in, we can reuse the original props object. This
// only works for `jsx`, not `createElement`, because `jsx` is a compiler
// target and the compiler always passes a new object. For `createElement`,
// we can't assume a new object is passed every time because it can be
// called manually.
//
// Spreading key is a warning in dev. In a future release, we will not
// remove a spread key from the props object. (But we'll still warn.) We'll
// always pass the object straight through.
props = config;
} else {
// We need to remove reserved props (key, prop, ref). Create a fresh props
// object and copy over all the non-reserved props. We don't use `delete`
// because in V8 it will deopt the object to dictionary mode.
props = {};

for (var propName in config) {
if (
hasOwnProperty.call(config, propName) && // Skip over reserved prop names
propName !== "key" &&
(enableRefAsProp || propName !== "ref")
) {
if (enableRefAsProp && !disableStringRefs && propName === "ref") {
props.ref = coerceStringRef(
config[propName],
ReactCurrentOwner.current,
type
);
} else {
props[propName] = config[propName];
}
}
}
}
Expand All @@ -1418,9 +1435,9 @@ if (__DEV__) {
if (type && type.defaultProps) {
var defaultProps = type.defaultProps;

for (propName in defaultProps) {
if (props[propName] === undefined) {
props[propName] = defaultProps[propName];
for (var _propName2 in defaultProps) {
if (props[_propName2] === undefined) {
props[_propName2] = defaultProps[_propName2];
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
bfd8da807c75a2d123627415f9eaf2d36ac3ed6a
d1547defe34cee6326a61059148afc83228d8ecf
61 changes: 39 additions & 22 deletions compiled/facebook-www/React-dev.classic.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ if (__DEV__) {
) {
__REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStart(new Error());
}
var ReactVersion = "19.0.0-www-classic-ff21e352";
var ReactVersion = "19.0.0-www-classic-4511ca3e";

// ATTENTION
// When adding new symbols to this file,
Expand Down Expand Up @@ -1760,9 +1760,6 @@ if (__DEV__) {
}
}

var propName; // Reserved names are extracted

var props = {};
var key = null;
var ref = null; // Currently, key can be spread in as a prop. This causes a potential
// issue if key is also explicitly declared (ie. <div {...props} key="Hi" />
Expand Down Expand Up @@ -1799,22 +1796,42 @@ if (__DEV__) {
{
warnIfStringRefCannotBeAutoConverted(config, self);
}
} // Remaining properties are added to a new props object
}

for (propName in config) {
if (
hasOwnProperty.call(config, propName) && // Skip over reserved prop names
propName !== "key" &&
(enableRefAsProp || propName !== "ref")
) {
if (enableRefAsProp && !disableStringRefs && propName === "ref") {
props.ref = coerceStringRef(
config[propName],
ReactCurrentOwner.current,
type
);
} else {
props[propName] = config[propName];
var props;

if (enableRefAsProp && disableStringRefs && !("key" in config)) {
// If key was not spread in, we can reuse the original props object. This
// only works for `jsx`, not `createElement`, because `jsx` is a compiler
// target and the compiler always passes a new object. For `createElement`,
// we can't assume a new object is passed every time because it can be
// called manually.
//
// Spreading key is a warning in dev. In a future release, we will not
// remove a spread key from the props object. (But we'll still warn.) We'll
// always pass the object straight through.
props = config;
} else {
// We need to remove reserved props (key, prop, ref). Create a fresh props
// object and copy over all the non-reserved props. We don't use `delete`
// because in V8 it will deopt the object to dictionary mode.
props = {};

for (var propName in config) {
if (
hasOwnProperty.call(config, propName) && // Skip over reserved prop names
propName !== "key" &&
(enableRefAsProp || propName !== "ref")
) {
if (enableRefAsProp && !disableStringRefs && propName === "ref") {
props.ref = coerceStringRef(
config[propName],
ReactCurrentOwner.current,
type
);
} else {
props[propName] = config[propName];
}
}
}
}
Expand All @@ -1824,9 +1841,9 @@ if (__DEV__) {
if (type && type.defaultProps) {
var defaultProps = type.defaultProps;

for (propName in defaultProps) {
if (props[propName] === undefined) {
props[propName] = defaultProps[propName];
for (var _propName2 in defaultProps) {
if (props[_propName2] === undefined) {
props[_propName2] = defaultProps[_propName2];
}
}
}
Expand Down
61 changes: 39 additions & 22 deletions compiled/facebook-www/React-dev.modern.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ if (__DEV__) {
) {
__REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStart(new Error());
}
var ReactVersion = "19.0.0-www-modern-99738e97";
var ReactVersion = "19.0.0-www-modern-8e3891bc";

// ATTENTION
// When adding new symbols to this file,
Expand Down Expand Up @@ -1762,9 +1762,6 @@ if (__DEV__) {
}
}

var propName; // Reserved names are extracted

var props = {};
var key = null;
var ref = null; // Currently, key can be spread in as a prop. This causes a potential
// issue if key is also explicitly declared (ie. <div {...props} key="Hi" />
Expand Down Expand Up @@ -1801,22 +1798,42 @@ if (__DEV__) {
{
warnIfStringRefCannotBeAutoConverted(config, self);
}
} // Remaining properties are added to a new props object
}

for (propName in config) {
if (
hasOwnProperty.call(config, propName) && // Skip over reserved prop names
propName !== "key" &&
(enableRefAsProp || propName !== "ref")
) {
if (enableRefAsProp && !disableStringRefs && propName === "ref") {
props.ref = coerceStringRef(
config[propName],
ReactCurrentOwner.current,
type
);
} else {
props[propName] = config[propName];
var props;

if (enableRefAsProp && disableStringRefs && !("key" in config)) {
// If key was not spread in, we can reuse the original props object. This
// only works for `jsx`, not `createElement`, because `jsx` is a compiler
// target and the compiler always passes a new object. For `createElement`,
// we can't assume a new object is passed every time because it can be
// called manually.
//
// Spreading key is a warning in dev. In a future release, we will not
// remove a spread key from the props object. (But we'll still warn.) We'll
// always pass the object straight through.
props = config;
} else {
// We need to remove reserved props (key, prop, ref). Create a fresh props
// object and copy over all the non-reserved props. We don't use `delete`
// because in V8 it will deopt the object to dictionary mode.
props = {};

for (var propName in config) {
if (
hasOwnProperty.call(config, propName) && // Skip over reserved prop names
propName !== "key" &&
(enableRefAsProp || propName !== "ref")
) {
if (enableRefAsProp && !disableStringRefs && propName === "ref") {
props.ref = coerceStringRef(
config[propName],
ReactCurrentOwner.current,
type
);
} else {
props[propName] = config[propName];
}
}
}
}
Expand All @@ -1826,9 +1843,9 @@ if (__DEV__) {
if (type && type.defaultProps) {
var defaultProps = type.defaultProps;

for (propName in defaultProps) {
if (props[propName] === undefined) {
props[propName] = defaultProps[propName];
for (var _propName2 in defaultProps) {
if (props[_propName2] === undefined) {
props[_propName2] = defaultProps[_propName2];
}
}
}
Expand Down
Loading

0 comments on commit 2cfe8d3

Please sign in to comment.