Skip to content

Commit d1547de

Browse files
acdliteacdlite
authored
Fast JSX: Don't clone props object (#28768)
(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.
1 parent bfd8da8 commit d1547de

File tree

2 files changed

+101
-44
lines changed

2 files changed

+101
-44
lines changed

‎packages/react/src/__tests__/ReactJSXRuntime-test.js

+33
Original file line numberDiff line numberDiff line change
@@ -374,4 +374,37 @@ describe('ReactJSXRuntime', () => {
374374
}
375375
expect(didCall).toBe(false);
376376
});
377+
378+
// @gate enableRefAsProp
379+
// @gate disableStringRefs
380+
it('does not clone props object if key is not spread', async () => {
381+
const config = {
382+
foo: 'foo',
383+
bar: 'bar',
384+
};
385+
386+
const element = __DEV__
387+
? JSXDEVRuntime.jsxDEV('div', config)
388+
: JSXRuntime.jsx('div', config);
389+
expect(element.props).toBe(config);
390+
391+
const configWithKey = {
392+
foo: 'foo',
393+
bar: 'bar',
394+
// This only happens when the key is spread onto the element. A statically
395+
// defined key is passed as a separate argument to the jsx() runtime.
396+
key: 'key',
397+
};
398+
399+
let elementWithSpreadKey;
400+
expect(() => {
401+
elementWithSpreadKey = __DEV__
402+
? JSXDEVRuntime.jsxDEV('div', configWithKey)
403+
: JSXRuntime.jsx('div', configWithKey);
404+
}).toErrorDev(
405+
'A props object containing a "key" prop is being spread into JSX',
406+
{withoutStack: true},
407+
);
408+
expect(elementWithSpreadKey.props).not.toBe(configWithKey);
409+
});
377410
});

‎packages/react/src/jsx/ReactJSXElement.js

+68-44
Original file line numberDiff line numberDiff line change
@@ -314,11 +314,6 @@ function ReactElement(type, key, _ref, self, source, owner, props) {
314314
* @param {string} key
315315
*/
316316
export function jsxProd(type, config, maybeKey) {
317-
let propName;
318-
319-
// Reserved names are extracted
320-
const props = {};
321-
322317
let key = null;
323318
let ref = null;
324319

@@ -351,22 +346,39 @@ export function jsxProd(type, config, maybeKey) {
351346
}
352347
}
353348

354-
// Remaining properties are added to a new props object
355-
for (propName in config) {
356-
if (
357-
hasOwnProperty.call(config, propName) &&
358-
// Skip over reserved prop names
359-
propName !== 'key' &&
360-
(enableRefAsProp || propName !== 'ref')
361-
) {
362-
if (enableRefAsProp && !disableStringRefs && propName === 'ref') {
363-
props.ref = coerceStringRef(
364-
config[propName],
365-
ReactCurrentOwner.current,
366-
type,
367-
);
368-
} else {
369-
props[propName] = config[propName];
349+
let props;
350+
if (enableRefAsProp && disableStringRefs && !('key' in config)) {
351+
// If key was not spread in, we can reuse the original props object. This
352+
// only works for `jsx`, not `createElement`, because `jsx` is a compiler
353+
// target and the compiler always passes a new object. For `createElement`,
354+
// we can't assume a new object is passed every time because it can be
355+
// called manually.
356+
//
357+
// Spreading key is a warning in dev. In a future release, we will not
358+
// remove a spread key from the props object. (But we'll still warn.) We'll
359+
// always pass the object straight through.
360+
props = config;
361+
} else {
362+
// We need to remove reserved props (key, prop, ref). Create a fresh props
363+
// object and copy over all the non-reserved props. We don't use `delete`
364+
// because in V8 it will deopt the object to dictionary mode.
365+
props = {};
366+
for (const propName in config) {
367+
if (
368+
hasOwnProperty.call(config, propName) &&
369+
// Skip over reserved prop names
370+
propName !== 'key' &&
371+
(enableRefAsProp || propName !== 'ref')
372+
) {
373+
if (enableRefAsProp && !disableStringRefs && propName === 'ref') {
374+
props.ref = coerceStringRef(
375+
config[propName],
376+
ReactCurrentOwner.current,
377+
type,
378+
);
379+
} else {
380+
props[propName] = config[propName];
381+
}
370382
}
371383
}
372384
}
@@ -375,7 +387,7 @@ export function jsxProd(type, config, maybeKey) {
375387
// Resolve default props
376388
if (type && type.defaultProps) {
377389
const defaultProps = type.defaultProps;
378-
for (propName in defaultProps) {
390+
for (const propName in defaultProps) {
379391
if (props[propName] === undefined) {
380392
props[propName] = defaultProps[propName];
381393
}
@@ -538,11 +550,6 @@ export function jsxDEV(type, config, maybeKey, isStaticChildren, source, self) {
538550
}
539551
}
540552

541-
let propName;
542-
543-
// Reserved names are extracted
544-
const props = {};
545-
546553
let key = null;
547554
let ref = null;
548555

@@ -578,22 +585,39 @@ export function jsxDEV(type, config, maybeKey, isStaticChildren, source, self) {
578585
}
579586
}
580587

581-
// Remaining properties are added to a new props object
582-
for (propName in config) {
583-
if (
584-
hasOwnProperty.call(config, propName) &&
585-
// Skip over reserved prop names
586-
propName !== 'key' &&
587-
(enableRefAsProp || propName !== 'ref')
588-
) {
589-
if (enableRefAsProp && !disableStringRefs && propName === 'ref') {
590-
props.ref = coerceStringRef(
591-
config[propName],
592-
ReactCurrentOwner.current,
593-
type,
594-
);
595-
} else {
596-
props[propName] = config[propName];
588+
let props;
589+
if (enableRefAsProp && disableStringRefs && !('key' in config)) {
590+
// If key was not spread in, we can reuse the original props object. This
591+
// only works for `jsx`, not `createElement`, because `jsx` is a compiler
592+
// target and the compiler always passes a new object. For `createElement`,
593+
// we can't assume a new object is passed every time because it can be
594+
// called manually.
595+
//
596+
// Spreading key is a warning in dev. In a future release, we will not
597+
// remove a spread key from the props object. (But we'll still warn.) We'll
598+
// always pass the object straight through.
599+
props = config;
600+
} else {
601+
// We need to remove reserved props (key, prop, ref). Create a fresh props
602+
// object and copy over all the non-reserved props. We don't use `delete`
603+
// because in V8 it will deopt the object to dictionary mode.
604+
props = {};
605+
for (const propName in config) {
606+
if (
607+
hasOwnProperty.call(config, propName) &&
608+
// Skip over reserved prop names
609+
propName !== 'key' &&
610+
(enableRefAsProp || propName !== 'ref')
611+
) {
612+
if (enableRefAsProp && !disableStringRefs && propName === 'ref') {
613+
props.ref = coerceStringRef(
614+
config[propName],
615+
ReactCurrentOwner.current,
616+
type,
617+
);
618+
} else {
619+
props[propName] = config[propName];
620+
}
597621
}
598622
}
599623
}
@@ -602,7 +626,7 @@ export function jsxDEV(type, config, maybeKey, isStaticChildren, source, self) {
602626
// Resolve default props
603627
if (type && type.defaultProps) {
604628
const defaultProps = type.defaultProps;
605-
for (propName in defaultProps) {
629+
for (const propName in defaultProps) {
606630
if (props[propName] === undefined) {
607631
props[propName] = defaultProps[propName];
608632
}

0 commit comments

Comments
 (0)