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

Prop value can be different than guaranted by TypeScript #14725

Open
thes01 opened this issue Dec 16, 2024 · 6 comments
Open

Prop value can be different than guaranted by TypeScript #14725

thes01 opened this issue Dec 16, 2024 · 6 comments

Comments

@thes01
Copy link

thes01 commented Dec 16, 2024

Describe the bug

This issue happens in Svelte 5 (both legacy and runes mode), but does not happen in Svelte 4.

Expected logs after clicking the Reset value button in the minimal example REPL:

on destroy
{ name: "John" }

Actual logs (Svelte 5):

on destroy
undefined

Note the TypeScript type shows the prop cannot be undefined.

Even though this would be a feature, it is quite inconvenient that the TypeScript type lies here. I was confused about this during a migration from Svelte 4 after I got a TypeError in runtime JS after accessing an attribute of a prop object that was actually undefined. I tried also the legacy mode, runes/stores and it is everytime the same behaviour in Svelte 5.

Note that in Svelte 4 the output is correct - i.e. the prop is defined even in the onDestroy callback.

Reproduction

https://svelte.dev/playground/dc576c8a0dd94ff095ddb9a6976cfeaa?version=5.14.0

Logs

No response

System Info

System:
    OS: Windows 11 10.0.26100
    CPU: (16) x64 AMD Ryzen 7 PRO 6850U with Radeon Graphics     
    Memory: 1.82 GB / 14.76 GB
  Binaries:
    Node: 22.9.0 - ~\AppData\Local\fnm_multishells\28452_1734370019306\node.EXE
    Yarn: 1.22.22 - ~\AppData\Local\fnm_multishells\28452_1734370019306\yarn.CMD
    npm: 10.9.0 - ~\AppData\Local\fnm_multishells\28452_1734370019306\npm.CMD
  Browsers:
    Edge: Chromium (131.0.2903.99)
    Internet Explorer: 11.0.26100.1882

Severity

annoyance

@webJose
Copy link
Contributor

webJose commented Dec 16, 2024

Most likely same as #14707. Find a detailed explanation about this in that closed issue.

@thes01
Copy link
Author

thes01 commented Dec 16, 2024

I see, thanks. Maybe it could be mentioned in the breaking changes then? 🤔 I didn't see any info about this there.

@jirihon
Copy link

jirihon commented Dec 17, 2024

If it is not a bug, as #14707 suggests, could at least language tools be more useful in this case, if at all possible? A mismatch between type and runtime value will always be tricky to debug and a source of confusion.

@thes01
Copy link
Author

thes01 commented Dec 17, 2024

To me it seems that because the props are rather getters, the typing should be more pessimistic at the parent level.

The only safe solution I can think of is that TS would not narrow the type of the prop in the getter positions:

<script lang="ts">
const thing: string | undefined = $state('foo');
</script>

{#if thing !== undefined}
   <Inner my_prop={thing} /> <!-- thing should be still string | undefined -->
{/if}

Note that TypeScript does an equivalent type-checking here:

https://www.typescriptlang.org/play/?#code/GYVwdgxgLglg9mABBOBbADggpmKAKAcyygC5E8BKRAXgD5EBnKAJxjAKoG8BYAKAEgUYBnAA2WAHSi4BQsUoUA3HwC+fPuKiIAbgENRILGSat2iAD6JwAEyzA2WazUQByABZZR0l8t58YwOR6BliIAITU1FZgtvZgjlx8gmiY8bh4lDT0wYZKqkA

@Leonidaz
Copy link

I think it would be nice to get a warning in the onDestroy or $effect return function (destroy) that would warn that, something like this: prop values may not be equal to the last time they were accessed in this component as the prop getters are defined in the parent component and they may have changed in the parent before this "destroy" callback was invoked.

@dummdidumm
Copy link
Member

We can't solve this at the type level, it would bring way too many false positives. It is inconvenient/surprising though, one could argue even a leak of the underlying signals implementation. Best would be to find a way to fix this.

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

No branches or pull requests

5 participants