Skip to content

Commit

Permalink
Remove _owner field from JSX elements in prod if string refs are disa…
Browse files Browse the repository at this point in the history
…bled (#28739)

In prod, the `_owner` field is only used for string refs so if we have
string refs disabled, we don't need this field. In fact, that's one of
the big benefits of deprecating them.
  • Loading branch information
sebmarkbage authored Apr 4, 2024
1 parent 48b4ecc commit fd0da3e
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 6 deletions.
10 changes: 9 additions & 1 deletion packages/jest-react/src/JestReact.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import {REACT_ELEMENT_TYPE, REACT_FRAGMENT_TYPE} from 'shared/ReactSymbols';
import {enableRefAsProp} from 'shared/ReactFeatureFlags';
import {disableStringRefs, enableRefAsProp} from 'shared/ReactFeatureFlags';

import isArray from 'shared/isArray';

Expand Down Expand Up @@ -54,6 +54,14 @@ function createJSXElementForTestComparison(type, props) {
value: null,
});
return element;
} else if (!__DEV__ && disableStringRefs) {
return {
$$typeof: REACT_ELEMENT_TYPE,
type: type,
key: null,
ref: null,
props: props,
};
} else {
return {
$$typeof: REACT_ELEMENT_TYPE,
Expand Down
11 changes: 11 additions & 0 deletions packages/react-client/src/ReactFlightClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import type {Postpone} from 'react/src/ReactPostpone';
import type {TemporaryReferenceSet} from './ReactFlightTemporaryReferences';

import {
disableStringRefs,
enableBinaryFlight,
enablePostpone,
enableRefAsProp,
Expand Down Expand Up @@ -498,6 +499,16 @@ function createElement(
enumerable: false,
get: nullRefGetter,
});
} else if (!__DEV__ && disableStringRefs) {
element = ({
// This tag allows us to uniquely identify this as a React Element
$$typeof: REACT_ELEMENT_TYPE,

type,
key,
ref: null,
props,
}: any);
} else {
element = ({
// This tag allows us to uniquely identify this as a React Element
Expand Down
14 changes: 13 additions & 1 deletion packages/react-noop-renderer/src/createReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@ import {
ConcurrentRoot,
LegacyRoot,
} from 'react-reconciler/constants';
import {enableRefAsProp, disableLegacyMode} from 'shared/ReactFeatureFlags';
import {
enableRefAsProp,
disableLegacyMode,
disableStringRefs,
} from 'shared/ReactFeatureFlags';

type Container = {
rootID: string,
Expand Down Expand Up @@ -799,6 +803,14 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
value: null,
});
return element;
} else if (!__DEV__ && disableStringRefs) {
return {
$$typeof: REACT_ELEMENT_TYPE,
type: type,
key: null,
ref: null,
props: props,
};
} else {
return {
$$typeof: REACT_ELEMENT_TYPE,
Expand Down
6 changes: 5 additions & 1 deletion packages/react/src/__tests__/ReactCreateElement-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,11 @@ describe('ReactCreateElement', () => {
}
const root = ReactDOMClient.createRoot(document.createElement('div'));
await act(() => root.render(React.createElement(Wrapper)));
expect(element._owner.stateNode).toBe(instance);
if (__DEV__ || !gate(flags => flags.disableStringRefs)) {
expect(element._owner.stateNode).toBe(instance);
} else {
expect('_owner' in element).toBe(false);
}
});

it('merges an additional argument onto the children prop', () => {
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 @@ -239,6 +239,19 @@ function ReactElement(type, key, _ref, self, source, owner, props) {
value: null,
});
}
} else if (!__DEV__ && disableStringRefs) {
// In prod, `ref` is a regular property and _owner doesn't exist.
element = {
// This tag allows us to uniquely identify this as a React Element
$$typeof: REACT_ELEMENT_TYPE,

// Built-in properties that belong on the element
type,
key,
ref,

props,
};
} else {
// In prod, `ref` is a regular property. It will be removed in a
// future release.
Expand Down Expand Up @@ -774,7 +787,7 @@ export function cloneAndReplaceKey(oldElement, newKey) {
enableRefAsProp ? null : oldElement.ref,
undefined,
undefined,
oldElement._owner,
!__DEV__ && disableStringRefs ? undefined : oldElement._owner,
oldElement.props,
);
}
Expand All @@ -800,7 +813,7 @@ export function cloneElement(element, config, children) {
let ref = enableRefAsProp ? null : element.ref;

// Owner will be preserved, unless ref is overridden
let owner = element._owner;
let owner = !__DEV__ && disableStringRefs ? undefined : element._owner;

if (config != null) {
if (hasValidRef(config)) {
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/ReactElementType.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export type ReactElement = {
key: any,
ref: any,
props: any,
// ReactFiber
// __DEV__ or for string refs
_owner: any,

// __DEV__
Expand Down

0 comments on commit fd0da3e

Please sign in to comment.