mutating the `data` prop in a universal load affects the data returned from a server load

[teemingc]

Describe the bug

I came across an issue on Discord where someone tried using dynamic import() to load a component and pass it to data in the universal load:

export async function load({ data }) {
  data.component = (await import('$lib/components/Button.svelte')).default
  return data
  //     ^ error
}

This threw an error along the lines of "cannot return non-serializable POJOs" since data was being returned by the server load in +page.server.js. Hopefully my contribution to the docs helps point out this lesser-known nuance.

https://github.com/sveltejs/kit/pull/11822#issue-2128689538

Reproduction

https://stackblitz.com/edit/sveltejs-kit-template-default-qnnygo?file=src%2Froutes%2F%2Bpage.js

Logs

No response

System Info

StackBlitz

Severity

annoyance

Additional Information

We could:

1. Keep the current behaviour where data is mutated.
2. Prevent mutations of data from affecting the serialization of the server load data.
3. Throw an error telling users not to mutate data.

[dummdidumm]

I don't think we should change anything here. Mutation is always something to be wary of. And the referenced PR would only solve this for top level entries but not nested ones which may further the confusion.

[teemingc]

Based on https://github.com/sveltejs/kit/issues/13835 we should probably throw a better error message here (and document this?)

[glyndwr-io]

Another thing I was thinking about last night, what about wrapping the data object in a proxy in dev mode in order to warn on mutation? Possibly even suggest the use of structuredClone()? Direct mutation of data is easy to avoid but there are many situations where you could run into this as a footgun as I did if you're not thinking about how JS works under the hood. Here's an example:

export async function load({ data }) {
  const { info } = data
  
  /**
   * This could even be obfuscated further behind a function call
   * or done without the user's knowledge by a library
   **/
  info.extra = new Map()

  const final = { info, a: 'b' }
  return final
  //     ^ error
}

It's totally valid to attribute this to the perils of mutation, but I think given both Svelte's intent and SvelteKit's mental model of how loading and data functions work, you wouldn't expect mutation to have an effect like this at the framework level (at least I didn't). A better error or even warning message would really help set clear expectations. Perhaps even a warning disclaimer in the docs as well?