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

Type scrictness is broken because default values are only initial values #1167

Closed
macmillen opened this issue Sep 6, 2021 · 3 comments
Closed
Labels
bug Something isn't working

Comments

@macmillen
Copy link

Describe the bug
Due to the fact that default values in Svelte components only behave like initial values there is a problem with the types.

Please check the following example:

Within the component the type is string.

image

But outside of the component the type is string | undefined.

image

This works for the first time the component is initialized but not for all the successive parameter changes. This causes the components to not have strict types which makes the strict option in the compiler useless.

Also even if this is the desired behavior for JavaScript components as soon as you use TypeScript it becomes a huge problem because you can't trust your component types anymore.

Expected behavior
I think there are two ways to address this issue:

  • Either make the default values behave like JavaScript functions, where every passed undefined value is replaced by the default value
  • OR fix the type inside the component and show the correct one like title: string | undefined

I would prefer the first solution because this is probably your initial thought of how the default values work. If you come from a web framework like React it will make more sense to you.

This issue is related to sveltejs/svelte#5673

System (please complete the following information):

  • OS: Linux
  • IDE: VSCode
  • Plugin/Package: Svelte for VSCode
@macmillen macmillen added the bug Something isn't working label Sep 6, 2021
@jasonlyu123
Copy link
Member

I would think the props in the svelte component as a class constructor injected property with a separated setter. When it's updating, it uses the setter instead of re-run the class constructor. Something like this, but not exactly how it's implemented:

class Component {
  constructor({  someProps = '' }) {
     this.someProps = someProps;
  }
  
  $set({ someProps }) {
     this.someProps = someProps;
  }
}

So I don't think an optional property in javascript is a good analogy. it may be confusing to a react developer but it is more of a behavior difference instead of a bug at this point. Changing this "behavior" is not really a viable solution with the reason mentioned in the issue you linked. Also, the solution here sveltejs/svelte#5673 (comment) would cover the typing issue. If there's is no default value we'll mark the props as not nullable. Then you can enforce props type strictness.

@macmillen
Copy link
Author

@jasonlyu123 thanks for the quick answer. I understand the class analogy but I think with the current situation we still have a bug because x = "default" results in x: string and not x: string | undefined inside the component.

The workaround from sveltejs/svelte#5673 (comment) fixes the default value situation but not the TypeScript type, unless you apply that workaround to every prop with a default value which again can't be the solution.

@dummdidumm
Copy link
Member

dummdidumm commented Sep 6, 2021

The undefined issue is a general Typescript inconsistency. Typescript 4.4 introduced a new flag for this which handles this: https://devblogs.microsoft.com/typescript/announcing-typescript-4-4/#exact-optional-property-types
So if you enable that, undefined is no longer a valid input.
That said, the language server does not run on 4.4 yet (see #1158) .
I'm still closing this issue as it's essentially two issues in one, both duplicates of an existing issue. I hear you though that the "initial vs default" behavior might be unintuitive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants