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

update for svelte 4.2 #508

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ryoppippi
Copy link

@ryoppippi ryoppippi commented Oct 29, 2023

Scince 4.2, we can declare 'svelte/elements' for custom attributes.
I use this and this is so good!

https://svelte.jp/docs/typescript#enhancing-built-in-dom-types

@fnimick
Copy link

fnimick commented Oct 29, 2023

@ryoppippi does this mean that modifying the global types for HTMLAttributes is no longer necessary? If so, there should be an indication in the typescript support section of the README that the modification is not necessary if svelte 4.2+ is used.

@isaacHagoel
Copy link
Owner

does this work for versions >=4.0 && < 4.2, based on the title I assume no but the README just says Svelte 4 so it might not work for some. Also since most people simply copy paste it and move on I wonder if it is worth keeping it as is just to save the mental overhead of "wait do I need to copy this or that?" and "wait, I am going to upgrade to 4.2 later, will I need to change this?"

@fnimick
Copy link

fnimick commented Oct 29, 2023

So the reason would be that if the signature of the events from this library changes, or a new event is added in the future, users won't have to manually update their vendored type definitions to keep them in sync.

@ryoppippi
Copy link
Author

Ok, we need to add like require 4.2

@ryoppippi
Copy link
Author

If the HTMLAttribute type could be defined in the library and automatically loaded at import, it would eliminate the need for the user to copy and paste the type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants