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

Wrong svelte/elements types loaded #2109

Closed
dummdidumm opened this issue Jul 27, 2023 · 7 comments
Closed

Wrong svelte/elements types loaded #2109

dummdidumm opened this issue Jul 27, 2023 · 7 comments
Labels
bug Something isn't working Fixed Fixed in master branch. Pending production release.

Comments

@dummdidumm
Copy link
Member

Describe the bug

When svelte-jsx(-v4).dts does import('svelte/elements') TypeScript resolves this to the Svelte typings of the language tools extension. But it should resolve it to the user's implementation instead.

Reproduction

Replace the svelte:options entry in svelte-jsx-v4.d.ts with 'svelte:options': import('svelte/elements').SvelteHTMLElements['svelte:options'];, then observe that in intellisense it still does not provide any type hints. Then click on the import and see that it directs you to the language tools' version of Svelte.

Expected behaviour

Should use user's implementation

System Info

irrelevant

Which package is the issue about?

svelte-language-server

Additional Information, eg. Screenshots

@jasonlyu123 do you have any idea how we could make this work? At runtime we have importPackage, but we don't have that here.

@dummdidumm dummdidumm added the bug Something isn't working label Jul 27, 2023
@jasonlyu123
Copy link
Member

Tough one. That also seems to be why we can't use declare module "svelte/elements" to enhance typing. The only solution I can think of is a virtual module that replaces the "svelte/elements" with a full file path to the user version. Maybe an actual file we just hide in the node_modules or a virtual file that only exists in the memory. It might also work if we use the same method we remove the "*.svelte" module declaration, but not sure if that work if monorepo.

@dummdidumm
Copy link
Member Author

Another not-so-great option could be to rename the Svelte package when building language tools for the VS Code extension. But that means it does not work for others using it through the LSP.. I think your idea of a hidden file or a virtual file is the most promising solution.

@dummdidumm
Copy link
Member Author

Another idea is to turn it around: Place this ambient file inside Svelte itself. This is basically what React etc do. This could have the added benefit that SvelteKit could enhance the typings independently, right now we have a forced relationship between all these parts, which this would resolve. For SvelteKit we could even spin it further and move the "these exports exist" logic currently within the TypeScript plugin and within svelte2tsx into SvelteKit by having some generic extension interface with language tools.
The one tough problem is how to ensure backwards compatibility so that it works the old way when we detect a Svelte version that doesn't have the typings yet.
What do you think about this approach?

@jasonlyu123
Copy link
Member

Yeah. That seems like the ideal solution. For backwards compatibility, we can keep a copy of it and only load that if we can't detect a supported version. We can manually copy it or use a script to download the latest from the Svelte repo. And after we bump our bundled version to Svelte 4, we just manually load bundled version if not supported.

@dummdidumm
Copy link
Member Author

Sounds great! Would you be willing to work on this or are you too busy the next few weeks?

@jasonlyu123
Copy link
Member

I can try it. BTW, Do you have any solution with #2003? It would probably be easier if we could fix it before moving it to the svelte core. The problem is that TypeScript can't infer the parameter type from the union.

@dummdidumm
Copy link
Member Author

Looked into it, no idea right now how to best solve this - shouldn't block us though, it's an edge case and if we come up with a solution we can adjust both the Svelte typings and our fallback types.

@dummdidumm dummdidumm added the Fixed Fixed in master branch. Pending production release. label Aug 11, 2023
dummdidumm added a commit that referenced this issue Nov 6, 2024
Since the global types now reference `svelte/...` imports, they need to be placed in the same context the users' Svelte package lives in. Else these imports would load the types of the Svelte version that comes bundled with the IDE.

#2493

(side note: we had this problem before in #2109 when loading `svelte/elements`, but that solution is not applicable here)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Fixed Fixed in master branch. Pending production release.
Projects
None yet
Development

No branches or pull requests

2 participants