Skip to content

Commit 1cd3b85

Browse files
committed
Move ref type check to receiver
The runtime contains a type check to determine if a user-provided ref is a valid type — a function or object (or a string, when `disableStringRefs` is off). This currently happens during child reconciliation. This changes it to happen only when the ref is passed to the component that the ref is being attached to. This is a continuation of the "ref as prop" change — until you actually pass a ref to a HostComponent, class, etc, ref is a normal prop that has no special behavior.
1 parent c979895 commit 1cd3b85

File tree

5 files changed

+41
-35
lines changed

5 files changed

+41
-35
lines changed

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

+4-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,10 @@ describe('ReactComponent', () => {
4646
act(() => {
4747
root.render(<div ref="badDiv" />);
4848
}),
49-
).rejects.toThrow();
49+
).rejects.toThrow(
50+
'Element ref was specified as a string (badDiv) but no owner ' +
51+
'was set',
52+
);
5053
});
5154

5255
it('should throw (in dev) when children are mutated during render', async () => {

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -401,14 +401,14 @@ describe('ref swapping', () => {
401401
root.render(<div ref={10} />);
402402
});
403403
}).rejects.toThrow(
404-
'Expected ref to be a function, a string, an object returned by React.createRef(), or null.',
404+
'Element ref was specified as a string (10) but no owner was set.',
405405
);
406406
await expect(async () => {
407407
await act(() => {
408408
root.render(<div ref={true} />);
409409
});
410410
}).rejects.toThrow(
411-
'Expected ref to be a function, a string, an object returned by React.createRef(), or null.',
411+
'Element ref was specified as a string (true) but no owner was set.',
412412
);
413413
await expect(async () => {
414414
await act(() => {

packages/react-reconciler/src/ReactChildFiber.js

+10-19
Original file line numberDiff line numberDiff line change
@@ -157,17 +157,17 @@ function convertStringRefToCallbackRef(
157157
returnFiber: Fiber,
158158
current: Fiber | null,
159159
element: ReactElement,
160-
mixedRef: any,
160+
mixedRef: string | number | boolean,
161161
): CoercedStringRef {
162+
if (__DEV__) {
163+
checkPropStringCoercion(mixedRef, 'ref');
164+
}
165+
const stringRef = '' + (mixedRef: any);
166+
162167
const owner: ?Fiber = (element._owner: any);
163168
if (!owner) {
164-
if (typeof mixedRef !== 'string') {
165-
throw new Error(
166-
'Expected ref to be a function, a string, an object returned by React.createRef(), or null.',
167-
);
168-
}
169169
throw new Error(
170-
`Element ref was specified as a string (${mixedRef}) but no owner was set. This could happen for one of` +
170+
`Element ref was specified as a string (${stringRef}) but no owner was set. This could happen for one of` +
171171
' the following reasons:\n' +
172172
'1. You may be adding a ref to a function component\n' +
173173
"2. You may be adding a ref to a component that was not created inside a component's render method\n" +
@@ -184,13 +184,6 @@ function convertStringRefToCallbackRef(
184184
);
185185
}
186186

187-
// At this point, we know the ref isn't an object or function but it could
188-
// be a number. Coerce it to a string.
189-
if (__DEV__) {
190-
checkPropStringCoercion(mixedRef, 'ref');
191-
}
192-
const stringRef = '' + mixedRef;
193-
194187
if (__DEV__) {
195188
if (
196189
// Will already warn with "Function components cannot be given refs"
@@ -267,12 +260,10 @@ function coerceRef(
267260
let coercedRef;
268261
if (
269262
!disableStringRefs &&
270-
mixedRef !== null &&
271-
typeof mixedRef !== 'function' &&
272-
typeof mixedRef !== 'object'
263+
(typeof mixedRef === 'string' ||
264+
typeof mixedRef === 'number' ||
265+
typeof mixedRef === 'boolean')
273266
) {
274-
// Assume this is a string ref. If it's not, then this will throw an error
275-
// to the user.
276267
coercedRef = convertStringRefToCallbackRef(
277268
returnFiber,
278269
current,

packages/react-reconciler/src/ReactFiberBeginWork.js

+17-9
Original file line numberDiff line numberDiff line change
@@ -1026,16 +1026,24 @@ function updateProfiler(
10261026
}
10271027

10281028
function markRef(current: Fiber | null, workInProgress: Fiber) {
1029-
// TODO: This is also where we should check the type of the ref and error if
1030-
// an invalid one is passed, instead of during child reconcilation.
1029+
// TODO: Check props.ref instead of fiber.ref when enableRefAsProp is on.
10311030
const ref = workInProgress.ref;
1032-
if (
1033-
(current === null && ref !== null) ||
1034-
(current !== null && current.ref !== ref)
1035-
) {
1036-
// Schedule a Ref effect
1037-
workInProgress.flags |= Ref;
1038-
workInProgress.flags |= RefStatic;
1031+
if (ref === null) {
1032+
if (current !== null && current.ref !== null) {
1033+
// Schedule a Ref effect
1034+
workInProgress.flags |= Ref | RefStatic;
1035+
}
1036+
} else {
1037+
if (typeof ref !== 'function' && typeof ref !== 'object') {
1038+
// TODO: Remove "string" from error message.
1039+
throw new Error(
1040+
'Expected ref to be a function, a string, an object returned by React.createRef(), or null.',
1041+
);
1042+
}
1043+
if (current === null || current.ref !== ref) {
1044+
// Schedule a Ref effect
1045+
workInProgress.flags |= Ref | RefStatic;
1046+
}
10391047
}
10401048
}
10411049

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

+8-4
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,13 @@ describe('ReactFiberRefs', () => {
115115
});
116116

117117
// @gate disableStringRefs
118-
test('log an error in dev if a string ref is passed to a ref-receiving component', async () => {
118+
test('throw if a string ref is passed to a ref-receiving component', async () => {
119119
let refProp;
120120
function Child({ref}) {
121+
// This component renders successfully because the ref type check does not
122+
// occur until you pass it to a component that accepts refs.
123+
//
124+
// So the div will throw, but not Child.
121125
refProp = ref;
122126
return <div ref={ref} />;
123127
}
@@ -129,9 +133,9 @@ describe('ReactFiberRefs', () => {
129133
}
130134

131135
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');
136+
await expect(act(() => root.render(<Owner />))).rejects.toThrow(
137+
'Expected ref to be a function',
138+
);
135139
expect(refProp).toBe('child');
136140
});
137141
});

0 commit comments

Comments
 (0)