Skip to content

Conversation

@Ocean-OS
Copy link
Member

@Ocean-OS Ocean-OS commented Nov 30, 2025

This serializes thenables in hydratable. However, there is a slight discrepancy: if you have an object that is a thenable on the server, it will be converted to a Promise on the client. I'm not sure if we should:

  1. convert thenables to Promises (and maybe reflect that in the types?)
  2. do nothing and close this PR
  3. resolve the thenable on the server, and on the client return an object that has the serialized contents of the thenable (excluding the original then function), with a then function that resolves to the awaited value, which would look like:
const create_thenable = () => ({
  foo: 1,
  bar: 2,
  then(onresolve, onreject) {
    return Promise.resolve(Math.random()).then(onresolve, onreject);
  }
});
const thenable = await hydratable('test', create_thenable);
// on client: Object.assign({ foo: 1, bar: 2 }, { then: (a, b) => r(0.1234).then(a, b) });

Option 3 would probably be the best in terms of correctness, but it'd also be the most complex; we'd have to add some extra errors and checks for values that cannot be serialized (since there isn't a way to omit a property from a non-POJO while preserving its prototype, which would let us make devalue do the checking for us), which would raise the issue of having two types of serialization errors: those from devalue and those from svelte, which would make error handling trickier. Additionally, we'd likely have to preserve key order in thenables, which could get ugly.

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
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.
  • If this PR changes code within packages/svelte/src, add a changeset (npx changeset).

Tests and linting

  • Run the tests with pnpm test and lint the project with pnpm lint

@changeset-bot
Copy link

changeset-bot bot commented Nov 30, 2025

🦋 Changeset detected

Latest commit: ac8e098

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte Patch

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

@github-actions
Copy link
Contributor

Playground

pnpm add https://pkg.pr.new/svelte@17275

@Rich-Harris
Copy link
Member

Is this related to #17235? My feeling is that we should handle cross-realm promises, but otherwise ignore anything funky — serializing non-promise thenables feels no different than serializing any other non-built-in type

@Ocean-OS
Copy link
Member Author

Ocean-OS commented Dec 1, 2025

I hadn't seen #17235 before, but I do agree with you. Is there a way to check if a value is a cross-realm Promise other than Object.prototype.toString, or is that the best we could do?

@Rich-Harris
Copy link
Member

Not that I'm aware of. But the toString check is in line with what devalue does for other things (Date, Set, etc) so I think it's fine. Just merged #17284

@Ocean-OS
Copy link
Member Author

Ocean-OS commented Dec 1, 2025

Closing in favor of #17284

@Ocean-OS Ocean-OS closed this Dec 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants