Skip to content

Commit

Permalink
Go back to shared refs instance object (#28911)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
acdlite authored Apr 25, 2024
1 parent ed71a3a commit d285b3a
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 11 deletions.
12 changes: 3 additions & 9 deletions packages/react-reconciler/src/ReactFiberClassComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);

Expand Down
27 changes: 27 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactFiberRefs-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,4 +162,31 @@ describe('ReactFiberRefs', () => {
await act(() => root.render(<App />));
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 <div />;
}
}

const root = ReactNoop.createRoot();
await act(() =>
root.render(
<>
<Component />
<Component />
</>,
),
);

expect(refsCollection.size).toBe(1);
const refsInstance = Array.from(refsCollection)[0];
expect(Object.isFrozen(refsInstance)).toBe(__DEV__);
});
});
10 changes: 8 additions & 2 deletions packages/react/src/ReactBaseClasses.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down

0 comments on commit d285b3a

Please sign in to comment.