diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 633adc3db68d8..03a6104e68db5 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -38,9 +38,16 @@ import hasOwnProperty from 'shared/hasOwnProperty'; import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion'; import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols'; import { - isFiberContainedBy, + isFiberContainedByFragment, isFiberFollowing, isFiberPreceding, + isFragmentContainedByFiber, + traverseFragmentInstance, + getFragmentParentHostFiber, + getNextSiblingHostFiber, + getInstanceFromHostFiber, + traverseFragmentInstanceDeeply, + fiberIsPortaledIntoHost, } from 'react-reconciler/src/ReactFiberTreeReflection'; export { @@ -63,13 +70,6 @@ import { markNodeAsHoistable, isOwnedInstance, } from './ReactDOMComponentTree'; -import { - traverseFragmentInstance, - getFragmentParentHostFiber, - getNextSiblingHostFiber, - getInstanceFromHostFiber, - traverseFragmentInstanceDeeply, -} from 'react-reconciler/src/ReactFiberTreeReflection'; export {detachDeletedInstance}; import {hasRole} from './DOMAccessibilityRoles'; @@ -3052,13 +3052,13 @@ FragmentInstance.prototype.compareDocumentPosition = function ( } const children: Array = []; traverseFragmentInstance(this._fragmentFiber, collectChildren, children); + const parentHostInstance = + getInstanceFromHostFiber(parentHostFiber); let result = Node.DOCUMENT_POSITION_DISCONNECTED; if (children.length === 0) { // If the fragment has no children, we can use the parent and // siblings to determine a position. - const parentHostInstance = - getInstanceFromHostFiber(parentHostFiber); const parentResult = parentHostInstance.compareDocumentPosition(otherNode); result = parentResult; if (parentHostInstance === otherNode) { @@ -3095,15 +3095,53 @@ FragmentInstance.prototype.compareDocumentPosition = function ( const lastElement = getInstanceFromHostFiber( children[children.length - 1], ); + + // If the fragment has been portaled into another host instance, we need to + // our best guess is to use the parent of the child instance, rather than + // the fiber tree host parent. + const parentHostInstanceFromDOM = fiberIsPortaledIntoHost(this._fragmentFiber) + ? (getInstanceFromHostFiber(children[0]).parentElement: ?Instance) + : parentHostInstance; + + if (parentHostInstanceFromDOM == null) { + return Node.DOCUMENT_POSITION_DISCONNECTED; + } + + // Check if first and last element are actually in the expected document position + // before relying on them as source of truth for other contained elements + const firstElementIsContained = + parentHostInstanceFromDOM.compareDocumentPosition(firstElement) & + Node.DOCUMENT_POSITION_CONTAINED_BY; + const lastElementIsContained = + parentHostInstanceFromDOM.compareDocumentPosition(lastElement) & + Node.DOCUMENT_POSITION_CONTAINED_BY; const firstResult = firstElement.compareDocumentPosition(otherNode); const lastResult = lastElement.compareDocumentPosition(otherNode); + + const otherNodeIsFirstOrLastChild = + (firstElementIsContained && firstElement === otherNode) || + (lastElementIsContained && lastElement === otherNode); + const otherNodeIsFirstOrLastChildDisconnected = + (!firstElementIsContained && firstElement === otherNode) || + (!lastElementIsContained && lastElement === otherNode); + const otherNodeIsWithinFirstOrLastChild = + firstResult & Node.DOCUMENT_POSITION_CONTAINED_BY || + lastResult & Node.DOCUMENT_POSITION_CONTAINED_BY; + const otherNodeIsBetweenFirstAndLastChildren = + firstElementIsContained && + lastElementIsContained && + firstResult & Node.DOCUMENT_POSITION_FOLLOWING && + lastResult & Node.DOCUMENT_POSITION_PRECEDING; + if ( - (firstResult & Node.DOCUMENT_POSITION_FOLLOWING && - lastResult & Node.DOCUMENT_POSITION_PRECEDING) || - otherNode === firstElement || - otherNode === lastElement + otherNodeIsFirstOrLastChild || + otherNodeIsWithinFirstOrLastChild || + otherNodeIsBetweenFirstAndLastChildren ) { result = Node.DOCUMENT_POSITION_CONTAINED_BY; + } else if (otherNodeIsFirstOrLastChildDisconnected) { + // otherNode has been portaled into another container + result = Node.DOCUMENT_POSITION_IMPLEMENTATION_SPECIFIC; } else { result = firstResult; } @@ -3141,7 +3179,9 @@ function validateDocumentPositionWithFiberTree( ): boolean { const otherFiber = getClosestInstanceFromNode(otherNode); if (documentPosition & Node.DOCUMENT_POSITION_CONTAINED_BY) { - return !!otherFiber && isFiberContainedBy(fragmentFiber, otherFiber); + return ( + !!otherFiber && isFiberContainedByFragment(otherFiber, fragmentFiber) + ); } if (documentPosition & Node.DOCUMENT_POSITION_CONTAINS) { if (otherFiber === null) { @@ -3149,7 +3189,7 @@ function validateDocumentPositionWithFiberTree( const ownerDocument = otherNode.ownerDocument; return otherNode === ownerDocument || otherNode === ownerDocument.body; } - return isFiberContainedBy(otherFiber, fragmentFiber); + return isFragmentContainedByFiber(fragmentFiber, otherFiber); } if (documentPosition & Node.DOCUMENT_POSITION_PRECEDING) { return ( diff --git a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js index e7e1d053a7a3e..521792d62e95f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js @@ -1197,14 +1197,14 @@ describe('FragmentRefs', () => { function Test() { return ( -
-
+
+
-
-
-
+
+
+
-
+
); } @@ -1289,7 +1289,7 @@ describe('FragmentRefs', () => { }, ); - // containerRef preceds and contains the fragment + // containerRef precedes and contains the fragment expectPosition( fragmentRef.current.compareDocumentPosition(containerRef.current), { @@ -1328,7 +1328,7 @@ describe('FragmentRefs', () => { function Test() { return (
-
+
@@ -1491,6 +1491,77 @@ describe('FragmentRefs', () => { ); }); + // @gate enableFragmentRefs + it('handles nested children', async () => { + const fragmentRef = React.createRef(); + const nestedFragmentRef = React.createRef(); + const childARef = React.createRef(); + const childBRef = React.createRef(); + const childCRef = React.createRef(); + document.body.appendChild(container); + const root = ReactDOMClient.createRoot(container); + + function Child() { + return ( +
+ C +
+ ); + } + + function Test() { + return ( + +
+ A +
+ +
+ B +
+
+ +
+ ); + } + + await act(() => root.render()); + + expectPosition( + fragmentRef.current.compareDocumentPosition(childARef.current), + { + preceding: false, + following: false, + contains: false, + containedBy: true, + disconnected: false, + implementationSpecific: false, + }, + ); + expectPosition( + fragmentRef.current.compareDocumentPosition(childBRef.current), + { + preceding: false, + following: false, + contains: false, + containedBy: true, + disconnected: false, + implementationSpecific: false, + }, + ); + expectPosition( + fragmentRef.current.compareDocumentPosition(childCRef.current), + { + preceding: false, + following: false, + contains: false, + containedBy: true, + disconnected: false, + implementationSpecific: false, + }, + ); + }); + // @gate enableFragmentRefs it('returns disconnected for comparison with an unmounted fragment instance', async () => { const fragmentRef = React.createRef(); @@ -1551,11 +1622,11 @@ describe('FragmentRefs', () => { function Test() { return ( -
- {createPortal(
, document.body)} +
+ {createPortal(
, container)} - {createPortal(
, document.body)} -
+ {createPortal(
, container)} +
); @@ -1600,6 +1671,8 @@ describe('FragmentRefs', () => { const childARef = React.createRef(); const childBRef = React.createRef(); const childCRef = React.createRef(); + const childDRef = React.createRef(); + const childERef = React.createRef(); function Test() { const [c, setC] = React.useState(false); @@ -1612,23 +1685,30 @@ describe('FragmentRefs', () => { {createPortal(
- {c ?
: null} + {c ? ( +
+
+
+ ) : null} , document.body, )} {createPortal(

, document.body)} +

); } await act(() => root.render()); - // Due to effect, order is A->B->C - expect(document.body.innerHTML).toBe( - '
' + + // Due to effect, order is E / A->B->C->D + expect(document.body.outerHTML).toBe( + '' + + '
' + '
' + '

' + - '
', + '
' + + '', ); expectPosition( @@ -1642,7 +1722,6 @@ describe('FragmentRefs', () => { implementationSpecific: false, }, ); - expectPosition( fragmentRef.current.compareDocumentPosition(childARef.current), { @@ -1654,6 +1733,7 @@ describe('FragmentRefs', () => { implementationSpecific: false, }, ); + // Contained by in DOM, but following in React tree expectPosition( fragmentRef.current.compareDocumentPosition(childBRef.current), { @@ -1676,6 +1756,29 @@ describe('FragmentRefs', () => { implementationSpecific: false, }, ); + expectPosition( + fragmentRef.current.compareDocumentPosition(childDRef.current), + { + preceding: false, + following: false, + contains: false, + containedBy: true, + disconnected: false, + implementationSpecific: false, + }, + ); + // Preceding DOM but following in React tree + expectPosition( + fragmentRef.current.compareDocumentPosition(childERef.current), + { + preceding: false, + following: false, + contains: false, + containedBy: false, + disconnected: false, + implementationSpecific: true, + }, + ); }); // @gate enableFragmentRefs diff --git a/packages/react-reconciler/src/ReactFiberTreeReflection.js b/packages/react-reconciler/src/ReactFiberTreeReflection.js index 45da707dfea23..49a357adf8268 100644 --- a/packages/react-reconciler/src/ReactFiberTreeReflection.js +++ b/packages/react-reconciler/src/ReactFiberTreeReflection.js @@ -26,6 +26,7 @@ import { ActivityComponent, SuspenseComponent, OffscreenComponent, + Fragment, } from './ReactWorkTags'; import {NoFlags, Placement, Hydrating} from './ReactFiberFlags'; @@ -405,6 +406,21 @@ export function getFragmentParentHostFiber(fiber: Fiber): null | Fiber { return null; } +export function fiberIsPortaledIntoHost(fiber: Fiber): boolean { + let foundPortalParent = false; + let parent = fiber.return; + while (parent !== null) { + if (parent.tag === HostPortal) { + foundPortalParent = true; + } + if (parent.tag === HostRoot || parent.tag === HostComponent) { + break; + } + parent = parent.return; + } + return foundPortalParent; +} + export function getInstanceFromHostFiber(fiber: Fiber): I { switch (fiber.tag) { case HostComponent: @@ -443,22 +459,38 @@ function findNextSibling(child: Fiber): boolean { return true; } -export function isFiberContainedBy( - maybeChild: Fiber, - maybeParent: Fiber, +export function isFiberContainedByFragment( + fiber: Fiber, + fragmentFiber: Fiber, ): boolean { - let parent = maybeParent.return; - if (parent === maybeChild || parent === maybeChild.alternate) { - return true; + let current: Fiber | null = fiber; + while (current !== null) { + if ( + current.tag === Fragment && + (current === fragmentFiber || current.alternate === fragmentFiber) + ) { + return true; + } + current = current.return; } - while (parent !== null && parent !== maybeChild) { + return false; +} + +export function isFragmentContainedByFiber( + fragmentFiber: Fiber, + otherFiber: Fiber, +): boolean { + let current: Fiber | null = fragmentFiber; + const fiberHostParent: Fiber | null = + getFragmentParentHostFiber(fragmentFiber); + while (current !== null) { if ( - (parent.tag === HostComponent || parent.tag === HostRoot) && - (parent.return === maybeChild || parent.return === maybeChild.alternate) + (current.tag === HostComponent || current.tag === HostRoot) && + (current === fiberHostParent || current.alternate === fiberHostParent) ) { return true; } - parent = parent.return; + current = current.return; } return false; }