Skip to content

Measure and apply names for the "new" phase #32612

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

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented 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.

@react-sizebot
Copy link

react-sizebot commented Mar 14, 2025

Comparing: 2e38573...3875508

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 = 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.14 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 +0.12% 604.03 kB 604.73 kB +0.10% 107.23 kB 107.34 kB
facebook-www/ReactDOM-prod.classic.js +0.04% 650.09 kB 650.34 kB +0.07% 114.54 kB 114.61 kB
facebook-www/ReactDOM-prod.modern.js +0.04% 640.40 kB 640.65 kB +0.07% 112.98 kB 113.05 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-art/cjs/react-art.production.js +1.62% 329.45 kB 334.78 kB +1.35% 55.89 kB 56.65 kB
oss-experimental/react-art/cjs/react-art.development.js +0.98% 623.71 kB 629.85 kB +0.78% 99.14 kB 99.91 kB

Generated by 🚫 dangerJS against e3cbaff

We can reuse the code from CommitViewTransitions here. It's the same except
we pass in the "current" deleted Fiber instead of the placement.
We can reuse the code from CommitViewTransitions here.

The difference is that instead of getting the measurements from a previous
pass, we measure them at the same time except we measure the clones first.
@sebmarkbage sebmarkbage merged commit 2c56037 into facebook:main Mar 14, 2025
194 checks passed
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)
sebmarkbage added a commit that referenced this pull request Mar 18, 2025
This does the same thing for `measureUpdateViewTransition` that we did
for `measureNestedViewTransitions` in
e3cbaff.
If a boundary hasn't mutated and didn't change in size, we mark it for
cancellation. Otherwise we add names to it. The different from the
CommitViewTransition path is that the "old" names are added to the
clones so this is the first time the "new" names.

Now we also cancel any boundaries that were unchanged. So now the root
no longer animates. We still have to clone them. There are other
optimizations that can avoid cloning but once we've done all the layouts
we can still cancel the running animation and let them just be the
regular content if they didn't change. Just like the regular
fire-and-forget path.

This also fixes the measurement so that we measure clones by adjusting
their position back into the viewport.

This actually surfaces a bug in Safari that was already in #32612. It
turns out that the old names aren't picked up for some reason and so in
Safari they looked more like a cross-fade than what #32612 was supposed
to fix. However, now that bug is even more apparent because they
actually just disappear in Safari. I'm not sure what that bug is but
it's unrelated to this PR so will fix that separately.
github-actions bot pushed a commit that referenced this pull request Mar 18, 2025
This does the same thing for  that we did
for  in
e3cbaff.
If a boundary hasn't mutated and didn't change in size, we mark it for
cancellation. Otherwise we add names to it. The different from the
CommitViewTransition path is that the old names are added to the
clones so this is the first time the new names.

Now we also cancel any boundaries that were unchanged. So now the root
no longer animates. We still have to clone them. There are other
optimizations that can avoid cloning but once we've done all the layouts
we can still cancel the running animation and let them just be the
regular content if they didn't change. Just like the regular
fire-and-forget path.

This also fixes the measurement so that we measure clones by adjusting
their position back into the viewport.

This actually surfaces a bug in Safari that was already in #32612. It
turns out that the old names aren't picked up for some reason and so in
Safari they looked more like a cross-fade than what #32612 was supposed
to fix. However, now that bug is even more apparent because they
actually just disappear in Safari. I'm not sure what that bug is but
it's unrelated to this PR so will fix that separately.

DiffTrain build for [3c3696d](3c3696d)
github-actions bot pushed a commit that referenced this pull request Mar 18, 2025
This does the same thing for  that we did
for  in
e3cbaff.
If a boundary hasn't mutated and didn't change in size, we mark it for
cancellation. Otherwise we add names to it. The different from the
CommitViewTransition path is that the old names are added to the
clones so this is the first time the new names.

Now we also cancel any boundaries that were unchanged. So now the root
no longer animates. We still have to clone them. There are other
optimizations that can avoid cloning but once we've done all the layouts
we can still cancel the running animation and let them just be the
regular content if they didn't change. Just like the regular
fire-and-forget path.

This also fixes the measurement so that we measure clones by adjusting
their position back into the viewport.

This actually surfaces a bug in Safari that was already in #32612. It
turns out that the old names aren't picked up for some reason and so in
Safari they looked more like a cross-fade than what #32612 was supposed
to fix. However, now that bug is even more apparent because they
actually just disappear in Safari. I'm not sure what that bug is but
it's unrelated to this PR so will fix that separately.

DiffTrain build for [3c3696d](3c3696d)
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