Skip to content

Commit

Permalink
Add flag to test fast jsx (#28816)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jackpope authored May 3, 2024
1 parent 1d618a9 commit 1beb73d
Show file tree
Hide file tree
Showing 10 changed files with 26 additions and 6 deletions.
7 changes: 3 additions & 4 deletions packages/react/src/__tests__/ReactJSXRuntime-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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',
Expand Down
17 changes: 15 additions & 2 deletions packages/react/src/jsx/ReactJSXElement.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
enableRefAsProp,
disableStringRefs,
disableDefaultPropsExceptForClasses,
enableFastJSX,
} from 'shared/ReactFeatureFlags';
import {checkPropStringCoercion} from 'shared/CheckStringCoercion';
import {ClassComponent} from 'react-reconciler/src/ReactWorkTags';
Expand Down Expand Up @@ -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')) {
Expand Down Expand Up @@ -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`,
Expand Down Expand Up @@ -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`,
Expand Down
1 change: 1 addition & 0 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -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__;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www-dynamic.js
Original file line number Diff line number Diff line change
Expand Up @@ -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__;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export const {
disableDefaultPropsExceptForClasses,
enableNoCloningMemoCache,
enableAddPropertiesFastPath,
enableFastJSX,
} = dynamicFeatureFlags;

// On WWW, __EXPERIMENTAL__ is used for a new modern build.
Expand Down

0 comments on commit 1beb73d

Please sign in to comment.