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

Remove Mutation Check Around commit/measureUpdateViewTransition #32617

Merged
merged 2 commits into from
Mar 14, 2025

Conversation

sebmarkbage
Copy link
Collaborator

There's two ways to find updated View Transitions.

One is the "commit/measureNestedViewTransitions" pass which is used to find things in unchanged subtrees. This can only lead to the relayout case since there's can't possibly be any mutations in the subtree. This is only triggered when none of the direct siblings have any mutations at all.

The other case is "commit/measureUpdateViewTransition" which is for a ViewTransition that itself has mutations scheduled inside of it which leads to the "update" case.

However, there's a case between these two cases. When a direct sibling has a mutation but there's also a ViewTransition exactly at the same level. In that case we can't bail out on the whole set of children so we won't trigger the "nested" case. Previously we also didn't trigger the "commit/measureUpdateViewTransition" case because we first checked if that had any mutations inside of it at all. This leads to neither case picking up this boundary.

We could check if the ViewTransition itself has any mutations inside and if not trigger the nested path.

There's a simpler way though. Because commit/measureUpdateViewTransition is actually just optimistic. The flags are pessimistic and we don't know for sure if there will actually be a mutation until we've traversed the tree. It can sometimes lead to the "relayout" case. So we can just use that same path, knowing that it'll just lead to the layout pass. Therefore it's safe to just remove this check.

…erMutation pass

Also add ContentReset which is also a type of mutation but it's usually
accompanied by an Update too.
@react-sizebot
Copy link

Comparing: c4a3b92...60a24ca

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.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 = 598.65 kB 598.35 kB = 106.29 kB 106.26 kB
facebook-www/ReactDOM-prod.classic.js = 649.91 kB 649.60 kB = 114.52 kB 114.49 kB
facebook-www/ReactDOM-prod.modern.js = 640.23 kB 639.92 kB = 112.95 kB 112.93 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 12fbbd9

@sebmarkbage sebmarkbage merged commit 17d274d into facebook:main Mar 14, 2025
196 checks passed
github-actions bot pushed a commit that referenced this pull request Mar 14, 2025
There's two ways to find updated View Transitions.

One is the "commit/measureNestedViewTransitions" pass which is used to
find things in unchanged subtrees. This can only lead to the relayout
case since there's can't possibly be any mutations in the subtree. This
is only triggered when none of the direct siblings have any mutations at
all.

The other case is "commit/measureUpdateViewTransition" which is for a
ViewTransition that itself has mutations scheduled inside of it which
leads to the "update" case.

However, there's a case between these two cases. When a direct sibling
has a mutation but there's also a ViewTransition exactly at the same
level. In that case we can't bail out on the whole set of children so we
won't trigger the "nested" case. Previously we also didn't trigger the
"commit/measureUpdateViewTransition" case because we first checked if
that had any mutations inside of it at all. This leads to neither case
picking up this boundary.

We could check if the ViewTransition itself has any mutations inside and
if not trigger the nested path.

There's a simpler way though. Because
`commit/measureUpdateViewTransition` is actually just optimistic. The
flags are pessimistic and we don't know for sure if there will actually
be a mutation until we've traversed the tree. It can sometimes lead to
the "relayout" case. So we can just use that same path, knowing that
it'll just lead to the layout pass. Therefore it's safe to just remove
this check.

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