Skip to content
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

Merge restoreEnterViewTransitions and restoreExitViewTransitions #32585

Merged
merged 1 commit into from
Mar 14, 2025

Conversation

sebmarkbage
Copy link
Collaborator

This is the exact same code in both cases. It's just general clean up.

By unifying them it becomes less confusing to reuse these helpers in the Apply Gesture path where the naming is reversed.

@react-sizebot
Copy link

Comparing: 4ab827b...74328c4

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 = 518.75 kB 518.54 kB = 92.46 kB 92.45 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 = 591.76 kB 591.14 kB = 105.36 kB 105.33 kB
facebook-www/ReactDOM-prod.classic.js = 652.39 kB 643.70 kB = 114.72 kB 113.30 kB
facebook-www/ReactDOM-prod.modern.js = 642.71 kB 634.02 kB = 113.15 kB 111.73 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
facebook-www/ReactART-dev.classic.js = 710.82 kB 708.01 kB = 111.42 kB 111.10 kB
facebook-www/ReactART-dev.modern.js = 701.33 kB 698.51 kB = 109.58 kB 109.27 kB
oss-experimental/react-noop-renderer/cjs/react-noop-renderer-persistent.development.js = 42.13 kB 41.93 kB = 7.65 kB 7.63 kB
oss-stable/react-noop-renderer/cjs/react-noop-renderer-persistent.development.js = 42.13 kB 41.92 kB = 7.65 kB 7.62 kB
oss-stable-semver/react-noop-renderer/cjs/react-noop-renderer-persistent.development.js = 42.10 kB 41.90 kB = 7.62 kB 7.59 kB
oss-experimental/react-noop-renderer/cjs/react-noop-renderer.development.js = 41.99 kB 41.79 kB = 7.64 kB 7.61 kB
oss-stable/react-noop-renderer/cjs/react-noop-renderer.development.js = 41.99 kB 41.78 kB = 7.63 kB 7.60 kB
oss-stable-semver/react-noop-renderer/cjs/react-noop-renderer.development.js = 41.96 kB 41.76 kB = 7.60 kB 7.58 kB
oss-experimental/react-noop-renderer/cjs/react-noop-renderer-persistent.production.js = 37.85 kB 37.66 kB = 7.06 kB 7.03 kB
oss-stable/react-noop-renderer/cjs/react-noop-renderer-persistent.production.js = 37.85 kB 37.65 kB = 7.05 kB 7.03 kB
oss-stable-semver/react-noop-renderer/cjs/react-noop-renderer-persistent.production.js = 37.82 kB 37.63 kB = 7.02 kB 7.00 kB
oss-experimental/react-noop-renderer/cjs/react-noop-renderer.production.js = 37.73 kB 37.53 kB = 7.04 kB 7.01 kB
oss-stable/react-noop-renderer/cjs/react-noop-renderer.production.js = 37.72 kB 37.53 kB = 7.04 kB 7.01 kB
oss-stable-semver/react-noop-renderer/cjs/react-noop-renderer.production.js = 37.70 kB 37.50 kB = 7.01 kB 6.98 kB
facebook-www/ReactART-prod.classic.js = 386.46 kB 384.41 kB = 64.87 kB 64.60 kB
facebook-www/ReactART-prod.modern.js = 376.51 kB 374.46 kB = 63.24 kB 62.99 kB
facebook-www/ReactReconciler-dev.classic.js = 820.07 kB 814.22 kB = 127.37 kB 126.64 kB
facebook-www/ReactReconciler-dev.modern.js = 810.86 kB 805.01 kB = 125.69 kB 124.97 kB
facebook-www/ReactDOMTesting-dev.classic.js = 1,194.21 kB 1,184.20 kB = 198.99 kB 197.40 kB
facebook-www/ReactDOMTesting-dev.modern.js = 1,185.07 kB 1,175.06 kB = 197.20 kB 195.63 kB
facebook-www/ReactDOM-dev.classic.js = 1,177.67 kB 1,167.29 kB = 195.13 kB 193.49 kB
facebook-www/ReactDOM-dev.modern.js = 1,168.53 kB 1,158.15 kB = 193.41 kB 191.79 kB
facebook-www/ReactReconciler-prod.classic.js = 502.84 kB 497.73 kB = 80.27 kB 79.63 kB
facebook-www/ReactReconciler-prod.modern.js = 492.57 kB 487.46 kB = 78.67 kB 78.03 kB
facebook-www/ReactDOM-profiling.classic.js = 715.32 kB 706.63 kB = 123.87 kB 122.35 kB
facebook-www/ReactDOM-profiling.modern.js = 707.28 kB 698.58 kB = 122.54 kB 121.03 kB
facebook-www/ReactDOMTesting-prod.classic.js = 666.80 kB 658.42 kB = 118.38 kB 117.01 kB
facebook-www/ReactDOMTesting-prod.modern.js = 657.11 kB 648.74 kB = 116.77 kB 115.44 kB
facebook-www/ReactDOM-prod.classic.js = 652.39 kB 643.70 kB = 114.72 kB 113.30 kB
facebook-www/ReactDOM-prod.modern.js = 642.71 kB 634.02 kB = 113.15 kB 111.73 kB

Generated by 🚫 dangerJS against ea1f986

@sebmarkbage sebmarkbage merged commit 1b6e3dd into facebook:main Mar 14, 2025
196 checks passed
sebmarkbage added a commit that referenced this pull request Mar 14, 2025
Stacked on #32585 and #32605.

This adds more loops for the phases of "Apply Gesture". It doesn't
implement the interesting bit yet like adding view-transition-names and
measurements. I'll do that in a separate PR to keep reviewing easier.

The three phases of this approach is roughly:

- Clone and apply names to the "old" state.
- Inside startViewTransition: Apply names to the "new" state. Measure
both the "old" and "new" state to know whether to cancel some of them.
Delete the clones which will include all the "old" names.
- After startViewTransition: Restore "new" names back to no
view-transition-name.

Since we don't have any other Effects in these phases we have a bit more
flexibility and we can avoid extra phases that traverse the tree. I've
tried to avoid any additional passes.

An interesting consequence of this approach is that we could measure
both the "old" and "new" state before `startViewTransition`. This would
be more efficient because we wouldn't need to take View Transition
snapshots of parts of the tree that won't actually animate. However,
that would require an extra pass and force layout earlier. It would also
have different semantics from the fire-and-forget View Transitions
because we could optimize better which can be visible. It would also not
account for any late mutations. So I decided to instead let the layout
be computed by painting as usual and then measure both "old" and "new"
inside the startViewTransition instead. Then canceling anything that
doesn't animate to keep it consistent.

Unfortunately, though there's not a lot of code sharing possible in
these phases because the strategy is so different with the cloning and
because the animation is performed in reverse. The "finishedWork" Fiber
represents the "old" state and the "current" Fiber represents the "new"
state.

The most complicated phase is the cloning. I actually ended up having to
make a very different pattern from the other phases and CommitWork in
general. Because we have to clone as we go and also do other things like
apply names and finding pairs, it has more phases. I ended up with an
approach that uses three different loops. The outer one for updated
trees, one for inserted trees that don't need cloning (doesn't include
reappearing offscreen) and one for not updated trees that still need
cloning. Inside each loop it can also be in different phases which I
track with the `visitPhase` enum - this pattern is kind of new.

Additionally, we need to measure the cloned nodes after we've applied
mutations to them and we have to wait until the whole tree is inserted.
We don't have a reference to these DOM elements in the Fiber tree since
that still refers to the original ones. We need to store the cloned
elements somewhere. So I added a temporary field on the
ViewTransitionState to keep track of any clones owned by that
ViewTransition.

When we deep clone an unchanged subtree we don't have DOM element
instances. It wouldn't be quite safe to try to find them from the tree
structure. So we need to avoid the deep clones if we might need DOM
elements. Therefore we keep traversing in the case where we need to find
nested ViewTransition boundaries that are either potentially affected by
layout or a "pair".

For the other two phases the pattern there's a lot of code duplication
since it's slightly different from the commit ones but they at least
follow the same pattern. For the restore phase I was actually able to
reuse most of the code.

I don't love how much code this is.
github-actions bot pushed a commit that referenced this pull request Mar 14, 2025
)

This is the exact same code in both cases. It's just general clean up.

By unifying them it becomes less confusing to reuse these helpers in the
Apply Gesture path where the naming is reversed.

DiffTrain build for [1b6e3dd](1b6e3dd)
github-actions bot pushed a commit that referenced this pull request Mar 14, 2025
Stacked on #32585 and #32605.

This adds more loops for the phases of "Apply Gesture". It doesn't
implement the interesting bit yet like adding view-transition-names and
measurements. I'll do that in a separate PR to keep reviewing easier.

The three phases of this approach is roughly:

- Clone and apply names to the "old" state.
- Inside startViewTransition: Apply names to the "new" state. Measure
both the "old" and "new" state to know whether to cancel some of them.
Delete the clones which will include all the "old" names.
- After startViewTransition: Restore "new" names back to no
view-transition-name.

Since we don't have any other Effects in these phases we have a bit more
flexibility and we can avoid extra phases that traverse the tree. I've
tried to avoid any additional passes.

An interesting consequence of this approach is that we could measure
both the "old" and "new" state before `startViewTransition`. This would
be more efficient because we wouldn't need to take View Transition
snapshots of parts of the tree that won't actually animate. However,
that would require an extra pass and force layout earlier. It would also
have different semantics from the fire-and-forget View Transitions
because we could optimize better which can be visible. It would also not
account for any late mutations. So I decided to instead let the layout
be computed by painting as usual and then measure both "old" and "new"
inside the startViewTransition instead. Then canceling anything that
doesn't animate to keep it consistent.

Unfortunately, though there's not a lot of code sharing possible in
these phases because the strategy is so different with the cloning and
because the animation is performed in reverse. The "finishedWork" Fiber
represents the "old" state and the "current" Fiber represents the "new"
state.

The most complicated phase is the cloning. I actually ended up having to
make a very different pattern from the other phases and CommitWork in
general. Because we have to clone as we go and also do other things like
apply names and finding pairs, it has more phases. I ended up with an
approach that uses three different loops. The outer one for updated
trees, one for inserted trees that don't need cloning (doesn't include
reappearing offscreen) and one for not updated trees that still need
cloning. Inside each loop it can also be in different phases which I
track with the `visitPhase` enum - this pattern is kind of new.

Additionally, we need to measure the cloned nodes after we've applied
mutations to them and we have to wait until the whole tree is inserted.
We don't have a reference to these DOM elements in the Fiber tree since
that still refers to the original ones. We need to store the cloned
elements somewhere. So I added a temporary field on the
ViewTransitionState to keep track of any clones owned by that
ViewTransition.

When we deep clone an unchanged subtree we don't have DOM element
instances. It wouldn't be quite safe to try to find them from the tree
structure. So we need to avoid the deep clones if we might need DOM
elements. Therefore we keep traversing in the case where we need to find
nested ViewTransition boundaries that are either potentially affected by
layout or a "pair".

For the other two phases the pattern there's a lot of code duplication
since it's slightly different from the commit ones but they at least
follow the same pattern. For the restore phase I was actually able to
reuse most of the code.

I don't love how much code this is.

DiffTrain build for [c4a3b92](c4a3b92)
github-actions bot pushed a commit that referenced this pull request Mar 14, 2025
Stacked on #32585 and #32605.

This adds more loops for the phases of "Apply Gesture". It doesn't
implement the interesting bit yet like adding view-transition-names and
measurements. I'll do that in a separate PR to keep reviewing easier.

The three phases of this approach is roughly:

- Clone and apply names to the "old" state.
- Inside startViewTransition: Apply names to the "new" state. Measure
both the "old" and "new" state to know whether to cancel some of them.
Delete the clones which will include all the "old" names.
- After startViewTransition: Restore "new" names back to no
view-transition-name.

Since we don't have any other Effects in these phases we have a bit more
flexibility and we can avoid extra phases that traverse the tree. I've
tried to avoid any additional passes.

An interesting consequence of this approach is that we could measure
both the "old" and "new" state before `startViewTransition`. This would
be more efficient because we wouldn't need to take View Transition
snapshots of parts of the tree that won't actually animate. However,
that would require an extra pass and force layout earlier. It would also
have different semantics from the fire-and-forget View Transitions
because we could optimize better which can be visible. It would also not
account for any late mutations. So I decided to instead let the layout
be computed by painting as usual and then measure both "old" and "new"
inside the startViewTransition instead. Then canceling anything that
doesn't animate to keep it consistent.

Unfortunately, though there's not a lot of code sharing possible in
these phases because the strategy is so different with the cloning and
because the animation is performed in reverse. The "finishedWork" Fiber
represents the "old" state and the "current" Fiber represents the "new"
state.

The most complicated phase is the cloning. I actually ended up having to
make a very different pattern from the other phases and CommitWork in
general. Because we have to clone as we go and also do other things like
apply names and finding pairs, it has more phases. I ended up with an
approach that uses three different loops. The outer one for updated
trees, one for inserted trees that don't need cloning (doesn't include
reappearing offscreen) and one for not updated trees that still need
cloning. Inside each loop it can also be in different phases which I
track with the `visitPhase` enum - this pattern is kind of new.

Additionally, we need to measure the cloned nodes after we've applied
mutations to them and we have to wait until the whole tree is inserted.
We don't have a reference to these DOM elements in the Fiber tree since
that still refers to the original ones. We need to store the cloned
elements somewhere. So I added a temporary field on the
ViewTransitionState to keep track of any clones owned by that
ViewTransition.

When we deep clone an unchanged subtree we don't have DOM element
instances. It wouldn't be quite safe to try to find them from the tree
structure. So we need to avoid the deep clones if we might need DOM
elements. Therefore we keep traversing in the case where we need to find
nested ViewTransition boundaries that are either potentially affected by
layout or a "pair".

For the other two phases the pattern there's a lot of code duplication
since it's slightly different from the commit ones but they at least
follow the same pattern. For the restore phase I was actually able to
reuse most of the code.

I don't love how much code this is.

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