Skip to content

Commit 83c88ad

Browse files
authored
Handle fabric root level fragment with compareDocumentPosition (#34533)
The root instance doesn't have a canonical property so we were not returning a public instance that we can call compareDocumentPosition on when a Fragment had no other host parent in Fabric. In this case we need to get the ReactNativeElement from the ReactNativeDocument. I've also added test coverage for this case in DOM for consistency, though it was already working there because we use DOM elements as root. This same test will be copied to RN using Fantom.
1 parent cad813a commit 83c88ad

File tree

2 files changed

+121
-6
lines changed

2 files changed

+121
-6
lines changed

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

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1613,6 +1613,116 @@ describe('FragmentRefs', () => {
16131613
);
16141614
});
16151615

1616+
// @gate enableFragmentRefs
1617+
it('compares a root-level Fragment', async () => {
1618+
const fragmentRef = React.createRef();
1619+
const emptyFragmentRef = React.createRef();
1620+
const childRef = React.createRef();
1621+
const siblingPrecedingRef = React.createRef();
1622+
const siblingFollowingRef = React.createRef();
1623+
const root = ReactDOMClient.createRoot(container);
1624+
1625+
function Test() {
1626+
return (
1627+
<Fragment>
1628+
<div ref={siblingPrecedingRef} />
1629+
<Fragment ref={fragmentRef}>
1630+
<div ref={childRef} />
1631+
</Fragment>
1632+
<Fragment ref={emptyFragmentRef} />
1633+
<div ref={siblingFollowingRef} />
1634+
</Fragment>
1635+
);
1636+
}
1637+
1638+
await act(() => root.render(<Test />));
1639+
1640+
const fragmentInstance = fragmentRef.current;
1641+
if (fragmentInstance == null) {
1642+
throw new Error('Expected fragment instance to be non-null');
1643+
}
1644+
const emptyFragmentInstance = emptyFragmentRef.current;
1645+
if (emptyFragmentInstance == null) {
1646+
throw new Error('Expected empty fragment instance to be non-null');
1647+
}
1648+
1649+
expectPosition(
1650+
fragmentInstance.compareDocumentPosition(childRef.current),
1651+
{
1652+
preceding: false,
1653+
following: false,
1654+
contains: false,
1655+
containedBy: true,
1656+
disconnected: false,
1657+
implementationSpecific: false,
1658+
},
1659+
);
1660+
1661+
expectPosition(
1662+
fragmentInstance.compareDocumentPosition(siblingPrecedingRef.current),
1663+
{
1664+
preceding: true,
1665+
following: false,
1666+
contains: false,
1667+
containedBy: false,
1668+
disconnected: false,
1669+
implementationSpecific: false,
1670+
},
1671+
);
1672+
1673+
expectPosition(
1674+
fragmentInstance.compareDocumentPosition(siblingFollowingRef.current),
1675+
{
1676+
preceding: false,
1677+
following: true,
1678+
contains: false,
1679+
containedBy: false,
1680+
disconnected: false,
1681+
implementationSpecific: false,
1682+
},
1683+
);
1684+
1685+
expectPosition(
1686+
emptyFragmentInstance.compareDocumentPosition(childRef.current),
1687+
{
1688+
preceding: true,
1689+
following: false,
1690+
contains: false,
1691+
containedBy: false,
1692+
disconnected: false,
1693+
implementationSpecific: true,
1694+
},
1695+
);
1696+
1697+
expectPosition(
1698+
emptyFragmentInstance.compareDocumentPosition(
1699+
siblingPrecedingRef.current,
1700+
),
1701+
{
1702+
preceding: true,
1703+
following: false,
1704+
contains: false,
1705+
containedBy: false,
1706+
disconnected: false,
1707+
implementationSpecific: true,
1708+
},
1709+
);
1710+
1711+
expectPosition(
1712+
emptyFragmentInstance.compareDocumentPosition(
1713+
siblingFollowingRef.current,
1714+
),
1715+
{
1716+
preceding: false,
1717+
following: true,
1718+
contains: false,
1719+
containedBy: false,
1720+
disconnected: false,
1721+
implementationSpecific: true,
1722+
},
1723+
);
1724+
});
1725+
16161726
describe('with portals', () => {
16171727
// @gate enableFragmentRefs
16181728
it('handles portaled elements', async () => {

packages/react-native-renderer/src/ReactFiberConfigFabric.js

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import type {Fiber} from 'react-reconciler/src/ReactInternalTypes';
2525
import {HostText} from 'react-reconciler/src/ReactWorkTags';
2626
import {
2727
getFragmentParentHostFiber,
28-
getInstanceFromHostFiber,
2928
traverseFragmentInstance,
3029
} from 'react-reconciler/src/ReactFiberTreeReflection';
3130

@@ -303,6 +302,13 @@ export function getPublicInstance(instance: Instance): null | PublicInstance {
303302
return instance.canonical.publicInstance;
304303
}
305304

305+
// Handle root containers
306+
if (instance.containerInfo != null) {
307+
if (instance.containerInfo.publicInstance != null) {
308+
return instance.containerInfo.publicInstance;
309+
}
310+
}
311+
306312
// For compatibility with the legacy renderer, in case it's used with Fabric
307313
// in the same app.
308314
// $FlowExpectedError[prop-missing]
@@ -347,9 +353,8 @@ export function getPublicInstanceFromInternalInstanceHandle(
347353
}
348354

349355
function getPublicInstanceFromHostFiber(fiber: Fiber): PublicInstance {
350-
const instance = getInstanceFromHostFiber<Instance>(fiber);
351-
const publicInstance = getPublicInstance(instance);
352-
if (publicInstance == null) {
356+
const publicInstance = getPublicInstance(fiber.stateNode);
357+
if (publicInstance === null) {
353358
throw new Error('Expected to find a host node. This is a bug in React.');
354359
}
355360
return publicInstance;
@@ -698,11 +703,11 @@ FragmentInstance.prototype.compareDocumentPosition = function (
698703
if (parentHostFiber === null) {
699704
return Node.DOCUMENT_POSITION_DISCONNECTED;
700705
}
701-
const parentHostInstance = getPublicInstanceFromHostFiber(parentHostFiber);
702706
const children: Array<Fiber> = [];
703707
traverseFragmentInstance(this._fragmentFiber, collectChildren, children);
704708
if (children.length === 0) {
705-
return compareDocumentPositionForEmptyFragment(
709+
const parentHostInstance = getPublicInstanceFromHostFiber(parentHostFiber);
710+
return compareDocumentPositionForEmptyFragment<PublicInstance>(
706711
this._fragmentFiber,
707712
parentHostInstance,
708713
otherNode,

0 commit comments

Comments
 (0)