Skip to content

Commit c979895

Browse files
authored
Remove string refs (behind flag) (#28322)
Depends on: - #28398 --- This removes string refs, which has been deprecated in Strict Mode for seven years. I've left them behind a flag for Meta, but in open source this fully removes the feature.
1 parent 3bcd2de commit c979895

23 files changed

+154
-88
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ describe('ReactComponent', () => {
3838
}).toThrowError(/Target container is not a DOM element./);
3939
});
4040

41+
// @gate !disableStringRefs || !__DEV__
4142
it('should throw when supplying a string ref outside of render method', async () => {
4243
const container = document.createElement('div');
4344
const root = ReactDOMClient.createRoot(container);
@@ -125,6 +126,7 @@ describe('ReactComponent', () => {
125126
}
126127
});
127128

129+
// @gate !disableStringRefs
128130
it('should support string refs on owned components', async () => {
129131
const innerObj = {};
130132
const outerObj = {};

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ describe('ReactDOMServerIntegration', () => {
7676
expect(refElement).toBe(e);
7777
});
7878

79+
// @gate !disableStringRefs
7980
it('should have string refs on client when rendered over server markup', async () => {
8081
class RefsComponent extends React.Component {
8182
render() {

packages/react-dom/src/__tests__/ReactDeprecationWarnings-test.js

Lines changed: 35 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ describe('ReactDeprecationWarnings', () => {
6464
);
6565
});
6666

67+
// @gate !disableStringRefs
6768
it('should warn when given string refs', async () => {
6869
class RefComponent extends React.Component {
6970
render() {
@@ -87,6 +88,7 @@ describe('ReactDeprecationWarnings', () => {
8788
);
8889
});
8990

91+
// @gate !disableStringRefs
9092
it('should warn when owner and self are the same for string refs', async () => {
9193
class RefComponent extends React.Component {
9294
render() {
@@ -109,6 +111,7 @@ describe('ReactDeprecationWarnings', () => {
109111
await waitForAll([]);
110112
});
111113

114+
// @gate !disableStringRefs
112115
it('should warn when owner and self are different for string refs', async () => {
113116
class RefComponent extends React.Component {
114117
render() {
@@ -139,39 +142,39 @@ describe('ReactDeprecationWarnings', () => {
139142
]);
140143
});
141144

142-
if (__DEV__) {
143-
it('should warn when owner and self are different for string refs', async () => {
144-
class RefComponent extends React.Component {
145-
render() {
146-
return null;
147-
}
145+
// @gate __DEV__
146+
// @gate !disableStringRefs
147+
it('should warn when owner and self are different for string refs', async () => {
148+
class RefComponent extends React.Component {
149+
render() {
150+
return null;
148151
}
149-
class Component extends React.Component {
150-
render() {
151-
return JSXDEVRuntime.jsxDEV(
152-
RefComponent,
153-
{ref: 'refComponent'},
154-
null,
155-
false,
156-
{},
157-
{},
158-
);
159-
}
152+
}
153+
class Component extends React.Component {
154+
render() {
155+
return JSXDEVRuntime.jsxDEV(
156+
RefComponent,
157+
{ref: 'refComponent'},
158+
null,
159+
false,
160+
{},
161+
{},
162+
);
160163
}
164+
}
161165

162-
ReactNoop.render(<Component />);
163-
await expect(async () => await waitForAll([])).toErrorDev([
164-
'Warning: Component "Component" contains the string ref "refComponent". ' +
165-
'Support for string refs will be removed in a future major release. ' +
166-
'This case cannot be automatically converted to an arrow function. ' +
167-
'We ask you to manually fix this case by using useRef() or createRef() instead. ' +
168-
'Learn more about using refs safely here: ' +
169-
'https://reactjs.org/link/strict-mode-string-ref',
170-
'Warning: Component "Component" contains the string ref "refComponent". ' +
171-
'Support for string refs will be removed in a future major release. We recommend ' +
172-
'using useRef() or createRef() instead. Learn more about using refs safely here: ' +
173-
'https://reactjs.org/link/strict-mode-string-ref',
174-
]);
175-
});
176-
}
166+
ReactNoop.render(<Component />);
167+
await expect(async () => await waitForAll([])).toErrorDev([
168+
'Warning: Component "Component" contains the string ref "refComponent". ' +
169+
'Support for string refs will be removed in a future major release. ' +
170+
'This case cannot be automatically converted to an arrow function. ' +
171+
'We ask you to manually fix this case by using useRef() or createRef() instead. ' +
172+
'Learn more about using refs safely here: ' +
173+
'https://reactjs.org/link/strict-mode-string-ref',
174+
'Warning: Component "Component" contains the string ref "refComponent". ' +
175+
'Support for string refs will be removed in a future major release. We recommend ' +
176+
'using useRef() or createRef() instead. Learn more about using refs safely here: ' +
177+
'https://reactjs.org/link/strict-mode-string-ref',
178+
]);
179+
});
177180
});

packages/react-dom/src/__tests__/ReactFunctionComponent-test.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ describe('ReactFunctionComponent', () => {
173173
).resolves.not.toThrowError();
174174
});
175175

176+
// @gate !disableStringRefs
176177
it('should throw on string refs in pure functions', async () => {
177178
function Child() {
178179
return <div ref="me" />;

packages/react-dom/src/__tests__/multiple-copies-of-react-test.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ class TextWithStringRef extends React.Component {
2222
}
2323

2424
describe('when different React version is used with string ref', () => {
25+
// @gate !disableStringRefs
2526
it('throws the "Refs must have owner" warning', async () => {
2627
const container = document.createElement('div');
2728
const root = ReactDOMClient.createRoot(container);

packages/react-dom/src/__tests__/refs-test.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ describe('reactiverefs', () => {
164164
* Ensure that for every click log there is a corresponding ref (from the
165165
* perspective of the injected ClickCounter component.
166166
*/
167+
// @gate !disableStringRefs
167168
it('Should increase refs with an increase in divs', async () => {
168169
const testRefsComponent = await renderTestRefsComponent();
169170
const clickIncrementer =
@@ -366,6 +367,7 @@ describe('ref swapping', () => {
366367
expect(refCalled).toBe(1);
367368
});
368369

370+
// @gate !disableStringRefs
369371
it('coerces numbers to strings', async () => {
370372
class A extends React.Component {
371373
render() {
@@ -390,6 +392,7 @@ describe('ref swapping', () => {
390392
expect(a.refs[1].nodeName).toBe('DIV');
391393
});
392394

395+
// @gate !disableStringRefs
393396
it('provides an error for invalid refs', async () => {
394397
const container = document.createElement('div');
395398
const root = ReactDOMClient.createRoot(container);
@@ -547,6 +550,7 @@ describe('creating element with string ref in constructor', () => {
547550
}
548551
}
549552

553+
// @gate !disableStringRefs
550554
it('throws an error', async () => {
551555
await expect(async function () {
552556
const container = document.createElement('div');
@@ -567,6 +571,7 @@ describe('creating element with string ref in constructor', () => {
567571
});
568572

569573
describe('strings refs across renderers', () => {
574+
// @gate !disableStringRefs
570575
it('does not break', async () => {
571576
class Parent extends React.Component {
572577
render() {

packages/react-reconciler/src/ReactChildFiber.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ import {
4444
import isArray from 'shared/isArray';
4545
import assign from 'shared/assign';
4646
import {checkPropStringCoercion} from 'shared/CheckStringCoercion';
47-
import {enableRefAsProp} from 'shared/ReactFeatureFlags';
47+
import {enableRefAsProp, disableStringRefs} from 'shared/ReactFeatureFlags';
4848

4949
import {
5050
createWorkInProgress,
@@ -266,6 +266,7 @@ function coerceRef(
266266

267267
let coercedRef;
268268
if (
269+
!disableStringRefs &&
269270
mixedRef !== null &&
270271
typeof mixedRef !== 'function' &&
271272
typeof mixedRef !== 'object'

packages/react-reconciler/src/ReactFiberCommitWork.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import {
5454
enableFloat,
5555
enableLegacyHidden,
5656
alwaysThrottleRetries,
57+
disableStringRefs,
5758
} from 'shared/ReactFeatureFlags';
5859
import {
5960
FunctionComponent,
@@ -1624,7 +1625,11 @@ function commitAttachRef(finishedWork: Fiber) {
16241625
}
16251626
} else {
16261627
if (__DEV__) {
1627-
if (!ref.hasOwnProperty('current')) {
1628+
// TODO: We should move these warnings to happen during the render
1629+
// phase (markRef).
1630+
if (disableStringRefs && typeof ref === 'string') {
1631+
console.error('String refs are no longer supported.');
1632+
} else if (!ref.hasOwnProperty('current')) {
16281633
console.error(
16291634
'Unexpected ref object provided for %s. ' +
16301635
'Use either a ref-setter function or React.createRef().',

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ describe('ReactFiberRefs', () => {
8686
});
8787

8888
// @gate enableRefAsProp
89+
// @gate !disableStringRefs
8990
test('string ref props are converted to function refs', async () => {
9091
let refProp;
9192
function Child({ref}) {
@@ -112,4 +113,25 @@ describe('ReactFiberRefs', () => {
112113
expect(typeof refProp === 'function').toBe(true);
113114
expect(owner.refs.child.type).toBe('div');
114115
});
116+
117+
// @gate disableStringRefs
118+
test('log an error in dev if a string ref is passed to a ref-receiving component', async () => {
119+
let refProp;
120+
function Child({ref}) {
121+
refProp = ref;
122+
return <div ref={ref} />;
123+
}
124+
125+
class Owner extends React.Component {
126+
render() {
127+
return <Child ref="child" />;
128+
}
129+
}
130+
131+
const root = ReactNoop.createRoot();
132+
await expect(async () => {
133+
await expect(act(() => root.render(<Owner />))).rejects.toThrow();
134+
}).toErrorDev('String refs are no longer supported');
135+
expect(refProp).toBe('child');
136+
});
115137
});

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1346,6 +1346,7 @@ describe('ReactIncrementalSideEffects', () => {
13461346
// TODO: Test that mounts, updates, refs, unmounts and deletions happen in the
13471347
// expected way for aborted and resumed render life-cycles.
13481348

1349+
// @gate !disableStringRefs
13491350
it('supports string refs', async () => {
13501351
let fooInstance = null;
13511352

0 commit comments

Comments
 (0)