Skip to content

Commit d285b3a

Browse files
authored
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.
1 parent ed71a3a commit d285b3a

File tree

3 files changed

+38
-11
lines changed

3 files changed

+38
-11
lines changed

packages/react-reconciler/src/ReactFiberClassComponent.js

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,12 @@ import {
1919
} from './ReactFiberFlags';
2020
import {
2121
debugRenderPhaseSideEffectsForStrictMode,
22-
disableDefaultPropsExceptForClasses,
2322
disableLegacyContext,
24-
disableStringRefs,
2523
enableDebugTracing,
24+
enableSchedulingProfiler,
2625
enableLazyContextPropagation,
2726
enableRefAsProp,
28-
enableSchedulingProfiler,
27+
disableDefaultPropsExceptForClasses,
2928
} from 'shared/ReactFeatureFlags';
3029
import ReactStrictModeWarnings from './ReactStrictModeWarnings';
3130
import {isMounted} from './ReactFiberTreeReflection';
@@ -820,12 +819,7 @@ function mountClassInstance(
820819
const instance = workInProgress.stateNode;
821820
instance.props = newProps;
822821
instance.state = workInProgress.memoizedState;
823-
if (!disableStringRefs) {
824-
// When string refs are used in create-react-class legacy components,
825-
// we need to make refs writable unless we patch all such copies of the
826-
// class code that sets to a frozen emptyObject.
827-
instance.refs = {};
828-
}
822+
instance.refs = {};
829823

830824
initializeUpdateQueue(workInProgress);
831825

packages/react-reconciler/src/__tests__/ReactFiberRefs-test.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,4 +162,31 @@ describe('ReactFiberRefs', () => {
162162
await act(() => root.render(<App />));
163163
expect(app.refs.div.prop).toBe('Hello!');
164164
});
165+
166+
test('class refs are initialized to a frozen shared object', async () => {
167+
const refsCollection = new Set();
168+
class Component extends React.Component {
169+
constructor(props) {
170+
super(props);
171+
refsCollection.add(this.refs);
172+
}
173+
render() {
174+
return <div />;
175+
}
176+
}
177+
178+
const root = ReactNoop.createRoot();
179+
await act(() =>
180+
root.render(
181+
<>
182+
<Component />
183+
<Component />
184+
</>,
185+
),
186+
);
187+
188+
expect(refsCollection.size).toBe(1);
189+
const refsInstance = Array.from(refsCollection)[0];
190+
expect(Object.isFrozen(refsInstance)).toBe(__DEV__);
191+
});
165192
});

packages/react/src/ReactBaseClasses.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,19 @@
88
import ReactNoopUpdateQueue from './ReactNoopUpdateQueue';
99
import assign from 'shared/assign';
1010

11+
const emptyObject = {};
12+
if (__DEV__) {
13+
Object.freeze(emptyObject);
14+
}
15+
1116
/**
1217
* Base class helpers for the updating state of a component.
1318
*/
1419
function Component(props, context, updater) {
1520
this.props = props;
1621
this.context = context;
17-
this.refs = {};
22+
// If a component has string refs, we will assign a different object later.
23+
this.refs = emptyObject;
1824
// We initialize the default updater but the real one gets injected by the
1925
// renderer.
2026
this.updater = updater || ReactNoopUpdateQueue;
@@ -127,7 +133,7 @@ function PureComponent(props, context, updater) {
127133
this.props = props;
128134
this.context = context;
129135
// If a component has string refs, we will assign a different object later.
130-
this.refs = {};
136+
this.refs = emptyObject;
131137
this.updater = updater || ReactNoopUpdateQueue;
132138
}
133139

0 commit comments

Comments
 (0)