kit
kit copied to clipboard
fix: clone server data before running universal load on server
fixes #12236
A refactor attempt to serialize the server data before it's passed to the universal load functions.
I'm sure I haven't implemented this correctly:
- I don't think it's correct to await all the server loads, to serialize the server data, and only then run the universal load functions (might slow down when they start and finish).
- I also don't like the code duplication shared between rendering the page and error (and whatever needs to serialise server code in the future).
Maybe someone can take a better shot at this.
EDIT: decided on shallow cloning the server data (see https://github.com/sveltejs/kit/pull/12235#issuecomment-2415821795)
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
- [x] 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
- [x] Run the tests with
pnpm testand lint the project withpnpm lintandpnpm check
Changesets
- [ ] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.
Edits
- [x] Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.
🦋 Changeset detected
Latest commit: d37f5f5d93948e6a5cc728f25d6cf30e0b39d8f7
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 1 package
| Name | Type |
|---|---|
| @sveltejs/kit | 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
I think the simplest fix for this is to shallow clone the object because:
- serialising the server data earlier requires awaiting all the server loads and it also messes up the timing of the chunk streaming (causes one of the tests to fail).
- deep cloning the server data causes errors if there's a promise that's returned by the server load.
Thanks for the exploration, but I'm closing this (for now, we can always reopen later) for the reasons given in https://github.com/sveltejs/kit/issues/12236#issuecomment-2595104925