Skip to content

Commit

Permalink
Go back to shared refs instance object
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 committed Apr 25, 2024
1 parent cf5ab8b commit 3727294
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 3727294

Please sign in to comment.