-
Notifications
You must be signed in to change notification settings - Fork 47k
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
Move Hydration Warnings from the DOM Config into the Fiber reconciliation #28476
Merged
sebmarkbage
merged 9 commits into
facebook:main
from
sebmarkbage:hydrationwarningsinfiber
Mar 26, 2024
Merged
Move Hydration Warnings from the DOM Config into the Fiber reconciliation #28476
sebmarkbage
merged 9 commits into
facebook:main
from
sebmarkbage:hydrationwarningsinfiber
Mar 26, 2024
Conversation
This file contains 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
facebook-github-bot
added
CLA Signed
React Core Team
Opened by a member of the React Core Team
labels
Mar 1, 2024
Comparing: 84c84d7...21f7e9d 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 |
sebmarkbage
force-pushed
the
hydrationwarningsinfiber
branch
4 times, most recently
from
March 1, 2024 20:25
75422d8
to
6acaed9
Compare
gnoff
approved these changes
Mar 2, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a test is still not updated for the new warning strings
sebmarkbage
force-pushed
the
hydrationwarningsinfiber
branch
5 times, most recently
from
March 4, 2024 21:45
abbdca3
to
89ac64c
Compare
sebmarkbage
force-pushed
the
hydrationwarningsinfiber
branch
from
March 26, 2024 21:54
767592f
to
2a38b43
Compare
… of html form in diffs
To do this we need to describe any nodes that don't have a match to diff against.
This unifies hydrateTextInstance and hydrateInstance so that they both throw on text mismatches in the same way in prod. They also get the diff before throwing for DEV.
sebmarkbage
force-pushed
the
hydrationwarningsinfiber
branch
from
March 26, 2024 21:55
2a38b43
to
4a22791
Compare
github-actions bot
pushed a commit
that referenced
this pull request
Mar 26, 2024
…tion (#28476) Stacked on #28458. This doesn't actually really change the messages yet, it's just a refactor. Hydration warnings can be presented either as HTML or React JSX format. If presented as HTML it makes more sense to make that a DOM specific concept, however, I think it's actually better to present it in terms of React JSX. Most of the time the errors aren't going to be something messing with them at the HTML/HTTP layer. It's because the JS code does something different. Most of the time you're working in just React. People don't necessarily even know what the HTML form of it looks like. So this takes the approach that the warnings are presented in React JSX in their rich object form. Therefore, I'm moving the approach to yield diff data to the reconciler but it's the reconciler that's actually printing all the warnings. DiffTrain build for [4b8dfd6](4b8dfd6)
kassens
added a commit
to kassens/react
that referenced
this pull request
Mar 26, 2024
Back out "Move Hydration Warnings from the DOM Config into the Fiber reconciliation (facebook#28476)" Original commit changeset: 4b8dfd6 Back out "Remove enableClientRenderFallbackOnTextMismatch flag (facebook#28458)" Original commit changeset: 84c84d7
sebmarkbage
added a commit
that referenced
this pull request
Mar 27, 2024
Stacked on #28476. We used to `console.error` for every mismatch we found, up until the error we threw for the hydration mismatch. This changes it so that we build up a set of diffs up until we either throw or complete hydrating the root/suspense boundary. If we throw, we append the diff to the error message which gets passed to onRecoverableError (which by default is also logged to console). If we complete, we append it to a `console.error`. Since we early abort when something throws, it effectively means that we can only collect multiple diffs if there were preceding non-throwing mismatches - i.e. only properties mismatched but tag name matched. There can still be multiple logs if multiple siblings Suspense boundaries all error hydrating but then they're separate errors entirely. We still log an extra line about something erroring but I think the goal should be that it leads to a single recoverable or console.error. This doesn't yet actually print the diff as part of this message. That's in a follow up PR.
github-actions bot
pushed a commit
that referenced
this pull request
Mar 27, 2024
Stacked on #28476. We used to `console.error` for every mismatch we found, up until the error we threw for the hydration mismatch. This changes it so that we build up a set of diffs up until we either throw or complete hydrating the root/suspense boundary. If we throw, we append the diff to the error message which gets passed to onRecoverableError (which by default is also logged to console). If we complete, we append it to a `console.error`. Since we early abort when something throws, it effectively means that we can only collect multiple diffs if there were preceding non-throwing mismatches - i.e. only properties mismatched but tag name matched. There can still be multiple logs if multiple siblings Suspense boundaries all error hydrating but then they're separate errors entirely. We still log an extra line about something erroring but I think the goal should be that it leads to a single recoverable or console.error. This doesn't yet actually print the diff as part of this message. That's in a follow up PR. DiffTrain build for [f7aa5e0](f7aa5e0)
EdisonVan
pushed a commit
to EdisonVan/react
that referenced
this pull request
Apr 15, 2024
…tion (facebook#28476) Stacked on facebook#28458. This doesn't actually really change the messages yet, it's just a refactor. Hydration warnings can be presented either as HTML or React JSX format. If presented as HTML it makes more sense to make that a DOM specific concept, however, I think it's actually better to present it in terms of React JSX. Most of the time the errors aren't going to be something messing with them at the HTML/HTTP layer. It's because the JS code does something different. Most of the time you're working in just React. People don't necessarily even know what the HTML form of it looks like. So this takes the approach that the warnings are presented in React JSX in their rich object form. Therefore, I'm moving the approach to yield diff data to the reconciler but it's the reconciler that's actually printing all the warnings.
EdisonVan
pushed a commit
to EdisonVan/react
that referenced
this pull request
Apr 15, 2024
…ok#28502) Stacked on facebook#28476. We used to `console.error` for every mismatch we found, up until the error we threw for the hydration mismatch. This changes it so that we build up a set of diffs up until we either throw or complete hydrating the root/suspense boundary. If we throw, we append the diff to the error message which gets passed to onRecoverableError (which by default is also logged to console). If we complete, we append it to a `console.error`. Since we early abort when something throws, it effectively means that we can only collect multiple diffs if there were preceding non-throwing mismatches - i.e. only properties mismatched but tag name matched. There can still be multiple logs if multiple siblings Suspense boundaries all error hydrating but then they're separate errors entirely. We still log an extra line about something erroring but I think the goal should be that it leads to a single recoverable or console.error. This doesn't yet actually print the diff as part of this message. That's in a follow up PR.
bigfootjon
pushed a commit
that referenced
this pull request
Apr 18, 2024
…tion (#28476) Stacked on #28458. This doesn't actually really change the messages yet, it's just a refactor. Hydration warnings can be presented either as HTML or React JSX format. If presented as HTML it makes more sense to make that a DOM specific concept, however, I think it's actually better to present it in terms of React JSX. Most of the time the errors aren't going to be something messing with them at the HTML/HTTP layer. It's because the JS code does something different. Most of the time you're working in just React. People don't necessarily even know what the HTML form of it looks like. So this takes the approach that the warnings are presented in React JSX in their rich object form. Therefore, I'm moving the approach to yield diff data to the reconciler but it's the reconciler that's actually printing all the warnings. DiffTrain build for commit 4b8dfd6.
bigfootjon
pushed a commit
that referenced
this pull request
Apr 18, 2024
Stacked on #28476. We used to `console.error` for every mismatch we found, up until the error we threw for the hydration mismatch. This changes it so that we build up a set of diffs up until we either throw or complete hydrating the root/suspense boundary. If we throw, we append the diff to the error message which gets passed to onRecoverableError (which by default is also logged to console). If we complete, we append it to a `console.error`. Since we early abort when something throws, it effectively means that we can only collect multiple diffs if there were preceding non-throwing mismatches - i.e. only properties mismatched but tag name matched. There can still be multiple logs if multiple siblings Suspense boundaries all error hydrating but then they're separate errors entirely. We still log an extra line about something erroring but I think the goal should be that it leads to a single recoverable or console.error. This doesn't yet actually print the diff as part of this message. That's in a follow up PR. DiffTrain build for commit f7aa5e0.
This was referenced Apr 24, 2024
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.
Stacked on #28458.
This doesn't actually really change the messages yet, it's just a refactor.
Hydration warnings can be presented either as HTML or React JSX format. If presented as HTML it makes more sense to make that a DOM specific concept, however, I think it's actually better to present it in terms of React JSX.
Most of the time the errors aren't going to be something messing with them at the HTML/HTTP layer. It's because the JS code does something different. Most of the time you're working in just React. People don't necessarily even know what the HTML form of it looks like. So this takes the approach that the warnings are presented in React JSX in their rich object form.
Therefore, I'm moving the approach to yield diff data to the reconciler but it's the reconciler that's actually printing all the warnings.