Skip to content

Find Pairs and Apply View Transition Names to the Clones in the "old" Phase #32599

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.

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 6 commits into from
Mar 14, 2025

Conversation

sebmarkbage
Copy link
Collaborator

Stacked on #32578.

We need to apply view-transition-names to the clones that we create in the "old" phase for the ViewTransition boundaries that should activate.

Finding pairs is a little trickier than in ReactFiberCommitViewTransitions. Normally we collect all name "insertions" in the accumulateSuspenseyCommit phase before we even commit. Then in the snapshot do we visit all "deletions" and since we already collected all the insertions we know immediately if the deletion had a pair and should therefore get a "name" assigned to activate the boundary. For ReactFiberApplyGesture we need to assign names to "insertions" since it's in reverse but we don't already have a map of deletions. Therefore we need to first visit all deletions.

Instead of doing that in a completely separate pass, we instead visit deletions in the same pass to find pairs. Since this is in the same pass we might visit insertions before deletions or vice versa depending on document order. However, we can deal with this by applying the name to the insertion when we find the deletion if we've already made the clones at that point.

Applying names to pure exits, updates or nested (relayout) is a bit more straight-forward.

@react-sizebot
Copy link

react-sizebot commented Mar 13, 2025

Comparing: c4a3b92...1652bc4

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 = 516.17 kB 516.17 kB = 92.15 kB 92.15 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB +0.05% 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js +0.90% 598.65 kB 604.03 kB +0.89% 106.29 kB 107.23 kB
facebook-www/ReactDOM-prod.classic.js +0.03% 649.91 kB 650.09 kB +0.02% 114.52 kB 114.54 kB
facebook-www/ReactDOM-prod.modern.js +0.03% 640.23 kB 640.40 kB +0.02% 112.95 kB 112.98 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/react-reconciler/cjs/react-reconciler.production.js +1.18% 452.37 kB 457.70 kB +1.37% 72.51 kB 73.50 kB
oss-experimental/react-reconciler/cjs/react-reconciler.profiling.js +1.05% 505.33 kB 510.66 kB +1.23% 80.46 kB 81.44 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js +0.90% 598.65 kB 604.03 kB +0.89% 106.29 kB 107.23 kB
oss-experimental/react-dom/cjs/react-dom-unstable_testing.production.js +0.88% 613.38 kB 618.75 kB +0.87% 109.88 kB 110.84 kB
oss-experimental/react-reconciler/cjs/react-reconciler.development.js +0.86% 747.56 kB 753.98 kB +0.94% 117.47 kB 118.58 kB
oss-experimental/react-dom/cjs/react-dom-profiling.profiling.js +0.82% 654.06 kB 659.43 kB +0.82% 115.01 kB 115.95 kB
oss-experimental/react-dom/cjs/react-dom-client.development.js +0.59% 1,082.12 kB 1,088.53 kB +0.59% 181.09 kB 182.15 kB
oss-experimental/react-dom/cjs/react-dom-profiling.development.js +0.58% 1,098.51 kB 1,104.92 kB +0.57% 183.94 kB 184.98 kB
oss-experimental/react-dom/cjs/react-dom-unstable_testing.development.js +0.58% 1,099.03 kB 1,105.44 kB +0.58% 184.87 kB 185.93 kB

Generated by 🚫 dangerJS against 851d21f

This is a little different because we have forked paths for insertions of
new DOM nodes, vs cloning of existing DOM nodes inside a hidden Offscreen.
But same principle.

Note that we don't know for sure when we find an insertion whether we'll
find a deletion later but if we do then that will override anyway.
This applies the "old" side of the update to the clones.
…er state

This doesn't matter for CommitViewTransitions since we only read on one side
but it matters for ApplyGesture when we read on the reverse end.
Copy link
Member

@rickhanlonii rickhanlonii left a comment

Choose a reason for hiding this comment

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

image

@sebmarkbage sebmarkbage merged commit 2e38573 into facebook:main Mar 14, 2025
194 checks passed
github-actions bot pushed a commit that referenced this pull request Mar 14, 2025
… Phase (#32599)

Stacked on #32578.

We need to apply view-transition-names to the clones that we create in
the "old" phase for the ViewTransition boundaries that should activate.

Finding pairs is a little trickier than in
ReactFiberCommitViewTransitions. Normally we collect all name
"insertions" in the `accumulateSuspenseyCommit` phase before we even
commit. Then in the snapshot do we visit all "deletions" and since we
already collected all the insertions we know immediately if the deletion
had a pair and should therefore get a "name" assigned to activate the
boundary. For ReactFiberApplyGesture we need to assign names to
"insertions" since it's in reverse but we don't already have a map of
deletions. Therefore we need to first visit all deletions.

Instead of doing that in a completely separate pass, we instead visit
deletions in the same pass to find pairs. Since this is in the same pass
we might visit insertions before deletions or vice versa depending on
document order. However, we can deal with this by applying the name to
the insertion when we find the deletion if we've already made the clones
at that point.

Applying names to pure exits, updates or nested (relayout) is a bit more
straight-forward.

DiffTrain build for [2e38573](2e38573)
sebmarkbage added a commit that referenced this pull request Mar 14, 2025
Stacked on #32599 and #32611.

This is able to reuse the code from CommitViewTransitions for "enter",
"shared" and "layout". The difference is that for "enter"/"shared" in
the "new" phase we pass in the deletions.

For "layout" of nested boundaries we just need to measure the clones at
the same time we measure the original nodes since we haven't measured
them in a previous phase in the current approach.

With these updates, things move around more like expected in the fixture
because we're now applying the appropriate pairs to trigger individual
animations instead of just the full document cross-fade.

The "update" phase is a little more complicated and is coming soon.
github-actions bot pushed a commit that referenced this pull request Mar 14, 2025
Stacked on #32599 and #32611.

This is able to reuse the code from CommitViewTransitions for "enter",
"shared" and "layout". The difference is that for "enter"/"shared" in
the "new" phase we pass in the deletions.

For "layout" of nested boundaries we just need to measure the clones at
the same time we measure the original nodes since we haven't measured
them in a previous phase in the current approach.

With these updates, things move around more like expected in the fixture
because we're now applying the appropriate pairs to trigger individual
animations instead of just the full document cross-fade.

The "update" phase is a little more complicated and is coming soon.

DiffTrain build for [2c56037](2c56037)
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.

4 participants