From d285b3acbade77f9b17e6171dbda69ff4a033878 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 25 Apr 2024 13:03:21 -0400 Subject: [PATCH] Go back to shared refs instance object (#28911) It turns out we already made refs writable in #25696, which has been in canary for over a year. The approach in that PR also has the benefit of being slightly more perf sensitive because it still uses a shared object until the fiber is mounted. So let's just go back to that. --- .../src/ReactFiberClassComponent.js | 12 +++------ .../src/__tests__/ReactFiberRefs-test.js | 27 +++++++++++++++++++ packages/react/src/ReactBaseClasses.js | 10 +++++-- 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index 037749f836d76..6bf04da4b41b6 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -19,13 +19,12 @@ import { } from './ReactFiberFlags'; import { debugRenderPhaseSideEffectsForStrictMode, - disableDefaultPropsExceptForClasses, disableLegacyContext, - disableStringRefs, enableDebugTracing, + enableSchedulingProfiler, enableLazyContextPropagation, enableRefAsProp, - enableSchedulingProfiler, + disableDefaultPropsExceptForClasses, } from 'shared/ReactFeatureFlags'; import ReactStrictModeWarnings from './ReactStrictModeWarnings'; import {isMounted} from './ReactFiberTreeReflection'; @@ -820,12 +819,7 @@ function mountClassInstance( const instance = workInProgress.stateNode; instance.props = newProps; instance.state = workInProgress.memoizedState; - if (!disableStringRefs) { - // When string refs are used in create-react-class legacy components, - // we need to make refs writable unless we patch all such copies of the - // class code that sets to a frozen emptyObject. - instance.refs = {}; - } + instance.refs = {}; initializeUpdateQueue(workInProgress); diff --git a/packages/react-reconciler/src/__tests__/ReactFiberRefs-test.js b/packages/react-reconciler/src/__tests__/ReactFiberRefs-test.js index 175c849d94710..8b46dc600ab05 100644 --- a/packages/react-reconciler/src/__tests__/ReactFiberRefs-test.js +++ b/packages/react-reconciler/src/__tests__/ReactFiberRefs-test.js @@ -162,4 +162,31 @@ describe('ReactFiberRefs', () => { await act(() => root.render()); expect(app.refs.div.prop).toBe('Hello!'); }); + + test('class refs are initialized to a frozen shared object', async () => { + const refsCollection = new Set(); + class Component extends React.Component { + constructor(props) { + super(props); + refsCollection.add(this.refs); + } + render() { + return
; + } + } + + const root = ReactNoop.createRoot(); + await act(() => + root.render( + <> + + + , + ), + ); + + expect(refsCollection.size).toBe(1); + const refsInstance = Array.from(refsCollection)[0]; + expect(Object.isFrozen(refsInstance)).toBe(__DEV__); + }); }); diff --git a/packages/react/src/ReactBaseClasses.js b/packages/react/src/ReactBaseClasses.js index ce81071937574..7895a97e3a1ef 100644 --- a/packages/react/src/ReactBaseClasses.js +++ b/packages/react/src/ReactBaseClasses.js @@ -8,13 +8,19 @@ import ReactNoopUpdateQueue from './ReactNoopUpdateQueue'; import assign from 'shared/assign'; +const emptyObject = {}; +if (__DEV__) { + Object.freeze(emptyObject); +} + /** * Base class helpers for the updating state of a component. */ function Component(props, context, updater) { this.props = props; this.context = context; - this.refs = {}; + // If a component has string refs, we will assign a different object later. + this.refs = emptyObject; // We initialize the default updater but the real one gets injected by the // renderer. this.updater = updater || ReactNoopUpdateQueue; @@ -127,7 +133,7 @@ function PureComponent(props, context, updater) { this.props = props; this.context = context; // If a component has string refs, we will assign a different object later. - this.refs = {}; + this.refs = emptyObject; this.updater = updater || ReactNoopUpdateQueue; }