Skip to content

Commit 03e3d6f

Browse files
committed
Fix FragmentInstance#compareDocumentPosition for root level fragments in Fabric
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.
1 parent 6eda534 commit 03e3d6f

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;
@@ -691,11 +696,11 @@ FragmentInstance.prototype.compareDocumentPosition = function (
691696
if (parentHostFiber === null) {
692697
return Node.DOCUMENT_POSITION_DISCONNECTED;
693698
}
694-
const parentHostInstance = getPublicInstanceFromHostFiber(parentHostFiber);
695699
const children: Array<Fiber> = [];
696700
traverseFragmentInstance(this._fragmentFiber, collectChildren, children);
697701
if (children.length === 0) {
698-
return compareDocumentPositionForEmptyFragment(
702+
const parentHostInstance = getPublicInstanceFromHostFiber(parentHostFiber);
703+
return compareDocumentPositionForEmptyFragment<PublicInstance>(
699704
this._fragmentFiber,
700705
parentHostInstance,
701706
otherNode,

0 commit comments

Comments
 (0)