Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Svelte 5: structuredClone tries to clone proxy object instead of its contents #13562

Open
Daw588 opened this issue Oct 10, 2024 · 15 comments · May be fixed by #14599
Open

Svelte 5: structuredClone tries to clone proxy object instead of its contents #13562

Daw588 opened this issue Oct 10, 2024 · 15 comments · May be fixed by #14599
Milestone

Comments

@Daw588
Copy link

Daw588 commented Oct 10, 2024

Describe the bug

Attempting to use structuredClone on any stateful object will result in an error with zero indication of a problem before running the code in both JavaScript and TypeScript.

Reproduction

https://svelte-5-preview.vercel.app/#H4sIAAAAAAAACm2QwUrEQAyGXyVEobtQ2nu3LojiwYvHPTgeujPpOjqblJmMq5S-uwwLHoq35OP7_0BmHH2ghN3rjDycCTu8nyasUX-msqQvCkpYY5IcbSF9stFPujdsNJBCThThDm6TDkqbuWCjpaoDg8_yzvAoZLDwZbszXAYrnCRQE-S0KfntDtoWDhI_E4ye6T-nKZ1rEYYEFwphHUgas9UcyT0EYboeuYafBh_IgQrQN9msBNXKrkAYqoNnJ5eqg5v-5fhBVvdgJQcHLApHAltU1xju27-PYI1ncX705LDTmGl5W34BHhXdL2IBAAA=

Logs

No response

System Info

System:
    OS: Windows 11 10.0.22631
    CPU: (20) x64 12th Gen Intel(R) Core(TM) i7-12700KF
    Memory: 48.76 GB / 63.85 GB
  Binaries:
    Node: 20.17.0 - C:\Program Files\nodejs\node.EXE
    npm: 10.8.2 - C:\Program Files\nodejs\npm.CMD
    pnpm: 9.3.0 - ~\AppData\Local\pnpm\pnpm.EXE
    bun: 1.1.30 - ~\.bun\bin\bun.EXE
  Browsers:
    Edge: Chromium (127.0.2651.74)
  npmPackages:
    svelte: ^5.0.0-next.264 => 5.0.0-next.264

Severity

annoyance

@Daw588 Daw588 changed the title structuredClone tries to clone proxy object instead of its contents Svelte 5: structuredClone tries to clone proxy object instead of its contents Oct 10, 2024
@Conduitry
Copy link
Member

This is what https://svelte-5-preview.vercel.app/docs/runes#$state-snapshot is for. I don't know whether it makes sense to patch structuredClone in dev to print a warning about this. There's going to be a never-ending list of libraries and APIs that don't expect proxies.

@Daw588
Copy link
Author

Daw588 commented Oct 10, 2024

I'm aware of that, but there is zero indication of a problem in code editor.

Code_V0pzgiunKW

Svelte lies to TypeScript that $state returns the value it holds, but in reality it returns the value wrapped in a proxy. New developers won't be aware of this, and tools like TypeScript will never know. Developer can easily introduce a bug in their code and the tools that suppose to catch it are unable to because they are being lied to.

Code_XVmSHVKp2G

I don't know what will Svelte team do regarding this issue, but this definitely should be fixed one way or another. I would suggest a fix but I am by no means qualified to talk about tool design (especially given the scale of Svelte).

@FoHoOV
Copy link
Contributor

FoHoOV commented Oct 10, 2024

I would really love the addition of Boxed<T>.

@trueadm
Copy link
Contributor

trueadm commented Oct 10, 2024

We can probably monkey patch structuredClone in DEV and warn to use $state.snapshot if encountering a Svelte proxied object. However, that can wait till 5.x as this isn't urgent.

@trueadm trueadm added this to the 5.x milestone Oct 10, 2024
@FoHoOV
Copy link
Contributor

FoHoOV commented Oct 11, 2024

Just one more thing I wanna say here, If a function returns an object that is something like:

{
  name: string
}

Just by seeing the type, you have no idea if this is reactive or not, You have no idea that if you do, const somehting = $derived(x.name + " something"), this is actually reactive or not. In most cases you have to check the implementation(or docs) to see if its a signal or just a normal property/getter.

@trueadm
Copy link
Contributor

trueadm commented Oct 11, 2024

Just one more thing I wanna say here, If a function returns an object that is something like:

{
  name: string
}

Just by seeing the type, you have no idea if this is reactive or not, You have no idea that if you do, const somehting = $derived(x.name + " something"), this is actually reactive or not. In most cases you have to check the implementation(or docs) to see if its a signal or just a normal property/getter.

Not much we can do about this with today's tooling. The same issue applies with any proxy reactivity library.

@FoHoOV
Copy link
Contributor

FoHoOV commented Oct 11, 2024

If the return type of signals were Boxed<T>, which is just an alias for T wouldn't it help (in type level I have some idea what I'm dealing with)? Also it wouldn't break current code if I'm correct.

@trueadm
Copy link
Contributor

trueadm commented Oct 11, 2024

@FoHoOV No, that wouldn't help at all. Boxing things doesn't solve anything – it just moves the problem to another area and in this case doesn't solve the problem of proxies. Unless you expected each property of the object/array to also be boxed – but that's terrible ergonomics.

@FoHoOV
Copy link
Contributor

FoHoOV commented Oct 11, 2024

I must be using the wrong terms here, Boxed<T> is just an alias for T. They are basically the same but have different semantics. All siganls should be returning Boxed<T> (again just an alias for T ie Boxed<T> = T)

@brunnerh
Copy link
Member

You would need to add a property or symbol to the type, otherwise the wrapper type gets erased. And if you do that, you are no longer allowed to assign to the variable, because the added property will be missing from the new value.

const stateKey = Symbol('state-key');
type StateProxy<T> = T & { [stateKey]: unknown };

declare function $state(value: number): number;
declare function $state(value: string): string;
// ...
declare function $state<T>(value: T): StateProxy<T>;

let a = $state(5); // number
let b = $state({ value: 3 }); // StateProxy<{ value: number }>

b = { value: 5 }; // Error

Playground

@7nik
Copy link

7nik commented Oct 11, 2024

You need to make the symbol optional { [stateKey]?: unknown } but the real problem is that you cannot rid of the marker:

let a =  $state(5); // StateProxy<number & { [stateKey]?: unknown }>
let b = a; // b will have the same reactive type while, in fact, it is static

@brunnerh
Copy link
Member

brunnerh commented Oct 11, 2024

you cannot rid of the marker

That is fine, it's a statement about the object, not the variable.
(And the type of primitives will not be affected.)

@7nik
Copy link

7nik commented Oct 11, 2024

It should be recursive (it's possible) but only for POJOs. In TS, a class instance and a POJO with the same fields are the same thing, aren't they?

@trueadm
Copy link
Contributor

trueadm commented Oct 11, 2024

It should be recursive (it's possible) but only for POJOs. In TS, a class instance and a POJO with the same fields are the same thing, aren't they?

Their prototypes also have to be the same.

@7nik
Copy link

7nik commented Oct 11, 2024

A mean this case which results in a wrong type

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants