-
-
Notifications
You must be signed in to change notification settings - Fork 144
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
feat!: add Svelte5 specific types #381
Conversation
Run & review this pull request in StackBlitz Codeflow. |
This PR should be breaking, we need to redirect We also need:
We don't use that formatting, enable eslint or try running svelte5.d.tsdeclare module 'virtual:icons/*' {
import { Component } from 'svelte'
import type { SvelteHTMLElements } from 'svelte/elements'
export default Component<SvelteHTMLElements['svg']>
}
declare module '~icons/*' {
import { Component } from 'svelte'
import type { SvelteHTMLElements } from 'svelte/elements'
export default Component<SvelteHTMLElements['svg']>
} |
At the point of filing the PR Svelte5 was still in RC so I didn't update the default but happy to make that change now that it is released. Ref linting, sorry thought I'd run eslint but apparently not. |
Yeah, I just merged Svelte 5 support a few minutes ago in main branch (I mean the PR about adding svelte 5 in supported versions) |
14f77ab
to
deb1908
Compare
I force pushed so that my comment is in line with conventional commit messages. The examples look to be working correctly so remain unchanged |
Add |
The example will require to update Svelte and some other dependencies (SvelteKit for example). We also need to update |
"@sveltejs/kit": "^2.7.2",
"@sveltejs/vite-plugin-svelte": "^4.0.0",
"svelte": "^5.0.3",
"svelte-check": "^4.0.5", |
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/@sveltejs/[email protected], npm/@sveltejs/[email protected] |
I think I've got all the dependencies and updated to svelte5 runes in the example. Having a little trouble testing because |
Maybe we should add empty |
I got the workspace examples working, as you noted it was the missing pnpm-lock - no idea what my pnpm install was doing! |
Both examples seem to be working now. I had a bit of a mess with my pnpm install. I cleared out all of the |
pnpm doing the same on my local (lock file unchanged) |
/cc @dominikg @benmccann can you review this 🙏 when you have some free time? |
@mattdavis90 can you check svelte compiler? maybe we can also remove the |
I'm still getting a TS error if I remove that annotation. I haven't added |
We need to review svelte 5 dts, seems to be not working, there is no auto completion in vscode, svg attrs should be there. |
I can't find a better type for it. |
|
Its now working for you? Did you change something? I still get no completion in nvim |
declare module 'virtual:icons/*' {
import { Component } from 'svelte'
import type { SvelteHTMLElements } from 'svelte/elements'
type Type<K> = {
[P in keyof K]: K[P]
}
const component: Component<Type<SvelteHTMLElements['svg']>>
export default component
}
declare module '~icons/*' {
import { Component } from 'svelte'
import type { SvelteHTMLElements } from 'svelte/elements'
type Type<K> = {
[P in keyof K]: K[P]
}
const component: Component<Type<SvelteHTMLElements['svg']>>
export default component
} |
Remove the type, just split the component + default export: declare module 'virtual:icons/*' {
import { Component } from 'svelte'
import type { SvelteHTMLElements } from 'svelte/elements'
const component: Component<SvelteHTMLElements['svg']>
export default component
}
declare module '~icons/*' {
import { Component } from 'svelte'
import type { SvelteHTMLElements } from 'svelte/elements'
const component: Component<SvelteHTMLElements['svg']>
export default component
} |
Cool - you totally beat me to it - this is what I was experimenting with. I hadn't got the declare module "virtual:icons/*" {
import type { Component } from "svelte";
import type { SVGAttributes } from "svelte/elements";
const c: Component<SVGAttributes<SVGSVGElement>>;
export = c;
} |
This is TS: |
Neither of your solutions give me completions in nvim but if they're working in VSCode them I'm happy to put it down to a client issue |
Change svelte5.d.ts: next time we should check any d.ts at root ;) |
Did you restarted ts service? |
Yeh, it works if I do |
It is working in a new project - looks like it is just broken in my local where I was overriding the types in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx ❤️
Thanks for the help :) |
I just noticed in #382 that svelte is added as a peer dependency so maybe you're right and we can drop the |
That's why I was asking you ;) We can even dispatch click events: /cc @dominikg @benmccann |
Is this stuck now? |
@mattdavis90 can you run again On my local I have this entry modified: |
Lock file fixed and I've enabled edit from maintainers - apologies |
So what is the idiomatic type? Is it |
Yes, but you should be able to follow the README and put the following in /// <reference types="svelte" />
/// <reference types="vite/client" />
/// <reference types="unplugin-icons/types/svelte" /> or the following in import 'unplugin-icons/types/svelte' |
Description
When using the library with Svelte5 I noticed that I was getting errors when importing the icon components. This is because Svelte5 changed the type of components so I couldn't assign the icon components to a
Component
typed variable in typescript.This PR updates the type exports to be in line with Svelte5 and removed all issues from my LSP and eslint.
Linked Issues
None that I could find
Additional context
None - happy to discuss further - I didn't open an issue first because it is a pretty trivial change and no real time lost if it isn't accepted. Thanks