Skip to content

[Fiber] Replay events between commits #33130

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 3 commits into from
May 6, 2025

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented May 6, 2025

Stacked on #33129. Flagged behind enableHydrationChangeEvent.

If you type into a controlled input before hydration and something else rerenders like a setState in an effect, then the controlled input will reset to whatever React thought it was. Even with event replaying that this is stacked on, if the second render happens before event replaying has fired in a separate task.

We don't want to flush inside the commit phase because then things like flushSync in these events wouldn't work since they're inside the commit stack.

This flushes all event replaying between renders by flushing it at the end of flushSpawned work. We've already committed at that point and is about to either do subsequent renders or yield to event loop for passive effects which could have these events fired anyway. This just ensures that they've already happened by the time subsequent renders fire. This means that there's now a type of event that fire between sync render passes.

@react-sizebot
Copy link

react-sizebot commented May 6, 2025

Comparing: 587cb8f...a37d64d

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 = 528.29 kB 528.29 kB = 93.14 kB 93.15 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 = 639.00 kB 638.84 kB +0.06% 112.19 kB 112.26 kB
facebook-www/ReactDOM-prod.classic.js = 671.70 kB 671.70 kB = 117.79 kB 117.79 kB
facebook-www/ReactDOM-prod.modern.js = 661.98 kB 661.98 kB = 116.23 kB 116.23 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against a37d64d

Copy link
Member

@rickhanlonii rickhanlonii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also dope

Outside the Commit execution context and before passive effects.
@sebmarkbage sebmarkbage force-pushed the hydrationchangeevent2 branch from 5581122 to a37d64d Compare May 6, 2025 04:13
@sebmarkbage sebmarkbage merged commit 54a5072 into facebook:main May 6, 2025
239 checks passed
github-actions bot pushed a commit that referenced this pull request May 6, 2025
Stacked on #33129. Flagged behind `enableHydrationChangeEvent`.

If you type into a controlled input before hydration and something else
rerenders like a setState in an effect, then the controlled input will
reset to whatever React thought it was. Even with event replaying that
this is stacked on, if the second render happens before event replaying
has fired in a separate task.

We don't want to flush inside the commit phase because then things like
flushSync in these events wouldn't work since they're inside the commit
stack.

This flushes all event replaying between renders by flushing it at the
end of `flushSpawned` work. We've already committed at that point and is
about to either do subsequent renders or yield to event loop for passive
effects which could have these events fired anyway. This just ensures
that they've already happened by the time subsequent renders fire. This
means that there's now a type of event that fire between sync render
passes.

DiffTrain build for [54a5072](54a5072)
github-actions bot pushed a commit that referenced this pull request May 6, 2025
Stacked on #33129. Flagged behind `enableHydrationChangeEvent`.

If you type into a controlled input before hydration and something else
rerenders like a setState in an effect, then the controlled input will
reset to whatever React thought it was. Even with event replaying that
this is stacked on, if the second render happens before event replaying
has fired in a separate task.

We don't want to flush inside the commit phase because then things like
flushSync in these events wouldn't work since they're inside the commit
stack.

This flushes all event replaying between renders by flushing it at the
end of `flushSpawned` work. We've already committed at that point and is
about to either do subsequent renders or yield to event loop for passive
effects which could have these events fired anyway. This just ensures
that they've already happened by the time subsequent renders fire. This
means that there's now a type of event that fire between sync render
passes.

DiffTrain build for [54a5072](54a5072)
github-actions bot pushed a commit to code/lib-react that referenced this pull request May 6, 2025
Stacked on facebook#33129. Flagged behind `enableHydrationChangeEvent`.

If you type into a controlled input before hydration and something else
rerenders like a setState in an effect, then the controlled input will
reset to whatever React thought it was. Even with event replaying that
this is stacked on, if the second render happens before event replaying
has fired in a separate task.

We don't want to flush inside the commit phase because then things like
flushSync in these events wouldn't work since they're inside the commit
stack.

This flushes all event replaying between renders by flushing it at the
end of `flushSpawned` work. We've already committed at that point and is
about to either do subsequent renders or yield to event loop for passive
effects which could have these events fired anyway. This just ensures
that they've already happened by the time subsequent renders fire. This
means that there's now a type of event that fire between sync render
passes.

DiffTrain build for [54a5072](facebook@54a5072)
github-actions bot pushed a commit to code/lib-react that referenced this pull request May 6, 2025
Stacked on facebook#33129. Flagged behind `enableHydrationChangeEvent`.

If you type into a controlled input before hydration and something else
rerenders like a setState in an effect, then the controlled input will
reset to whatever React thought it was. Even with event replaying that
this is stacked on, if the second render happens before event replaying
has fired in a separate task.

We don't want to flush inside the commit phase because then things like
flushSync in these events wouldn't work since they're inside the commit
stack.

This flushes all event replaying between renders by flushing it at the
end of `flushSpawned` work. We've already committed at that point and is
about to either do subsequent renders or yield to event loop for passive
effects which could have these events fired anyway. This just ensures
that they've already happened by the time subsequent renders fire. This
means that there's now a type of event that fire between sync render
passes.

DiffTrain build for [54a5072](facebook@54a5072)
Copy link

@Hardanish-Singh Hardanish-Singh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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