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-check and VSCode extension errors regarding component types... How to declare them in Svelte 5? #2522

Closed
charlie-roosen opened this issue Sep 27, 2024 · 11 comments
Labels
Fixed Fixed in master branch. Pending production release. question A user question

Comments

@charlie-roosen
Copy link

charlie-roosen commented Sep 27, 2024

Here is the way that I've been declaring components that I'm binding to a JavaScript reference via "bind:this":

<script lang="ts">
    import CloneWorkflowDialog from './dialogs/CloneWorkflowDialog.svelte';

   let cloneWorkflowDialog : CloneWorkflowDialog;   
</script>

<CloneWorkflowDialog bind:this={cloneWorkflowDialog}/>

With svelte-check 4.0.3, I'm getting new errors that are not flagged by 4.0.2:

c:\cr-bitbucket\RMP Platform Scala\w2e-svelte\svelteui\src\lib\components\workflow\WorkflowTitlePanel.svelte:102:30
Error: 'CloneWorkflowDialog' refers to a value, but is being used as a type here. Did you mean 'typeof CloneWorkflowDialog'? (ts)

   let cloneWorkflowDialog : CloneWorkflowDialog;

This is also being displayed as an error by "Svelte for VS Code" v109.0.2 and not by v108.6.1.

Is this a bug in the language tools, or has the idiom for declaring the types of components change?

@jrmajor
Copy link

jrmajor commented Sep 27, 2024

It's probably related to #2517. I encountered a similar error and tried to add typeof, as suggested in the PR description, but got another error: Type 'SvelteComponent<$$ComponentProps, { [evt: string]: CustomEvent<any>; }, {}> & { $$bindings?: "" | undefined; } & { setPhotoUri: (uri: string, source: string) => void; }' is not assignable to type '__sveltets_2_IsomorphicComponent<$$ComponentProps, { [evt: string]: CustomEvent<any>; }, {}, { setPhotoUri: (uri: string, source: string) => void; }, "">'. In my case both components are using runes.

@wheeOs
Copy link

wheeOs commented Sep 27, 2024

yep, I have the same issue. The VS Code extension complains, the check fails, but the app still runs and compiles

@wheeOs
Copy link

wheeOs commented Sep 27, 2024

I did a rollback to v109.0.1 for now, and keep it so until this gets fixed

@dummdidumm
Copy link
Member

This works correctly. In Svelte 5, components are functions. The types reflect that, so the default export is a function now. To type "I expect an instance of this function", you now do this:

<script lang="ts">
    import CloneWorkflowDialog from './dialogs/CloneWorkflowDialog.svelte';

-   let cloneWorkflowDialog : CloneWorkflowDialog;
+   let cloneWorkflowDialog : ReturnType<typeof CloneWorkflowDialog>;
</script>

<CloneWorkflowDialog bind:this={cloneWorkflowDialog}/>

@dummdidumm dummdidumm added the question A user question label Sep 27, 2024
@bdmackie
Copy link

bdmackie commented Sep 28, 2024

@dummdidumm they've been functions for most of the RC. Is it necessary for it to be that verbose? Sorry if this sounds ranty but this already felt tedious/noisy, especially when triggered by not being able to reference 'this' component, and this makes it even more boilerplatey IMO. At least glad I saw this as I was pulling my hair out wondering why VS code caught on fire with red squiggles after a version bump.

Edit: I've read the related issue so have a vague idea that your hand was forced.... still if I can vote for some kind of ergonomics improvement for this requirement please... maybe even some compiler help?

P.S. I've been getting the 'non reactive state' warning and so to shush it I have been using syntax like this:

let editor1 = $state<Editor | null>(null);

So what you are saying is I need to do something like this:

let editor1 = $state<ReturnType<typeof Editor> | undefined>();

P.P.S. I even messed with Component<> types to try to match the component thinking that would be a cleaner way to do this, but I couldn't get a type definition that was assignable for the binding. I believe that is what the Component<> type is for? I suggest it's an anti-pattern to require the instance of an object to get its type.

Edit: struck out the point about using state as it's incorrect. The last point is what matters.

@ndom91
Copy link

ndom91 commented Oct 2, 2024

@dummdidumm We just came across this issue in our project as well. The updated syntax works and the svelte LSP no longer complains, however svelte-check still doesn't seem to be catching these issues. Meaning that these issues are getting through CI, but then being flagged by the LSP in the editor. So svelte-check probably needs updated as well, right?

@PatrickG
Copy link
Member

PatrickG commented Oct 5, 2024

Is it possible for the language-tools to export the typeof X type as default? So that as a user you don't have to write it.

// virtual/transformed code for `SomeComponent.svelte`
function Component() { ... }
type Component = typeof Component;
export default Component;
<script lang="ts">
  import SomeComponent from './SomeComponent.svelte';

  let instance: ReturnType<SomeComponent>;
  // or with https://github.com/sveltejs/svelte/pull/13441
  let instance: ComponentExports<SomeComponent>;
...

@dummdidumm
Copy link
Member

I briefly thought about that, but the problem with that is that it would no work when you would declare the type by hand. And that could be very confusing ("why can I use the import as a type from a Svelte component, but if I declare a component using the Component type myself, I can't?")

@charlie-roosen
Copy link
Author

Hi everyone and thanks for the feedback.

I've changed my code to use "ReturnType".

I'll go ahead and close the question.

@ptrxyz
Copy link

ptrxyz commented Oct 31, 2024

How does this work with generics? Consider this:

(1) A generic component like this:

<script lang="ts" generics="T extends { foo: string }">
    let { opt }: { opt: T } = $props()

    export function action(): T {
        // do stuff here
        return opt
    }
</script>

(2) Used like so:

<script lang="ts">
    import Comp from './comp.svelte'

    let instance: ReturnType<typeof Comp>

    const opt = {
        foo: 'bar',
        other: 1
    }

    function _callAction() {
        return instance.action().other    // this should not error if instance was properly typed.
    }
</script>

<Comp bind:this={instance} {opt}></Comp>

My problem is that I have no idea how to type instance to properly reflect that opt indeed has the other property. Somehow I have to add the generic parameter to instance to make it work, but I can't wrap my head around this to express it in Typescript.

Can anyone help?

@dummdidumm
Copy link
Member

We're gonna implement the automatic type Foo = ReturnType<typeof Foo> trick within language tools so people don't run into this problem as often.

@dummdidumm dummdidumm reopened this Oct 31, 2024
dummdidumm added a commit that referenced this issue Oct 31, 2024
While it's a gotcha for people declaring their own types, the vast majority of people will use component types via importing other components, and as such it makes sense to provide the same convenience we know from class components

#2522
dummdidumm added a commit that referenced this issue Oct 31, 2024
While it's a gotcha for people declaring their own types, the vast majority of people will use component types via importing other components, and as such it makes sense to provide the same convenience we know from class components

#2522
@dummdidumm dummdidumm added the Fixed Fixed in master branch. Pending production release. label Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed Fixed in master branch. Pending production release. question A user question
Projects
None yet
Development

No branches or pull requests

8 participants