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

feat!: remove as prop in favor of svelte:element #1628

Closed
wants to merge 3 commits into from

Conversation

metonym
Copy link
Collaborator

@metonym metonym commented Jan 15, 2023

This was developed initially to address #1605. It removes the as prop, which was added before svelte:element was supported in Svelte. The purpose is to allow the consumer to customize the element tag.

However, this should not be merged since it has a flaw.

The issue is that $$restProps are spread to the element; the tag attributes in the generated types should be generic to match the tag. This is a limitation of sveld, which does not support generics in props.

Perhaps another reason to switch to TypeScript + @sveltejs/package (#1616)

@theetrain
Copy link
Collaborator

Thanks for exploring this. Usage of $$restProps seems essential in order for users to apply standard attributes to a component's canonical element. I'm also exploring svelte-package in #1629 and results look promising so far.

Since $$props and $$restProps are discouraged, maybe we should use a custom prop such as buttonAttributes for <Button /> and apply its respective HTML element type to that prop. At least it'll look more proper, can be documented, and hopefully can be optimized in the final Svelte output.

@metonym
Copy link
Collaborator Author

metonym commented Jan 16, 2023

maybe we should use a custom prop such as buttonAttributes for <Button /> and apply its respective HTML element type to that prop. At least it'll look more proper, can be documented, and hopefully can be optimized in the final Svelte output.

That's a very elegant solution. +1

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.

2 participants