[Fiber] Replay onChange Events if input/textarea/select has changed before hydration #33129
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
React Core Team
sebmarkbage
force-pushed
the
hydrationchangeevent
branch
from
15345d5 to
d8209c7
Compare
sebmarkbage
force-pushed
the
hydrationchangeevent
branch
from
d8209c7 to
17c14e6
Compare
sebmarkbage
force-pushed
the
hydrationchangeevent
branch
from
17c14e6 to
7b3fbec
Compare
|
Comparing: 79586c7...0812b1f Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show |
This is like commitMount but for hydration. Allows us to do work in the commit phase. We need to separate Update from Hydrate so I reuse the Callback flag for this.
Just prepares to allow for the next commit.
hydrateInput/Select/Textarea is like initInput/Select/Textarea except we don't actually set any defaultValue or value or name etc. we assume that they're what we expected just like any attribute hydration in prod. If the value has changed by the time we commit, we should track the value that we last observed. Any new value should trigger an onChange so the initial tracked value should be what the server rendered which we assume was the same thing we got from the hydrating props.
sebmarkbage
force-pushed
the
hydrationchangeevent
branch
from
7b3fbec to
6b4d976
Compare
…ation We do this by simulating a real DOM event so that custom listeners get to observe it as well.
sebmarkbage
force-pushed
the
hydrationchangeevent
branch
from
6b4d976 to
0812b1f
Compare
rickhanlonii
approved these changes
May 6, 2025
| await act(async () => { | ||
| setUntrackedChecked.call(b, true); | ||
| dispatchEventOnNode(b, 'click'); | ||
| }); | ||
| assertLog(['click b']); | ||
| if (gate(flags => flags.enableHydrationChangeEvent)) { | ||
| // Since we already had this selected, this doesn't trigger a change again. |
| expect(changeCount).toBe(0); | ||
| expect(changeCount).toBe( | ||
| gate(flags => flags.enableHydrationChangeEvent) ? 1 : 0, | ||
| ); |
There was a problem hiding this comment.
Lots of nice test comments fixed, sick
| // eslint-disable-next-line jest/no-disabled-tests | ||
| it.skip('should not blow away user-entered text on successful reconnect to an uncontrolled textarea', () => | ||
| // @gate enableHydrationChangeEvent | ||
| it('should not blow away user-entered text on successful reconnect to an uncontrolled textarea', () => |
sebmarkbage
commented
May 6, 2025
| hydrateTextarea(domElement, props.value, props.defaultValue); | ||
| break; | ||
| case 'img': | ||
| // TODO: Should we replay onLoad events? |
There was a problem hiding this comment.
@gnoff One thing this enables us to do now is replay the onLoad event for images if they have already loaded before hydration happens. Should we?
github-actions bot
pushed a commit
that referenced
this pull request
May 6, 2025
…efore hydration (#33129) This fixes a long standing issue that controlled inputs gets out of sync with the browser state if it's changed before we hydrate. This resolves the issue by replaying the change events (click, input and change) if the value has changed by the time we commit the hydration. That way you can reflect the new value in state to bring it in sync. It does this whether controlled or uncontrolled. The idea is that this should be ok to replay because it's similar to the continuous events in that it doesn't replay a sequence but only reflects the current state of the tree. Since this is a breaking change I added it behind `enableHydrationChangeEvent` flag. There is still an additional issue remaining that I intend to address in a follow up. If a `useLayoutEffect` triggers an sync rerender on hydration (always a bad idea) then that can rerender before we have had a chance to replay the change events. If that renders through a input then that input will always override the browser value with the controlled value. Which will reset it before we've had a change to update to the new value. DiffTrain build for [587cb8f](587cb8f)
github-actions bot
pushed a commit
that referenced
this pull request
May 6, 2025
…efore hydration (#33129) This fixes a long standing issue that controlled inputs gets out of sync with the browser state if it's changed before we hydrate. This resolves the issue by replaying the change events (click, input and change) if the value has changed by the time we commit the hydration. That way you can reflect the new value in state to bring it in sync. It does this whether controlled or uncontrolled. The idea is that this should be ok to replay because it's similar to the continuous events in that it doesn't replay a sequence but only reflects the current state of the tree. Since this is a breaking change I added it behind `enableHydrationChangeEvent` flag. There is still an additional issue remaining that I intend to address in a follow up. If a `useLayoutEffect` triggers an sync rerender on hydration (always a bad idea) then that can rerender before we have had a chance to replay the change events. If that renders through a input then that input will always override the browser value with the controlled value. Which will reset it before we've had a change to update to the new value. DiffTrain build for [587cb8f](587cb8f)
sebmarkbage
added 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.
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)
This was referenced May 6, 2025
This was referenced May 13, 2025
This was referenced May 21, 2025
16 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes a long standing issue that controlled inputs gets out of sync with the browser state if it's changed before we hydrate.
This resolves the issue by replaying the change events (click, input and change) if the value has changed by the time we commit the hydration. That way you can reflect the new value in state to bring it in sync. It does this whether controlled or uncontrolled.
The idea is that this should be ok to replay because it's similar to the continuous events in that it doesn't replay a sequence but only reflects the current state of the tree.
Since this is a breaking change I added it behind
enableHydrationChangeEventflag.There is still an additional issue remaining that I intend to address in a follow up. If a
useLayoutEffecttriggers an sync rerender on hydration (always a bad idea) then that can rerender before we have had a chance to replay the change events. If that renders through a input then that input will always override the browser value with the controlled value. Which will reset it before we've had a change to update to the new value.