Move Hydration Mismatch Errors to Throw or Log Once (Kind of) #28502
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
|
Comparing: 4b8dfd6...ec5901d 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
hydrationerrors
branch
from
0d181dc to
aa03948
Compare
gaearon
reviewed
Mar 6, 2024
We collect diffs up until the point we throw to abort. If we collect only dev-only diffs without throwing we log them once the boundary completes.
… tests Previously we had a console.error + throw when something threw. We ignored the throw in tests. Now we only have the throw.
sebmarkbage
force-pushed
the
hydrationerrors
branch
from
dd76e5c to
fb32aff
Compare
sebmarkbage
force-pushed
the
hydrationerrors
branch
from
3f02366 to
ec5901d
Compare
|
Stamped downstream. |
sebmarkbage
added a commit
that referenced
this pull request
Mar 27, 2024
Stacked on #28502. This builds on the mechanism in #28502 by adding a diff of everything we've collected so far to the thrown error or logged error. This isn't actually a longest common subsequence diff. This means that there are certain cases that can appear confusing such as a node being added/removed when it really would've appeared later in the list. In fact once a node mismatches, we abort rendering so we don't have the context of what would've been rendered. It's not quite right to use the result of the recovery render because it can use client-only code paths using useSyncExternalStore which would yield false differences. That's why diffing the HTML isn't quite right. I also present abstract components in the stack, these are presented with the client props and no diff since we don't have the props that were on the server. The lack of difference might be confusing but it's useful for context. The main thing that's data new here is that we're adding some siblings and props for context. Examples in the [snapshot commit](e14532f).
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)
github-actions bot
pushed a commit
that referenced
this pull request
Mar 27, 2024
Stacked on #28502. This builds on the mechanism in #28502 by adding a diff of everything we've collected so far to the thrown error or logged error. This isn't actually a longest common subsequence diff. This means that there are certain cases that can appear confusing such as a node being added/removed when it really would've appeared later in the list. In fact once a node mismatches, we abort rendering so we don't have the context of what would've been rendered. It's not quite right to use the result of the recovery render because it can use client-only code paths using useSyncExternalStore which would yield false differences. That's why diffing the HTML isn't quite right. I also present abstract components in the stack, these are presented with the client props and no diff since we don't have the props that were on the server. The lack of difference might be confusing but it's useful for context. The main thing that's data new here is that we're adding some siblings and props for context. Examples in the [snapshot commit](e14532f). DiffTrain build for [2ec2aae](2ec2aae)
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.
EdisonVan
pushed a commit
to EdisonVan/react
that referenced
this pull request
Apr 15, 2024
Stacked on facebook#28502. This builds on the mechanism in facebook#28502 by adding a diff of everything we've collected so far to the thrown error or logged error. This isn't actually a longest common subsequence diff. This means that there are certain cases that can appear confusing such as a node being added/removed when it really would've appeared later in the list. In fact once a node mismatches, we abort rendering so we don't have the context of what would've been rendered. It's not quite right to use the result of the recovery render because it can use client-only code paths using useSyncExternalStore which would yield false differences. That's why diffing the HTML isn't quite right. I also present abstract components in the stack, these are presented with the client props and no diff since we don't have the props that were on the server. The lack of difference might be confusing but it's useful for context. The main thing that's data new here is that we're adding some siblings and props for context. Examples in the [snapshot commit](facebook@e14532f).
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.
bigfootjon
pushed a commit
that referenced
this pull request
Apr 18, 2024
Stacked on #28502. This builds on the mechanism in #28502 by adding a diff of everything we've collected so far to the thrown error or logged error. This isn't actually a longest common subsequence diff. This means that there are certain cases that can appear confusing such as a node being added/removed when it really would've appeared later in the list. In fact once a node mismatches, we abort rendering so we don't have the context of what would've been rendered. It's not quite right to use the result of the recovery render because it can use client-only code paths using useSyncExternalStore which would yield false differences. That's why diffing the HTML isn't quite right. I also present abstract components in the stack, these are presented with the client props and no diff since we don't have the props that were on the server. The lack of difference might be confusing but it's useful for context. The main thing that's data new here is that we're adding some siblings and props for context. Examples in the [snapshot commit](e14532f). DiffTrain build for commit 2ec2aae.
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 #28476.
We used to
console.errorfor 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.