Skip to content

Commit 335b94c

Browse files
committed
Convert string ref props to callback props
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 ca keep using string refs temporarily while they finish migrating their codebase.
1 parent 90e6a90 commit 335b94c

File tree

2 files changed

+57
-3
lines changed

2 files changed

+57
-3
lines changed

packages/react-reconciler/src/ReactChildFiber.js

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import {
4141
Fragment,
4242
} from './ReactWorkTags';
4343
import isArray from 'shared/isArray';
44+
import assign from 'shared/assign';
4445
import {checkPropStringCoercion} from 'shared/CheckStringCoercion';
4546
import {enableRefAsProp} from 'shared/ReactFeatureFlags';
4647

@@ -276,13 +277,38 @@ function coerceRef(
276277
element,
277278
mixedRef,
278279
);
280+
281+
if (enableRefAsProp) {
282+
// When enableRefAsProp is on, we should always use the props as the
283+
// source of truth for refs. Not a field on the fiber.
284+
//
285+
// In the case of string refs, this presents a problem, because string
286+
// refs are not passed around internally as strings; they are converted to
287+
// callback refs. The ref used by the reconciler is not the same as the
288+
// one the user provided.
289+
//
290+
// But since this is a deprecated feature anyway, what we can do is clone
291+
// the props object and replace it with the internal callback ref. Then we
292+
// can continue to use the props object as the source of truth.
293+
//
294+
// This means the internal callback ref will leak into userspace. The
295+
// receiving component will receive a callback ref even though the parent
296+
// passed a string. Which is weird, but again, this is a deprecated
297+
// feature, and we're only leaving it around behind a flag so that Meta
298+
// can keep using string refs temporarily while they finish migrating
299+
// their codebase.
300+
const userProvidedProps = workInProgress.pendingProps;
301+
const propsWithInternalCallbackRef = assign({}, userProvidedProps);
302+
propsWithInternalCallbackRef.ref = coercedRef;
303+
workInProgress.pendingProps = propsWithInternalCallbackRef;
304+
}
279305
} else {
280306
coercedRef = mixedRef;
281307
}
282308

283-
// TODO: If enableRefAsProp is on, we shouldn't use the `ref` field. We
284-
// should always read the ref from the prop.
285-
workInProgress.ref = coercedRef;
309+
// TODO: If enableRefAsProp is on, we shouldn't use the `ref` field. We
310+
// should always read the ref from the prop.
311+
workInProgress.ref = coercedRef;
286312
}
287313

288314
function throwOnInvalidObjectType(returnFiber: Fiber, newChild: Object) {

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,4 +84,32 @@ describe('ReactFiberRefs', () => {
8484
expect(ref1.current).toBe(null);
8585
expect(ref2.current).not.toBe(null);
8686
});
87+
88+
// @gate enableRefAsProp
89+
test('string ref props are converted to function refs', async () => {
90+
let refProp;
91+
function Child({ref}) {
92+
refProp = ref;
93+
return <div ref={ref} />;
94+
}
95+
96+
let owner;
97+
class Owner extends React.Component {
98+
render() {
99+
owner = this;
100+
return <Child ref="child" />;
101+
}
102+
}
103+
104+
const root = ReactNoop.createRoot();
105+
await act(() => root.render(<Owner />));
106+
107+
// When string refs aren't disabled, and enableRefAsProp is on, string refs
108+
// the receiving component receives a callback ref, not the original string.
109+
// This behavior should never be shipped to open source; it's only here to
110+
// allow Meta to keep using string refs temporarily while they finish
111+
// migrating their codebase.
112+
expect(typeof refProp === 'function').toBe(true);
113+
expect(owner.refs.child.type).toBe('div');
114+
});
87115
});

0 commit comments

Comments
 (0)