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

[Fiber] Track Appearing Named ViewTransition in the accumulateSuspenseyCommit Phase #32254

Merged
merged 2 commits into from
Jan 30, 2025

Conversation

sebmarkbage
Copy link
Collaborator

When a named ViewTransition component unmounts in one place and mounts in a different place we need to match these up so we know a pair has been created. Since the unmounts are tracked in the snapshot phase we need some way to track the mounts before that.

Originally the way I did that is by reusing the render phase since there was no other phase in the commit before that. However, that's not quite correct. Just because something is visited in render doesn't mean it'll commit. E.g. if that tree ends up suspending or erroring. Which would lead to a false positive on match. The unmount shouldn't animate in that case.

(Un)fortunately we have already added a traversal before the snapshot phase for tracking suspensey CSS. The accumulateSuspenseyCommit phase. This needs to find new mounts of Suspensey CSS or if there was a reappearing Offscreen boundary it needs to find any Suspensey CSS already inside that tree. This is exactly the same traversal we need to find newly appearing View Transition components. So we can just reuse that.

@react-sizebot
Copy link

react-sizebot commented Jan 28, 2025

Comparing: f02ba2f...2e0ad18

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 = 515.10 kB 514.64 kB = 91.81 kB 91.76 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB +0.11% 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 558.79 kB 557.67 kB = 99.13 kB 99.01 kB
facebook-www/ReactDOM-prod.classic.js = 597.07 kB 596.61 kB = 104.95 kB 104.89 kB
facebook-www/ReactDOM-prod.modern.js = 587.50 kB 587.04 kB = 103.41 kB 103.36 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-dom/cjs/react-dom-client.production.js = 558.79 kB 557.67 kB = 99.13 kB 99.01 kB
oss-experimental/react-dom/cjs/react-dom-profiling.profiling.js = 614.48 kB 613.19 kB = 107.85 kB 107.72 kB
oss-experimental/react-reconciler/cjs/react-reconciler.development.js = 716.63 kB 715.01 kB = 112.97 kB 112.82 kB
oss-experimental/react-art/cjs/react-art.development.js = 614.76 kB 613.10 kB = 97.89 kB 97.76 kB
oss-experimental/react-reconciler/cjs/react-reconciler.profiling.js = 481.65 kB 480.35 kB = 76.73 kB 76.62 kB
oss-experimental/react-reconciler/cjs/react-reconciler.production.js = 428.68 kB 427.45 kB = 68.91 kB 68.84 kB
oss-experimental/react-art/cjs/react-art.production.js = 322.53 kB 321.32 kB = 54.87 kB 54.77 kB

Generated by 🚫 dangerJS against e6ff683

This only keeps the map around between the accumulateSuspenseyCommit
and the beforeMutation phase but doesn't actually track yet.
@sebmarkbage sebmarkbage merged commit 4b3728f into facebook:main Jan 30, 2025
191 checks passed
github-actions bot pushed a commit that referenced this pull request Jan 30, 2025
…eyCommit Phase (#32254)

When a named ViewTransition component unmounts in one place and mounts
in a different place we need to match these up so we know a pair has
been created. Since the unmounts are tracked in the snapshot phase we
need some way to track the mounts before that.

Originally the way I did that is by reusing the render phase since there
was no other phase in the commit before that. However, that's not quite
correct. Just because something is visited in render doesn't mean it'll
commit. E.g. if that tree ends up suspending or erroring. Which would
lead to a false positive on match. The unmount shouldn't animate in that
case.

(Un)fortunately we have already added a traversal before the snapshot
phase for tracking suspensey CSS. The `accumulateSuspenseyCommit` phase.
This needs to find new mounts of Suspensey CSS or if there was a
reappearing Offscreen boundary it needs to find any Suspensey CSS
already inside that tree. This is exactly the same traversal we need to
find newly appearing View Transition components. So we can just reuse
that.

DiffTrain build for [4b3728f](4b3728f)
github-actions bot pushed a commit that referenced this pull request Jan 30, 2025
…eyCommit Phase (#32254)

When a named ViewTransition component unmounts in one place and mounts
in a different place we need to match these up so we know a pair has
been created. Since the unmounts are tracked in the snapshot phase we
need some way to track the mounts before that.

Originally the way I did that is by reusing the render phase since there
was no other phase in the commit before that. However, that's not quite
correct. Just because something is visited in render doesn't mean it'll
commit. E.g. if that tree ends up suspending or erroring. Which would
lead to a false positive on match. The unmount shouldn't animate in that
case.

(Un)fortunately we have already added a traversal before the snapshot
phase for tracking suspensey CSS. The `accumulateSuspenseyCommit` phase.
This needs to find new mounts of Suspensey CSS or if there was a
reappearing Offscreen boundary it needs to find any Suspensey CSS
already inside that tree. This is exactly the same traversal we need to
find newly appearing View Transition components. So we can just reuse
that.

DiffTrain build for [4b3728f](4b3728f)
github-actions bot pushed a commit to code/lib-react that referenced this pull request Jan 30, 2025
…eyCommit Phase (facebook#32254)

When a named ViewTransition component unmounts in one place and mounts
in a different place we need to match these up so we know a pair has
been created. Since the unmounts are tracked in the snapshot phase we
need some way to track the mounts before that.

Originally the way I did that is by reusing the render phase since there
was no other phase in the commit before that. However, that's not quite
correct. Just because something is visited in render doesn't mean it'll
commit. E.g. if that tree ends up suspending or erroring. Which would
lead to a false positive on match. The unmount shouldn't animate in that
case.

(Un)fortunately we have already added a traversal before the snapshot
phase for tracking suspensey CSS. The `accumulateSuspenseyCommit` phase.
This needs to find new mounts of Suspensey CSS or if there was a
reappearing Offscreen boundary it needs to find any Suspensey CSS
already inside that tree. This is exactly the same traversal we need to
find newly appearing View Transition components. So we can just reuse
that.

DiffTrain build for [4b3728f](facebook@4b3728f)
github-actions bot pushed a commit to code/lib-react that referenced this pull request Jan 30, 2025
…eyCommit Phase (facebook#32254)

When a named ViewTransition component unmounts in one place and mounts
in a different place we need to match these up so we know a pair has
been created. Since the unmounts are tracked in the snapshot phase we
need some way to track the mounts before that.

Originally the way I did that is by reusing the render phase since there
was no other phase in the commit before that. However, that's not quite
correct. Just because something is visited in render doesn't mean it'll
commit. E.g. if that tree ends up suspending or erroring. Which would
lead to a false positive on match. The unmount shouldn't animate in that
case.

(Un)fortunately we have already added a traversal before the snapshot
phase for tracking suspensey CSS. The `accumulateSuspenseyCommit` phase.
This needs to find new mounts of Suspensey CSS or if there was a
reappearing Offscreen boundary it needs to find any Suspensey CSS
already inside that tree. This is exactly the same traversal we need to
find newly appearing View Transition components. So we can just reuse
that.

DiffTrain build for [4b3728f](facebook@4b3728f)
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.

5 participants