Skip to content

Commit 7f5d25e

Browse files
Fix cloneElement using string ref w no owner (#28797)
Fix for an issue introduced in #28473 where cloneElement() with a string ref fails due to lack of an owner. We should use the current owner in this case. --------- Co-authored-by: Rick Hanlon <rickhanlonii@fb.com>
1 parent bf40b02 commit 7f5d25e

File tree

2 files changed

+60
-3
lines changed

2 files changed

+60
-3
lines changed

Diff for: packages/react/src/__tests__/ReactElementClone-test.js

+59-2
Original file line numberDiff line numberDiff line change
@@ -274,8 +274,56 @@ describe('ReactElementClone', () => {
274274

275275
const root = ReactDOMClient.createRoot(document.createElement('div'));
276276
await act(() => root.render(<Grandparent />));
277-
expect(component.childRef).toEqual({current: null});
278-
expect(component.parentRef.current.xyzRef.current.tagName).toBe('SPAN');
277+
if (gate(flags => flags.enableRefAsProp && flags.disableStringRefs)) {
278+
expect(component.childRef).toEqual({current: null});
279+
expect(component.parentRef.current.xyzRef.current.tagName).toBe('SPAN');
280+
} else if (
281+
gate(flags => !flags.enableRefAsProp && !flags.disableStringRefs)
282+
) {
283+
expect(component.childRef).toEqual({current: null});
284+
expect(component.parentRef.current.xyzRef.current.tagName).toBe('SPAN');
285+
} else if (
286+
gate(flags => flags.enableRefAsProp && !flags.disableStringRefs)
287+
) {
288+
expect(component.childRef).toEqual({current: null});
289+
expect(component.parentRef.current.xyzRef.current.tagName).toBe('SPAN');
290+
} else {
291+
// Not going to bother testing every possible combination.
292+
}
293+
});
294+
295+
// @gate !disableStringRefs
296+
it('should steal the ref if a new string ref is specified without an owner', async () => {
297+
// Regression test for this specific feature combination calling cloneElement on an element
298+
// without an owner
299+
await expect(async () => {
300+
// create an element without an owner
301+
const element = React.createElement('div', {id: 'some-id'});
302+
class Parent extends React.Component {
303+
render() {
304+
return <Child>{element}</Child>;
305+
}
306+
}
307+
let child;
308+
class Child extends React.Component {
309+
render() {
310+
child = this;
311+
const clone = React.cloneElement(this.props.children, {
312+
ref: 'xyz',
313+
});
314+
return <div>{clone}</div>;
315+
}
316+
}
317+
318+
const root = ReactDOMClient.createRoot(document.createElement('div'));
319+
await act(() => root.render(<Parent />));
320+
expect(child.refs.xyz.tagName).toBe('DIV');
321+
}).toErrorDev([
322+
'Warning: Component "Child" contains the string ref "xyz". Support for ' +
323+
'string refs will be removed in a future major release. We recommend ' +
324+
'using useRef() or createRef() instead. Learn more about using refs ' +
325+
'safely here: https://react.dev/link/strict-mode-string-ref',
326+
]);
279327
});
280328

281329
it('should overwrite props', async () => {
@@ -371,6 +419,15 @@ describe('ReactElementClone', () => {
371419
) {
372420
expect(clone.ref).toBe(element.ref);
373421
expect(clone.props).toEqual({foo: 'ef'});
422+
} else if (
423+
gate(flags => flags.enableRefAsProp && !flags.disableStringRefs)
424+
) {
425+
expect(() => {
426+
expect(clone.ref).toBe(element.ref);
427+
}).toErrorDev('Accessing element.ref was removed in React 19', {
428+
withoutStack: true,
429+
});
430+
expect(clone.props).toEqual({foo: 'ef', ref: element.ref});
374431
} else {
375432
// Not going to bother testing every possible combination.
376433
}

Diff for: packages/react/src/jsx/ReactJSXElement.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -866,14 +866,14 @@ export function cloneElement(element, config, children) {
866866

867867
if (config != null) {
868868
if (hasValidRef(config)) {
869+
owner = ReactSharedInternals.owner;
869870
if (!enableRefAsProp) {
870871
// Silently steal the ref from the parent.
871872
ref = config.ref;
872873
if (!disableStringRefs) {
873874
ref = coerceStringRef(ref, owner, element.type);
874875
}
875876
}
876-
owner = ReactSharedInternals.owner;
877877
}
878878
if (hasValidKey(config)) {
879879
if (__DEV__) {

0 commit comments

Comments
 (0)