feat: dedupe serialized hydratable values#18406
Open
Rich-Harris wants to merge 8 commits into
Open
Conversation
🦋 Changeset detectedLatest commit: 6f84293 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
Rich-Harris
changed the title
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
Member
Author
|
(It turns out this isn't sufficient to dedupe |
…ng warnings due to missing `return` statement in catch block
This commit fixes the issue reported at packages/svelte/src/internal/server/hydratable.js:97
## Bug Analysis
### What's happening
The `compare` function in `hydratable.js` is designed to check whether `hydratable()` is called twice with the same key but different values. It tries to serialize both values and compare them:
```javascript
try {
if ((await serialize(a.value)) === (await serialize(b.value))) {
return; // Values are equal, no error
}
} catch {
// disregard any errors that happen during serialization,
// they will be dealt with separately
// BUG: No return here!
}
// This code runs for BOTH "values differ" AND "serialization failed"
e.hydratable_clobbering(key, stack); // Called incorrectly for serialization errors
```
### The problem
When `serialize()` throws an error:
1. The catch block executes
2. Code falls through to `e.hydratable_clobbering()`
3. A false "clobbering" error is reported
The comment says errors "will be dealt with separately" - and indeed they are! The `renderer.js` file (around line 870) has proper handling using `e.hydratable_serialization_failed()` for serialization errors. The `compare` function should simply return and not interfere.
### Impact
Users would see confusing "Attempted to set `hydratable` with key ... twice with different values" errors when the actual issue is that the value is not serializable. This makes debugging harder since the error message is misleading.
### The fix
Add `return;` inside the catch block so that serialization errors cause the function to exit cleanly without reporting a clobbering error:
```javascript
} catch {
// disregard any errors that happen during serialization,
// they will be dealt with separately
return;
}
```
Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com>
Co-authored-by: Rich-Harris <hello@rich-harris.dev>
6 tasks
Rich-Harris
added a commit
to sveltejs/kit
that referenced
this pull request
Jun 11, 2026
I thought that sveltejs/svelte#18406 would be necessary to do this, but I was wrong — it actually seems easier and simpler to bypass `hydratable` altogether. Instead, we serialize all the remote data in one go, allowing devalue to do its thing and deduplicate everything. (I still think it's worth merging that PR.) That way, if you have (for example) a `getUser(): Promise<User | null>` and a `requireUser(): Promise<User>` that calls `getUser` internally (and redirects if it returns `null`), the `User` object is only serialized once. I can't remember exactly why we chose to use `hydratable` in the first place. Perhaps it was a lifecycle thing — we wanted to be careful about data being stale by the time it was read? In which case I think that has changed now that query lifecycle is determined by the garbage collector. Maybe @elliott-with-the-longest-name-on-github remembers. --- ### Please don't delete this checklist! Before submitting the PR, please make sure you do the following: - [ ] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs - [x] This message body should clearly illustrate what problems it solves. - [x] Ideally, include a test that fails without this PR but passes with it. ### Tests - [ ] Run the tests with `pnpm test` and lint the project with `pnpm lint` and `pnpm check` ### Changesets - [x] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running `pnpm changeset` and following the prompts. Changesets that add features should be `minor` and those that fix bugs should be `patch`. Please prefix changeset messages with `feat:`, `fix:`, or `chore:`. ### Edits - [x] Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed. --------- Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com> Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com> Co-authored-by: Elliott Johnson <hello@ell.iott.dev>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
If two hydratables share a reference to a single object, that object is serialized twice in the SSR'd output. For example if you have a
getUserquery and arequireUserquery that callsgetUser(and throws a redirect ifnull), then you might end up with something like this:If we
devalue.unevalall the entries together, we avoid this.TODO
Before submitting the PR, please make sure you do the following
feat:,fix:,chore:, ordocs:.packages/svelte/src, add a changeset (npx changeset).Tests and linting
pnpm testand lint the project withpnpm lint