Skip to content

Add compareDocumentPosition to fragment instances #32722

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 6, 2025
Merged

Add compareDocumentPosition to fragment instances #32722

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
00b78c5
Add compareDocumentPosition to fragment instances
jackpope Mar 18, 2025
f916dbc
Return disconnected when fragment instance is unmounted
jackpope Apr 4, 2025
cffff32
Support empty fragment positions
jackpope Apr 4, 2025
ec67519
Dont consider portals target children for layout related methods
jackpope Apr 4, 2025
b57b847
Compare Fiber tree position with DOM position
jackpope Apr 11, 2025
c6a043b
Refactor search traversals to avoid obj allocations
jackpope May 6, 2025
5f155a2
Fix fiber/instance issues in Fabric
jackpope May 6, 2025
a57f3dd
Make Instance type generic
jackpope May 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Make Instance type generic
  • Loading branch information
@jackpope
jackpope committed May 6, 2025
commit a57f3dd27dd6e569e4e336aff8aefe9de43dc8cc
28 changes: 16 additions & 12 deletions packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2649,7 +2649,7 @@ function addEventListenerToChild(
listener: EventListener,
optionsOrUseCapture?: EventListenerOptionsOrUseCapture,
): boolean {
const instance = getInstanceFromHostFiber(child);
const instance = getInstanceFromHostFiber<Instance>(child);
instance.addEventListener(type, listener, optionsOrUseCapture);
return false;
}
Expand Down Expand Up @@ -2689,7 +2689,7 @@ function removeEventListenerFromChild(
listener: EventListener,
optionsOrUseCapture?: EventListenerOptionsOrUseCapture,
): boolean {
const instance = getInstanceFromHostFiber(child);
const instance = getInstanceFromHostFiber<Instance>(child);
instance.removeEventListener(type, listener, optionsOrUseCapture);
return false;
}
Expand All @@ -2708,7 +2708,7 @@ function setFocusOnFiberIfFocusable(
fiber: Fiber,
focusOptions?: FocusOptions,
): boolean {
const instance = getInstanceFromHostFiber(fiber);
const instance = getInstanceFromHostFiber<Instance>(fiber);
return setFocusIfFocusable(instance, focusOptions);
}
// $FlowFixMe[prop-missing]
Expand Down Expand Up @@ -2740,7 +2740,7 @@ FragmentInstance.prototype.blur = function (this: FragmentInstanceType): void {
};
function blurActiveElementWithinFragment(child: Fiber): boolean {
// TODO: We can get the activeElement from the parent outside of the loop when we have a reference.
const instance = getInstanceFromHostFiber(child);
const instance = getInstanceFromHostFiber<Instance>(child);
const ownerDocument = instance.ownerDocument;
if (instance === ownerDocument.activeElement) {
// $FlowFixMe[prop-missing]
Expand All @@ -2764,7 +2764,7 @@ function observeChild(
child: Fiber,
observer: IntersectionObserver | ResizeObserver,
) {
const instance = getInstanceFromHostFiber(child);
const instance = getInstanceFromHostFiber<Instance>(child);
observer.observe(instance);
return false;
}
Expand All @@ -2789,7 +2789,7 @@ function unobserveChild(
child: Fiber,
observer: IntersectionObserver | ResizeObserver,
) {
const instance = getInstanceFromHostFiber(child);
const instance = getInstanceFromHostFiber<Instance>(child);
observer.unobserve(instance);
return false;
}
Expand All @@ -2802,7 +2802,7 @@ FragmentInstance.prototype.getClientRects = function (
return rects;
};
function collectClientRects(child: Fiber, rects: Array<DOMRect>): boolean {
const instance = getInstanceFromHostFiber(child);
const instance = getInstanceFromHostFiber<Instance>(child);
// $FlowFixMe[method-unbinding]
rects.push.apply(rects, instance.getClientRects());
return false;
Expand All @@ -2816,7 +2816,8 @@ FragmentInstance.prototype.getRootNode = function (
if (parentHostFiber === null) {
return this;
}
const parentHostInstance = getInstanceFromHostFiber(parentHostFiber);
const parentHostInstance =
getInstanceFromHostFiber<Instance>(parentHostFiber);
const rootNode =
// $FlowFixMe[incompatible-cast] Flow expects Node
(parentHostInstance.getRootNode(getRootNodeOptions): Document | ShadowRoot);
Expand All @@ -2838,7 +2839,8 @@ FragmentInstance.prototype.compareDocumentPosition = function (
if (children.length === 0) {
Copy link
Contributor Author

@jackpope jackpope Apr 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated support for empty fragments to compare against parent/siblings

// If the fragment has no children, we can use the parent and
// siblings to determine a position.
const parentHostInstance = getInstanceFromHostFiber(parentHostFiber);
const parentHostInstance =
getInstanceFromHostFiber<Instance>(parentHostFiber);
const parentResult = parentHostInstance.compareDocumentPosition(otherNode);
result = parentResult;
if (parentHostInstance === otherNode) {
Expand All @@ -2852,7 +2854,7 @@ FragmentInstance.prototype.compareDocumentPosition = function (
result = Node.DOCUMENT_POSITION_PRECEDING;
} else {
const nextSiblingInstance =
getInstanceFromHostFiber(nextSiblingFiber);
getInstanceFromHostFiber<Instance>(nextSiblingFiber);
const nextSiblingResult =
nextSiblingInstance.compareDocumentPosition(otherNode);
if (
Expand All @@ -2871,8 +2873,10 @@ FragmentInstance.prototype.compareDocumentPosition = function (
return result;
}

const firstElement = getInstanceFromHostFiber(children[0]);
const lastElement = getInstanceFromHostFiber(children[children.length - 1]);
const firstElement = getInstanceFromHostFiber<Instance>(children[0]);
const lastElement = getInstanceFromHostFiber<Instance>(
children[children.length - 1],
);
const firstResult = firstElement.compareDocumentPosition(otherNode);
const lastResult = lastElement.compareDocumentPosition(otherNode);
if (
Expand Down
4 changes: 2 additions & 2 deletions packages/react-native-renderer/src/ReactFiberConfigFabric.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ FragmentInstance.prototype.observeUsing = function (
traverseFragmentInstance(this._fragmentFiber, observeChild, observer);
};
function observeChild(child: Fiber, observer: IntersectionObserver) {
const instance = getInstanceFromHostFiber(child);
const instance = getInstanceFromHostFiber<Instance>(child);
const publicInstance = getPublicInstance(instance);
if (publicInstance == null) {
throw new Error('Expected to find a host node. This is a bug in React.');
Expand All @@ -671,7 +671,7 @@ FragmentInstance.prototype.unobserveUsing = function (
}
};
function unobserveChild(child: Fiber, observer: IntersectionObserver) {
const instance = getInstanceFromHostFiber(child);
const instance = getInstanceFromHostFiber<Instance>(child);
const publicInstance = getPublicInstance(instance);
if (publicInstance == null) {
throw new Error('Expected to find a host node. This is a bug in React.');
Expand Down
3 changes: 1 addition & 2 deletions packages/react-reconciler/src/ReactFiberTreeReflection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import type {
Container,
ActivityInstance,
SuspenseInstance,
Instance,
} from './ReactFiberConfig';
import type {ActivityState} from './ReactFiberActivityComponent';
import type {SuspenseState} from './ReactFiberSuspenseComponent';
Expand Down Expand Up @@ -411,7 +410,7 @@ export function getFragmentParentHostFiber(fiber: Fiber): null | Fiber {
return null;
}

export function getInstanceFromHostFiber(fiber: Fiber): Instance {
export function getInstanceFromHostFiber<I>(fiber: Fiber): I {
switch (fiber.tag) {
case HostComponent:
return fiber.stateNode;
Expand Down
Loading