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

Resolve types for "* .svelte" files from "* .svelte.d.ts" if they exist #878

Closed
trash-and-fire opened this issue Mar 13, 2021 · 6 comments
Closed
Labels
feature request New feature or request Fixed Fixed in master branch. Pending production release.

Comments

@trash-and-fire
Copy link

trash-and-fire commented Mar 13, 2021

Is your feature request related to a problem? Please describe.

I want to publish some svelte modules initially written in typescript and preprocessed into "plain" *.svelte. Also I want to provide typings for those files. It is convenient to place *.svelte.d.ts files near original *.svelte file, but svelte-language-server ignores these definition files.

Describe the solution you'd like

For plain *.svelte file, which doesn't have lang="ts", lang-server should use simple typescript node-module resolving rules as first attempt. It means that if *.svelte.d.ts file exists then it should be used.

Additional context

I made a sample project where we can ensure that two files are checked differently and manually typed *.svelte file is not properly checked. Just clone https://github.com/trash-and-fire/svelte-typings-demo and npm install && npm run validate.

Errors should be the same between usages of typed.svelte and manually-typed.svelte components.

@dummdidumm
Copy link
Member

Standard TS only looks at the file ending and prioritizes d.ts over .js . I think it's even technically impossible for TS to load both and then decide which one to use, because the internal program can only contain one definition for one file, not two. This means that it's not possible for Svelte files to change this behavior by actually looking into the file and determine if it has TS or not. This means we got to decide if we either always prioritize .svelte (current state) or always prioritize .svelte.d.ts . Since this only would really occur in the library case, I'm inclined to always prioritize svelte.d.ts . @jasonlyu123 @tomblachut thoughts?

@jasonlyu123
Copy link
Member

One advantage being there are indeed some cases where it's not easy to type the component in the svelte file. Maybe allowing this would make it easier.

@dummdidumm
Copy link
Member

The disadvantage is that if you rename props inside the svelte.d.ts file, it will not be reflected in the .svelte file. But that's the same behavior for .d.ts / .js files so I'd say this is expected behavior.

@non25
Copy link

non25 commented Mar 13, 2021

There should be documented way to automatically generate .svelte.d.ts from lang="ts" svelte file. 🤔

And that would leave the need to write .svelte.d.ts manually only for non-TS users.

@trash-and-fire
Copy link
Author

I think it's even technically impossible for TS to load both and then decide which one to use, because the internal program can only contain one definition for one file, not two.

I think it is possible but there may be some flaws. My pseudocode:

    function resolveModuleName(
        name: string,
        containingFile: string
    ): ts.ResolvedModule | undefined {
        const bySvelte = resolveUsingSvelteResolver(name, containingFile);
        const byTypescript = resolveUsingTypescriptResolver(name, containingFile);
        
        if (byTypescript && bySvelte) {
              // d.ts is possible
              if (isSveleteTS(name, containingFile, bySvelte)) {  // read this file and try to find lang="ts"
                   return bySvelte; // lang="ts" is more important than d.ts
              } else {
                   return byTypescript; // d.ts is more important plain *.svelte
              }
        }
        ...
    }

I think always prioritize .svelte.d.ts is also a good option and can be easely implemented.

dummdidumm pushed a commit to dummdidumm/language-tools that referenced this issue Mar 17, 2021
Files ending with .svelte.d.ts are now resolved prior to a same file ending with .svelte . This enables library authors to ship type definitions of Svelte files next to their implementation.
sveltejs#878
dummdidumm added a commit that referenced this issue Mar 19, 2021
Files ending with .svelte.d.ts are now resolved prior to a same file ending with .svelte . This enables library authors to ship type definitions of Svelte files next to their implementation.
#878
@dummdidumm dummdidumm added feature request New feature or request Fixed Fixed in master branch. Pending production release. labels Mar 19, 2021
@dummdidumm
Copy link
Member

dummdidumm commented Mar 19, 2021

@firefish5000 FYI, you might be interested in this upcoming change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request Fixed Fixed in master branch. Pending production release.
Projects
None yet
Development

No branches or pull requests

4 participants