Skip to content

Commit e137890

Browse files
kassenskassens
authored
[string-refs] cleanup string ref code (#31443)
1 parent a88b9e5 commit e137890

36 files changed

Lines changed: 35 additions & 1160 deletions

‎packages/jest-react/src/JestReact.js‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
*/
77

88
import {REACT_ELEMENT_TYPE, REACT_FRAGMENT_TYPE} from 'shared/ReactSymbols';
9-
import {disableStringRefs} from 'shared/ReactFeatureFlags';
109
const {assertConsoleLogsCleared} = require('internal-test-utils/consoleMock');
1110

1211
import isArray from 'shared/isArray';
@@ -56,23 +55,13 @@ function createJSXElementForTestComparison(type, props) {
5655
value: null,
5756
});
5857
return element;
59-
} else if (!__DEV__ && disableStringRefs) {
60-
return {
61-
$$typeof: REACT_ELEMENT_TYPE,
62-
type: type,
63-
key: null,
64-
ref: null,
65-
props: props,
66-
};
6758
} else {
6859
return {
6960
$$typeof: REACT_ELEMENT_TYPE,
7061
type: type,
7162
key: null,
7263
ref: null,
7364
props: props,
74-
_owner: null,
75-
_store: __DEV__ ? {} : undefined,
7665
};
7766
}
7867
}

‎packages/react-client/src/ReactFlightClient.js‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ import type {Postpone} from 'react/src/ReactPostpone';
4141
import type {TemporaryReferenceSet} from './ReactFlightTemporaryReferences';
4242

4343
import {
44-
disableStringRefs,
4544
enableBinaryFlight,
4645
enablePostpone,
4746
enableFlightReadableStream,
@@ -688,16 +687,6 @@ function createElement(
688687
enumerable: false,
689688
get: nullRefGetter,
690689
});
691-
} else if (!__DEV__ && disableStringRefs) {
692-
element = ({
693-
// This tag allows us to uniquely identify this as a React Element
694-
$$typeof: REACT_ELEMENT_TYPE,
695-
696-
type,
697-
key,
698-
ref: null,
699-
props,
700-
}: any);
701690
} else {
702691
element = ({
703692
// This tag allows us to uniquely identify this as a React Element
@@ -707,9 +696,6 @@ function createElement(
707696
key,
708697
ref: null,
709698
props,
710-
711-
// Record the component responsible for creating this element.
712-
_owner: __DEV__ && owner === null ? response._debugRootOwner : owner,
713699
}: any);
714700
}
715701

‎packages/react-client/src/__tests__/ReactFlight-test.js‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3268,9 +3268,7 @@ describe('ReactFlight', () => {
32683268
expect(greeting._owner).toBe(greeting._debugInfo[0]);
32693269
} else {
32703270
expect(greeting._debugInfo).toBe(undefined);
3271-
expect(greeting._owner).toBe(
3272-
gate(flags => flags.disableStringRefs) ? undefined : null,
3273-
);
3271+
expect(greeting._owner).toBe(undefined);
32743272
}
32753273
ReactNoop.render(greeting);
32763274
});

‎packages/react-dom/src/__tests__/ReactComponent-test.js‎

Lines changed: 0 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -42,19 +42,6 @@ describe('ReactComponent', () => {
4242
}).toThrowError(/Target container is not a DOM element./);
4343
});
4444

45-
// @gate !disableStringRefs
46-
it('should throw when supplying a string ref outside of render method', async () => {
47-
const container = document.createElement('div');
48-
const root = ReactDOMClient.createRoot(container);
49-
await expect(
50-
act(() => {
51-
root.render(<div ref="badDiv" />);
52-
}),
53-
// TODO: This throws an AggregateError. Need to update test infra to
54-
// support matching against AggregateError.
55-
).rejects.toThrow();
56-
});
57-
5845
it('should throw (in dev) when children are mutated during render', async () => {
5946
function Wrapper(props) {
6047
props.children[1] = <p key={1} />; // Mutation is illegal
@@ -132,105 +119,6 @@ describe('ReactComponent', () => {
132119
}
133120
});
134121

135-
// @gate !disableStringRefs
136-
it('string refs do not detach and reattach on every render', async () => {
137-
let refVal;
138-
class Child extends React.Component {
139-
componentDidUpdate() {
140-
// The parent ref should still be attached because it hasn't changed
141-
// since the last render. If the ref had changed, then this would be
142-
// undefined because refs are attached during the same phase (layout)
143-
// as componentDidUpdate, in child -> parent order. So the new parent
144-
// ref wouldn't have attached yet.
145-
refVal = this.props.contextRef();
146-
}
147-
148-
render() {
149-
if (this.props.show) {
150-
return <div>child</div>;
151-
}
152-
}
153-
}
154-
155-
class Parent extends React.Component {
156-
render() {
157-
return (
158-
<div id="test-root" ref="root">
159-
<Child
160-
contextRef={() => this.refs.root}
161-
show={this.props.showChild}
162-
/>
163-
</div>
164-
);
165-
}
166-
}
167-
168-
const container = document.createElement('div');
169-
const root = ReactDOMClient.createRoot(container);
170-
171-
await act(() => {
172-
root.render(<Parent />);
173-
});
174-
175-
assertConsoleErrorDev(['contains the string ref']);
176-
177-
expect(refVal).toBe(undefined);
178-
await act(() => {
179-
root.render(<Parent showChild={true} />);
180-
});
181-
expect(refVal).toBe(container.querySelector('#test-root'));
182-
});
183-
184-
// @gate !disableStringRefs
185-
it('should support string refs on owned components', async () => {
186-
const innerObj = {};
187-
const outerObj = {};
188-
189-
class Wrapper extends React.Component {
190-
getObject = () => {
191-
return this.props.object;
192-
};
193-
194-
render() {
195-
return <div>{this.props.children}</div>;
196-
}
197-
}
198-
199-
class Component extends React.Component {
200-
render() {
201-
const inner = <Wrapper object={innerObj} ref="inner" />;
202-
const outer = (
203-
<Wrapper object={outerObj} ref="outer">
204-
{inner}
205-
</Wrapper>
206-
);
207-
return outer;
208-
}
209-
210-
componentDidMount() {
211-
expect(this.refs.inner.getObject()).toEqual(innerObj);
212-
expect(this.refs.outer.getObject()).toEqual(outerObj);
213-
}
214-
}
215-
216-
const container = document.createElement('div');
217-
const root = ReactDOMClient.createRoot(container);
218-
await expect(async () => {
219-
await act(() => {
220-
root.render(<Component />);
221-
});
222-
}).toErrorDev([
223-
'Component "Component" contains the string ref "inner". ' +
224-
'Support for string refs will be removed in a future major release. ' +
225-
'We recommend using useRef() or createRef() instead. ' +
226-
'Learn more about using refs safely here: https://react.dev/link/strict-mode-string-ref\n' +
227-
' in Wrapper (at **)\n' +
228-
' in div (at **)\n' +
229-
' in Wrapper (at **)\n' +
230-
' in Component (at **)',
231-
]);
232-
});
233-
234122
it('should not have string refs on unmounted components', async () => {
235123
class Parent extends React.Component {
236124
render() {

‎packages/react-dom/src/__tests__/ReactCompositeComponent-test.js‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -537,11 +537,8 @@ describe('ReactCompositeComponent', () => {
537537
});
538538

539539
it('should cleanup even if render() fatals', async () => {
540-
const dispatcherEnabled =
541-
__DEV__ ||
542-
!gate(flags => flags.disableStringRefs) ||
543-
gate(flags => flags.enableCache);
544-
const ownerEnabled = __DEV__ || !gate(flags => flags.disableStringRefs);
540+
const dispatcherEnabled = __DEV__ || gate(flags => flags.enableCache);
541+
const ownerEnabled = __DEV__;
545542

546543
let stashedDispatcher;
547544
class BadComponent extends React.Component {

‎packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js‎

Lines changed: 2 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,8 @@ function initModules() {
2929
};
3030
}
3131

32-
const {
33-
resetModules,
34-
asyncReactDOMRender,
35-
clientRenderOnServerString,
36-
expectMarkupMatch,
37-
} = ReactDOMServerIntegrationUtils(initModules);
32+
const {resetModules, clientRenderOnServerString, expectMarkupMatch} =
33+
ReactDOMServerIntegrationUtils(initModules);
3834

3935
describe('ReactDOMServerIntegration', () => {
4036
beforeEach(() => {
@@ -75,36 +71,6 @@ describe('ReactDOMServerIntegration', () => {
7571
expect(refElement).not.toBe(null);
7672
expect(refElement).toBe(e);
7773
});
78-
79-
// @gate !disableStringRefs
80-
it('should have string refs on client when rendered over server markup', async () => {
81-
class RefsComponent extends React.Component {
82-
render() {
83-
return <div ref="myDiv" />;
84-
}
85-
}
86-
87-
const markup = ReactDOMServer.renderToString(<RefsComponent />);
88-
const root = document.createElement('div');
89-
root.innerHTML = markup;
90-
let component = null;
91-
resetModules();
92-
await expect(async () => {
93-
await asyncReactDOMRender(
94-
<RefsComponent ref={e => (component = e)} />,
95-
root,
96-
true,
97-
);
98-
}).toErrorDev([
99-
'Component "RefsComponent" contains the string ref "myDiv". ' +
100-
'Support for string refs will be removed in a future major release. ' +
101-
'We recommend using useRef() or createRef() instead. ' +
102-
'Learn more about using refs safely here: https://react.dev/link/strict-mode-string-ref\n' +
103-
' in div (at **)\n' +
104-
' in RefsComponent (at **)',
105-
]);
106-
expect(component.refs.myDiv).toBe(root.firstChild);
107-
});
10874
});
10975

11076
it('should forward refs', async () => {

0 commit comments

Comments
 (0)