From 1beb73de0f7c3261a0de37620453b102caaa6236 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Fri, 3 May 2024 10:47:13 -0400 Subject: [PATCH] Add flag to test fast jsx (#28816) Following #28768, add a path to testing Fast JSX on www. We want to measure the impact of Fast JSX and enable a path to testing before string refs are completely removed in www (which is a work in progress). Without `disableStringRefs`, we need to copy any object with a `ref` key so we can pass it through `coerceStringRef()` and copy it into the object. This de-opt path is what is gated behind `enableFastJSXWithStringRefs`. The additional checks should have no perf impact in OSS as the flags remain true there and the build output is not changed. For www, I've benchmarked the addition of the boolean checks with values cached at module scope. There is no significant change observed from our benchmarks and any latency will apply to test and control branches evenly. This added experiment complexity is temporary. We should be able to clean it up, along with the flag checks for `enableRefAsProp` and `disableStringRefs` shortly. --- .../react/src/__tests__/ReactJSXRuntime-test.js | 7 +++---- packages/react/src/jsx/ReactJSXElement.js | 17 +++++++++++++++-- packages/shared/ReactFeatureFlags.js | 1 + .../shared/forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + ...ReactFeatureFlags.test-renderer.native-fb.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../forks/ReactFeatureFlags.www-dynamic.js | 1 + packages/shared/forks/ReactFeatureFlags.www.js | 1 + 10 files changed, 26 insertions(+), 6 deletions(-) diff --git a/packages/react/src/__tests__/ReactJSXRuntime-test.js b/packages/react/src/__tests__/ReactJSXRuntime-test.js index 0ed8391f36..973b2d65af 100644 --- a/packages/react/src/__tests__/ReactJSXRuntime-test.js +++ b/packages/react/src/__tests__/ReactJSXRuntime-test.js @@ -375,9 +375,8 @@ describe('ReactJSXRuntime', () => { expect(didCall).toBe(false); }); - // @gate enableRefAsProp - // @gate disableStringRefs - it('does not clone props object if key is not spread', async () => { + // @gate enableFastJSX && enableRefAsProp + it('does not clone props object if key and ref is not spread', async () => { const config = { foo: 'foo', bar: 'bar', @@ -386,7 +385,7 @@ describe('ReactJSXRuntime', () => { const element = __DEV__ ? JSXDEVRuntime.jsxDEV('div', config) : JSXRuntime.jsx('div', config); - expect(element.props).toBe(config); + expect(Object.is(element.props, config)).toBe(true); const configWithKey = { foo: 'foo', diff --git a/packages/react/src/jsx/ReactJSXElement.js b/packages/react/src/jsx/ReactJSXElement.js index d8452a2e36..fad4d42bcc 100644 --- a/packages/react/src/jsx/ReactJSXElement.js +++ b/packages/react/src/jsx/ReactJSXElement.js @@ -22,6 +22,7 @@ import { enableRefAsProp, disableStringRefs, disableDefaultPropsExceptForClasses, + enableFastJSX, } from 'shared/ReactFeatureFlags'; import {checkPropStringCoercion} from 'shared/CheckStringCoercion'; import {ClassComponent} from 'react-reconciler/src/ReactWorkTags'; @@ -51,6 +52,10 @@ if (__DEV__) { didWarnAboutElementRef = {}; } +const enableFastJSXWithStringRefs = enableFastJSX && enableRefAsProp; +const enableFastJSXWithoutStringRefs = + enableFastJSXWithStringRefs && disableStringRefs; + function hasValidRef(config) { if (__DEV__) { if (hasOwnProperty.call(config, 'ref')) { @@ -355,7 +360,11 @@ export function jsxProd(type, config, maybeKey) { } let props; - if (enableRefAsProp && disableStringRefs && !('key' in config)) { + if ( + (enableFastJSXWithoutStringRefs || + (enableFastJSXWithStringRefs && !('ref' in config))) && + !('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`, @@ -578,7 +587,11 @@ export function jsxDEV(type, config, maybeKey, isStaticChildren, source, self) { } let props; - if (enableRefAsProp && disableStringRefs && !('key' in config)) { + if ( + (enableFastJSXWithoutStringRefs || + (enableFastJSXWithStringRefs && !('ref' in config))) && + !('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`, diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index ff611728ec..6273b7aedd 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -185,6 +185,7 @@ export const enableInfiniteRenderLoopDetection = true; // during element creation. export const enableRefAsProp = true; export const disableStringRefs = true; +export const enableFastJSX = true; // Warn on any usage of ReactTestRenderer export const enableReactTestRendererWarning = true; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index dc6c8ffd85..ef96c1c216 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -98,6 +98,7 @@ export const enableServerComponentLogs = true; // because JSX is an extremely hot path. export const enableRefAsProp = false; export const disableStringRefs = false; +export const enableFastJSX = false; export const enableReactTestRendererWarning = false; export const disableLegacyMode = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 7d2ef89024..f3cc342017 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -20,6 +20,7 @@ import typeof * as ExportsType from './ReactFeatureFlags.native-oss'; const __TODO_NEXT_RN_MAJOR__ = false; export const enableRefAsProp = __TODO_NEXT_RN_MAJOR__; export const disableStringRefs = __TODO_NEXT_RN_MAJOR__; +export const enableFastJSX = __TODO_NEXT_RN_MAJOR__; export const disableLegacyMode = __TODO_NEXT_RN_MAJOR__; export const disableDOMTestUtils = __TODO_NEXT_RN_MAJOR__; export const useModernStrictMode = __TODO_NEXT_RN_MAJOR__; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 8e362743b3..6a6a9c75ec 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -91,6 +91,7 @@ export const renameElementSymbol = true; // const __NEXT_MAJOR__ = __EXPERIMENTAL__; export const enableRefAsProp = true; export const disableStringRefs = true; +export const enableFastJSX = true; export const disableLegacyMode = true; export const disableLegacyContext = true; export const disableDOMTestUtils = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index 7c142782ff..4193ddc64f 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -82,6 +82,7 @@ export const enableServerComponentLogs = true; export const enableRefAsProp = false; export const disableStringRefs = false; +export const enableFastJSX = false; export const enableReactTestRendererWarning = false; export const disableLegacyMode = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 43f3d9553b..9e87b71193 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -82,6 +82,7 @@ export const enableInfiniteRenderLoopDetection = false; export const enableRefAsProp = false; export const disableStringRefs = false; +export const enableFastJSX = false; export const enableReactTestRendererWarning = false; export const disableLegacyMode = false; diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index d0b39d5abf..a7dbeac15d 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -24,6 +24,7 @@ export const enableDO_NOT_USE_disableStrictPassiveEffect = __VARIANT__; export const enableUseDeferredValueInitialArg = __VARIANT__; export const enableRenderableContext = __VARIANT__; export const enableRefAsProp = __VARIANT__; +export const enableFastJSX = __VARIANT__; export const enableRetryLaneExpiration = __VARIANT__; export const favorSafetyOverHydrationPerf = __VARIANT__; export const disableDefaultPropsExceptForClasses = __VARIANT__; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index dd899e5830..ef562feeba 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -36,6 +36,7 @@ export const { disableDefaultPropsExceptForClasses, enableNoCloningMemoCache, enableAddPropertiesFastPath, + enableFastJSX, } = dynamicFeatureFlags; // On WWW, __EXPERIMENTAL__ is used for a new modern build.