Skip to content

Conversation

jackpope
Copy link
Contributor

@jackpope jackpope commented Aug 4, 2025

Stacked on #34069

Same basic semantics as the react-dom for determining document position of a Fragment compared to a given node. It's simpler here because we don't have to deal with inserted nodes or portals. So we can skip a bunch of the validation logic.

The logic for handling empty fragments is the same so I've split out compareDocumentPositionForEmptyFragment into a shared module. There doesn't seem to be a great place to put shared DOM logic between Fabric and DOM configs at the moment. There may be more of this coming as we add more and more DOM APIs to RN.

For testing I've written Fantom tests internally which pass the basic cases on this build. The renderer we have configured for Fabric tests in the repo doesn't support the Element APIs we need like compareDocumentPosition.

@meta-cla meta-cla bot added the CLA Signed label Aug 4, 2025
@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label Aug 4, 2025
@react-sizebot
Copy link

react-sizebot commented Aug 4, 2025

Comparing: a96a0f3...500e172

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.68 kB 6.68 kB +0.05% 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 530.18 kB 530.18 kB = 93.39 kB 93.39 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 655.86 kB 655.81 kB = 115.32 kB 115.30 kB
facebook-www/ReactDOM-prod.classic.js = 675.63 kB 675.58 kB = 118.55 kB 118.54 kB
facebook-www/ReactDOM-prod.modern.js = 666.06 kB 666.00 kB = 116.88 kB 116.87 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
react-native/implementations/ReactFabric-prod.fb.js +0.70% 378.88 kB 381.54 kB +0.76% 65.46 kB 65.95 kB
react-native/implementations/ReactFabric-profiling.fb.js +0.59% 448.41 kB 451.07 kB +0.67% 74.83 kB 75.34 kB
react-native/implementations/ReactFabric-dev.fb.js +0.37% 749.17 kB 751.93 kB +0.42% 118.91 kB 119.42 kB

Generated by 🚫 dangerJS against 500e172

@jackpope jackpope force-pushed the compare-doc-position-rn2 branch from d939075 to 0166a05 Compare August 4, 2025 21:23
@jackpope jackpope requested a review from rubennorte August 13, 2025 14:51
@jackpope jackpope force-pushed the compare-doc-position-rn2 branch from 0166a05 to 8e7d87d Compare August 15, 2025 15:04
@jackpope jackpope force-pushed the compare-doc-position-rn2 branch from 8e7d87d to 500e172 Compare August 15, 2025 16:16
@jackpope jackpope merged commit 45a6532 into facebook:main Aug 15, 2025
241 checks passed
@jackpope jackpope deleted the compare-doc-position-rn2 branch August 15, 2025 19:07
github-actions bot pushed a commit that referenced this pull request Aug 15, 2025
Stacked on #34069

Same basic semantics as the react-dom for determining document position
of a Fragment compared to a given node. It's simpler here because we
don't have to deal with inserted nodes or portals. So we can skip a
bunch of the validation logic.

The logic for handling empty fragments is the same so I've split out
`compareDocumentPositionForEmptyFragment` into a shared module. There
doesn't seem to be a great place to put shared DOM logic between Fabric
and DOM configs at the moment. There may be more of this coming as we
add more and more DOM APIs to RN.

For testing I've written Fantom tests internally which pass the basic
cases on this build. The renderer we have configured for Fabric tests in
the repo doesn't support the Element APIs we need like
`compareDocumentPosition`.

DiffTrain build for [45a6532](45a6532)
github-actions bot pushed a commit that referenced this pull request Aug 15, 2025
Stacked on #34069

Same basic semantics as the react-dom for determining document position
of a Fragment compared to a given node. It's simpler here because we
don't have to deal with inserted nodes or portals. So we can skip a
bunch of the validation logic.

The logic for handling empty fragments is the same so I've split out
`compareDocumentPositionForEmptyFragment` into a shared module. There
doesn't seem to be a great place to put shared DOM logic between Fabric
and DOM configs at the moment. There may be more of this coming as we
add more and more DOM APIs to RN.

For testing I've written Fantom tests internally which pass the basic
cases on this build. The renderer we have configured for Fabric tests in
the repo doesn't support the Element APIs we need like
`compareDocumentPosition`.

DiffTrain build for [45a6532](45a6532)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team Opened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants