Skip to content

Commit

Permalink
Convert string ref props to callback props (facebook#28398)
Browse files Browse the repository at this point in the history 
When enableRefAsProp is on, we should always use the props as the source
of truth for refs. Not a field on the fiber.

In the case of string refs, this presents a problem, because string refs
are not passed around internally as strings; they are converted to
callback refs. The ref used by the reconciler is not the same as the one
the user provided.

But since this is a deprecated feature anyway, what we can do is clone
the props object and replace it with the internal callback ref. Then we
can continue to use the props object as the source of truth.

This means the internal callback ref will leak into userspace. The
receiving component will receive a callback ref even though the parent
passed a string. Which is weird, but again, this is a deprecated
feature, and we're only leaving it around behind a flag so that Meta can
keep using string refs temporarily while they finish migrating their
codebase.
  • Loading branch information
@acdlite @AndyPengc12
acdlite authored and AndyPengc12 committed Apr 15, 2024
1 parent 7a654b7 commit 3802ef6
Show file tree
Hide file tree
Showing 2 changed files with 169 additions and 103 deletions.
244 changes: 141 additions & 103 deletions packages/react-reconciler/src/ReactChildFiber.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
Fragment,
} from './ReactWorkTags';
import isArray from 'shared/isArray';
import assign from 'shared/assign';
import {checkPropStringCoercion} from 'shared/CheckStringCoercion';
import {enableRefAsProp} from 'shared/ReactFeatureFlags';

Expand Down Expand Up @@ -149,128 +150,165 @@ function unwrapThenable<T>(thenable: Thenable<T>): T {
return trackUsedThenable(thenableState, thenable, index);
}

type CoercedStringRef = ((handle: mixed) => void) & {_stringRef: ?string, ...};

function convertStringRefToCallbackRef(
returnFiber: Fiber,
current: Fiber | null,
element: ReactElement,
mixedRef: any,
): CoercedStringRef {
const owner: ?Fiber = (element._owner: any);
if (!owner) {
if (typeof mixedRef !== 'string') {
throw new Error(
'Expected ref to be a function, a string, an object returned by React.createRef(), or null.',
);
}
throw new Error(
`Element ref was specified as a string (${mixedRef}) but no owner was set. This could happen for one of` +
' the following reasons:\n' +
'1. You may be adding a ref to a function component\n' +
"2. You may be adding a ref to a component that was not created inside a component's render method\n" +
'3. You have multiple copies of React loaded\n' +
'See https://reactjs.org/link/refs-must-have-owner for more information.',
);
}
if (owner.tag !== ClassComponent) {
throw new Error(
'Function components cannot have string refs. ' +
'We recommend using useRef() instead. ' +
'Learn more about using refs safely here: ' +
'https://reactjs.org/link/strict-mode-string-ref',
);
}

// At this point, we know the ref isn't an object or function but it could
// be a number. Coerce it to a string.
if (__DEV__) {
checkPropStringCoercion(mixedRef, 'ref');
}
const stringRef = '' + mixedRef;

if (__DEV__) {
if (
// Will already warn with "Function components cannot be given refs"
!(typeof element.type === 'function' && !isReactClass(element.type))
) {
const componentName =
getComponentNameFromFiber(returnFiber) || 'Component';
if (!didWarnAboutStringRefs[componentName]) {
console.error(
'Component "%s" contains the string ref "%s". Support for string refs ' +
'will be removed in a future major release. We recommend using ' +
'useRef() or createRef() instead. ' +
'Learn more about using refs safely here: ' +
'https://reactjs.org/link/strict-mode-string-ref',
componentName,
stringRef,
);
didWarnAboutStringRefs[componentName] = true;
}
}
}

const inst = owner.stateNode;
if (!inst) {
throw new Error(
`Missing owner for string ref ${stringRef}. This error is likely caused by a ` +
'bug in React. Please file an issue.',
);
}

// Check if previous string ref matches new string ref
if (
current !== null &&
current.ref !== null &&
typeof current.ref === 'function' &&
current.ref._stringRef === stringRef
) {
// Reuse the existing string ref
const currentRef: CoercedStringRef = ((current.ref: any): CoercedStringRef);
return currentRef;
}

// Create a new string ref
const ref = function (value: mixed) {
const refs = inst.refs;
if (value === null) {
delete refs[stringRef];
} else {
refs[stringRef] = value;
}
};
ref._stringRef = stringRef;
return ref;
}

function coerceRef(
returnFiber: Fiber,
current: Fiber | null,
workInProgress: Fiber,
element: ReactElement,
) {
): void {
let mixedRef;
if (enableRefAsProp) {
// TODO: This is a temporary, intermediate step. When enableRefAsProp is on,
// we should resolve the `ref` prop during the begin phase of the component
// it's attached to (HostComponent, ClassComponent, etc).

const refProp = element.props.ref;
mixedRef = refProp !== undefined ? refProp : null;
} else {
// Old behavior.
mixedRef = element.ref;
}

let coercedRef;
if (
mixedRef !== null &&
typeof mixedRef !== 'function' &&
typeof mixedRef !== 'object'
) {
if (__DEV__) {
if (
// Will already throw with "Function components cannot have string refs"
!(
element._owner &&
((element._owner: any): Fiber).tag !== ClassComponent
) &&
// Will already warn with "Function components cannot be given refs"
!(typeof element.type === 'function' && !isReactClass(element.type)) &&
// Will already throw with "Element ref was specified as a string (someStringRef) but no owner was set"
element._owner
) {
const componentName =
getComponentNameFromFiber(returnFiber) || 'Component';
if (!didWarnAboutStringRefs[componentName]) {
console.error(
'Component "%s" contains the string ref "%s". Support for string refs ' +
'will be removed in a future major release. We recommend using ' +
'useRef() or createRef() instead. ' +
'Learn more about using refs safely here: ' +
'https://reactjs.org/link/strict-mode-string-ref',
componentName,
mixedRef,
);
didWarnAboutStringRefs[componentName] = true;
}
}
}

if (element._owner) {
const owner: ?Fiber = (element._owner: any);
let inst;
if (owner) {
const ownerFiber = ((owner: any): Fiber);

if (ownerFiber.tag !== ClassComponent) {
throw new Error(
'Function components cannot have string refs. ' +
'We recommend using useRef() instead. ' +
'Learn more about using refs safely here: ' +
'https://reactjs.org/link/strict-mode-string-ref',
);
}

inst = ownerFiber.stateNode;
}

if (!inst) {
throw new Error(
`Missing owner for string ref ${mixedRef}. This error is likely caused by a ` +
'bug in React. Please file an issue.',
);
}
// Assigning this to a const so Flow knows it won't change in the closure
const resolvedInst = inst;

if (__DEV__) {
checkPropStringCoercion(mixedRef, 'ref');
}
const stringRef = '' + mixedRef;
// Check if previous string ref matches new string ref
if (
current !== null &&
current.ref !== null &&
typeof current.ref === 'function' &&
current.ref._stringRef === stringRef
) {
return current.ref;
}
const ref = function (value: mixed) {
const refs = resolvedInst.refs;
if (value === null) {
delete refs[stringRef];
} else {
refs[stringRef] = value;
}
};
ref._stringRef = stringRef;
return ref;
} else {
if (typeof mixedRef !== 'string') {
throw new Error(
'Expected ref to be a function, a string, an object returned by React.createRef(), or null.',
);
}
// Assume this is a string ref. If it's not, then this will throw an error
// to the user.
coercedRef = convertStringRefToCallbackRef(
returnFiber,
current,
element,
mixedRef,
);

if (!element._owner) {
throw new Error(
`Element ref was specified as a string (${mixedRef}) but no owner was set. This could happen for one of` +
' the following reasons:\n' +
'1. You may be adding a ref to a function component\n' +
"2. You may be adding a ref to a component that was not created inside a component's render method\n" +
'3. You have multiple copies of React loaded\n' +
'See https://reactjs.org/link/refs-must-have-owner for more information.',
);
}
if (enableRefAsProp) {
// When enableRefAsProp is on, we should always use the props as the
// source of truth for refs. Not a field on the fiber.
//
// In the case of string refs, this presents a problem, because string
// refs are not passed around internally as strings; they are converted to
// callback refs. The ref used by the reconciler is not the same as the
// one the user provided.
//
// But since this is a deprecated feature anyway, what we can do is clone
// the props object and replace it with the internal callback ref. Then we
// can continue to use the props object as the source of truth.
//
// This means the internal callback ref will leak into userspace. The
// receiving component will receive a callback ref even though the parent
// passed a string. Which is weird, but again, this is a deprecated
// feature, and we're only leaving it around behind a flag so that Meta
// can keep using string refs temporarily while they finish migrating
// their codebase.
const userProvidedProps = workInProgress.pendingProps;
const propsWithInternalCallbackRef = assign({}, userProvidedProps);
propsWithInternalCallbackRef.ref = coercedRef;
workInProgress.pendingProps = propsWithInternalCallbackRef;
}
} else {
coercedRef = mixedRef;
}
return mixedRef;

// TODO: If enableRefAsProp is on, we shouldn't use the `ref` field. We
// should always read the ref from the prop.
workInProgress.ref = coercedRef;
}

function throwOnInvalidObjectType(returnFiber: Fiber, newChild: Object) {
Expand Down Expand Up @@ -537,7 +575,7 @@ function createChildReconciler(
) {
// Move based on index
const existing = useFiber(current, element.props);
existing.ref = coerceRef(returnFiber, current, element);
coerceRef(returnFiber, current, existing, element);
existing.return = returnFiber;
if (__DEV__) {
existing._debugOwner = element._owner;
Expand All @@ -548,7 +586,7 @@ function createChildReconciler(
}
// Insert
const created = createFiberFromElement(element, returnFiber.mode, lanes);
created.ref = coerceRef(returnFiber, current, element);
coerceRef(returnFiber, current, created, element);
created.return = returnFiber;
if (__DEV__) {
created._debugInfo = debugInfo;
Expand Down Expand Up @@ -652,7 +690,7 @@ function createChildReconciler(
returnFiber.mode,
lanes,
);
created.ref = coerceRef(returnFiber, null, newChild);
coerceRef(returnFiber, null, created, newChild);
created.return = returnFiber;
if (__DEV__) {
created._debugInfo = mergeDebugInfo(debugInfo, newChild._debugInfo);
Expand Down Expand Up @@ -1481,7 +1519,7 @@ function createChildReconciler(
) {
deleteRemainingChildren(returnFiber, child.sibling);
const existing = useFiber(child, element.props);
existing.ref = coerceRef(returnFiber, child, element);
coerceRef(returnFiber, child, existing, element);
existing.return = returnFiber;
if (__DEV__) {
existing._debugOwner = element._owner;
Expand Down Expand Up @@ -1513,7 +1551,7 @@ function createChildReconciler(
return created;
} else {
const created = createFiberFromElement(element, returnFiber.mode, lanes);
created.ref = coerceRef(returnFiber, currentFirstChild, element);
coerceRef(returnFiber, currentFirstChild, created, element);
created.return = returnFiber;
if (__DEV__) {
created._debugInfo = debugInfo;
Expand Down
28 changes: 28 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactFiberRefs-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,32 @@ describe('ReactFiberRefs', () => {
expect(ref1.current).toBe(null);
expect(ref2.current).not.toBe(null);
});

// @gate enableRefAsProp
test('string ref props are converted to function refs', async () => {
let refProp;
function Child({ref}) {
refProp = ref;
return <div ref={ref} />;
}

let owner;
class Owner extends React.Component {
render() {
owner = this;
return <Child ref="child" />;
}
}

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

// When string refs aren't disabled, and enableRefAsProp is on, string refs
// the receiving component receives a callback ref, not the original string.
// This behavior should never be shipped to open source; it's only here to
// allow Meta to keep using string refs temporarily while they finish
// migrating their codebase.
expect(typeof refProp === 'function').toBe(true);
expect(owner.refs.child.type).toBe('div');
});
});

0 comments on commit 3802ef6

Please sign in to comment.