Skip to content

Commit

Permalink
Convert string ref props to callback props
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 ca
keep using string refs temporarily while they finish migrating
their codebase.
  • Loading branch information
acdlite committed Feb 20, 2024
1 parent 90e6a90 commit 335b94c
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 3 deletions.
32 changes: 29 additions & 3 deletions packages/react-reconciler/src/ReactChildFiber.js
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 @@ -276,13 +277,38 @@ function coerceRef(
element,
mixedRef,
);

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;
}

// TODO: If enableRefAsProp is on, we shouldn't use the `ref` field. We
// should always read the ref from the prop.
workInProgress.ref = coercedRef;
// 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
28 changes: 28 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactFiberRefs-test.js
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 335b94c

Please sign in to comment.