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

Execute layout phase before after mutation phase inside view transition #32029

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Jan 9, 2025

This allows mutations and scrolling in the layout phase to be counted towards the mutation. This would maybe not be the case for gestures but it is useful for fire-and-forget.

This also avoids the issue that if you resolve navigation in useLayoutEffect that it ends up dead locked.

It also means that useLayoutEffect does not observe the scroll restoration and in fact, the scroll restoration would win over any manual scrolling in layout effects. For better or worse, this is more in line with how things worked before and how it works in popstate. So it's less of a breaking change. This does mean that we can't unify the after mutation phase with the layout phase though.

To do this we need split out flushSpawnedWork from the flushLayoutEffect call.

Spawned work from setState inside the layout phase is done outside and not counted towards the transition. They're sync updates and so are not eligible for their own View Transitions. It's also tricky to support this since it's unclear what things like exits in that update would mean. This work will still be able to mutate the live DOM but it's just not eligible to trigger new transitions or adjust the target of those.

One difference between popstate is that this spawned work is after scroll restoration. So any scrolling spawned from a second pass would now win over scroll restoration.

Another consequence of this change is that you can't safely animate pseudo elements in useLayoutEffect. We'll introduce a better event for that anyway.

@react-sizebot
Copy link

react-sizebot commented Jan 9, 2025

Comparing: 800c9db...e25fd55

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 +0.02% 513.86 kB 513.98 kB +0.01% 91.78 kB 91.79 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.04% 546.98 kB 547.19 kB +0.05% 97.31 kB 97.36 kB
facebook-www/ReactDOM-prod.classic.js +0.02% 595.76 kB 595.86 kB +0.02% 104.86 kB 104.88 kB
facebook-www/ReactDOM-prod.modern.js +0.02% 586.19 kB 586.29 kB +0.01% 103.31 kB 103.32 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 331b8f8

This allows mutations and scrolling in the layout phase to be counted
towards the mutation.

Spawned work from setState inside the layout phase is done outside and not
counted towards the transition.
@sebmarkbage sebmarkbage merged commit fd9cfa4 into facebook:main Jan 9, 2025
186 checks passed
github-actions bot pushed a commit that referenced this pull request Jan 9, 2025
…on (#32029)

This allows mutations and scrolling in the layout phase to be counted
towards the mutation. This would maybe not be the case for gestures but
it is useful for fire-and-forget.

This also avoids the issue that if you resolve navigation in
useLayoutEffect that it ends up dead locked.

It also means that useLayoutEffect does not observe the scroll
restoration and in fact, the scroll restoration would win over any
manual scrolling in layout effects. For better or worse, this is more in
line with how things worked before and how it works in popstate. So it's
less of a breaking change. This does mean that we can't unify the after
mutation phase with the layout phase though.

To do this we need split out flushSpawnedWork from the flushLayoutEffect
call.

Spawned work from setState inside the layout phase is done outside and
not counted towards the transition. They're sync updates and so are not
eligible for their own View Transitions. It's also tricky to support
this since it's unclear what things like exits in that update would
mean. This work will still be able to mutate the live DOM but it's just
not eligible to trigger new transitions or adjust the target of those.

One difference between popstate is that this spawned work is after
scroll restoration. So any scrolling spawned from a second pass would
now win over scroll restoration.

Another consequence of this change is that you can't safely animate
pseudo elements in useLayoutEffect. We'll introduce a better event for
that anyway.

DiffTrain build for [fd9cfa4](fd9cfa4)
github-actions bot pushed a commit that referenced this pull request Jan 9, 2025
…on (#32029)

This allows mutations and scrolling in the layout phase to be counted
towards the mutation. This would maybe not be the case for gestures but
it is useful for fire-and-forget.

This also avoids the issue that if you resolve navigation in
useLayoutEffect that it ends up dead locked.

It also means that useLayoutEffect does not observe the scroll
restoration and in fact, the scroll restoration would win over any
manual scrolling in layout effects. For better or worse, this is more in
line with how things worked before and how it works in popstate. So it's
less of a breaking change. This does mean that we can't unify the after
mutation phase with the layout phase though.

To do this we need split out flushSpawnedWork from the flushLayoutEffect
call.

Spawned work from setState inside the layout phase is done outside and
not counted towards the transition. They're sync updates and so are not
eligible for their own View Transitions. It's also tricky to support
this since it's unclear what things like exits in that update would
mean. This work will still be able to mutate the live DOM but it's just
not eligible to trigger new transitions or adjust the target of those.

One difference between popstate is that this spawned work is after
scroll restoration. So any scrolling spawned from a second pass would
now win over scroll restoration.

Another consequence of this change is that you can't safely animate
pseudo elements in useLayoutEffect. We'll introduce a better event for
that anyway.

DiffTrain build for [fd9cfa4](fd9cfa4)
github-actions bot pushed a commit to code/lib-react that referenced this pull request Jan 9, 2025
…on (facebook#32029)

This allows mutations and scrolling in the layout phase to be counted
towards the mutation. This would maybe not be the case for gestures but
it is useful for fire-and-forget.

This also avoids the issue that if you resolve navigation in
useLayoutEffect that it ends up dead locked.

It also means that useLayoutEffect does not observe the scroll
restoration and in fact, the scroll restoration would win over any
manual scrolling in layout effects. For better or worse, this is more in
line with how things worked before and how it works in popstate. So it's
less of a breaking change. This does mean that we can't unify the after
mutation phase with the layout phase though.

To do this we need split out flushSpawnedWork from the flushLayoutEffect
call.

Spawned work from setState inside the layout phase is done outside and
not counted towards the transition. They're sync updates and so are not
eligible for their own View Transitions. It's also tricky to support
this since it's unclear what things like exits in that update would
mean. This work will still be able to mutate the live DOM but it's just
not eligible to trigger new transitions or adjust the target of those.

One difference between popstate is that this spawned work is after
scroll restoration. So any scrolling spawned from a second pass would
now win over scroll restoration.

Another consequence of this change is that you can't safely animate
pseudo elements in useLayoutEffect. We'll introduce a better event for
that anyway.

DiffTrain build for [fd9cfa4](facebook@fd9cfa4)
github-actions bot pushed a commit to code/lib-react that referenced this pull request Jan 9, 2025
…on (facebook#32029)

This allows mutations and scrolling in the layout phase to be counted
towards the mutation. This would maybe not be the case for gestures but
it is useful for fire-and-forget.

This also avoids the issue that if you resolve navigation in
useLayoutEffect that it ends up dead locked.

It also means that useLayoutEffect does not observe the scroll
restoration and in fact, the scroll restoration would win over any
manual scrolling in layout effects. For better or worse, this is more in
line with how things worked before and how it works in popstate. So it's
less of a breaking change. This does mean that we can't unify the after
mutation phase with the layout phase though.

To do this we need split out flushSpawnedWork from the flushLayoutEffect
call.

Spawned work from setState inside the layout phase is done outside and
not counted towards the transition. They're sync updates and so are not
eligible for their own View Transitions. It's also tricky to support
this since it's unclear what things like exits in that update would
mean. This work will still be able to mutate the live DOM but it's just
not eligible to trigger new transitions or adjust the target of those.

One difference between popstate is that this spawned work is after
scroll restoration. So any scrolling spawned from a second pass would
now win over scroll restoration.

Another consequence of this change is that you can't safely animate
pseudo elements in useLayoutEffect. We'll introduce a better event for
that anyway.

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