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

Fix IntrinsicAttributes error in TypeScript definitions #304

Closed
metonym opened this issue Oct 6, 2020 · 5 comments
Closed

Fix IntrinsicAttributes error in TypeScript definitions #304

metonym opened this issue Oct 6, 2020 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@metonym
Copy link
Collaborator

metonym commented Oct 6, 2020

This error has appeared several times now: #253, #300, #303


Component props should be typed as the intersection of the semantic element props and props exported from the component.

Furthermore, potentially adding { [key: string]: any; } can serve as a catch-all for custom component attributes. A common use case would be data-{X} selectors used for testing/analytics.

For example, the following should not throw a TS error:

<Link data-test style="color: red;" href="#">Link</Link>

The data-test and style attributes are not explicitly defined in the Link component, but they should still be valid.

@metonym metonym added the bug Something isn't working label Oct 6, 2020
@metonym metonym self-assigned this Oct 6, 2020
@metonym
Copy link
Collaborator Author

metonym commented Oct 24, 2020

If the "Strongly Typed Svelte Component Interface" RFC is accepted and implemented in the language tools, writing component library definitions should be a lot easier. Attempting to auto-generate definitions may work on the surface for props/slots but is untenable in the long run. Especially since it's not the official way, it's prone to breakage if the internal API changes.

@semenodp
Copy link

semenodp commented Nov 3, 2020

Hello. Is there any update on this issue? Would be happy to help adding { [key: string]: any; } if that's the solution you're taking.

@metonym
Copy link
Collaborator Author

metonym commented Nov 3, 2020

@semenodp I'm rewriting the types, but I don't want this to block anybody in the meantime. I'd welcome a PR of adding the { [key: string]: any } for each component's props as a short term fix in the generation script would be appreciated.

@metonym
Copy link
Collaborator Author

metonym commented Nov 3, 2020

@semenodp I've uploaded what I currently have in PR #385

A component props definition must extend its semantic HTML attributes ONLY if $$restProps are forwarded to a native HTML element.

class Tag {
  $$prop_def: svelte.JSX.HTMLAttributes<HTMLElementTagNameMap["span"]> & {
    // ...
  }
}

As a result, the following should work (no errors from the VS Code Svelte Language Server extension or from svelte-check).

<!-- the "class", "style" attributes should not cause an error -->
<Tag
  class="class-name"
  style="color: red;"
  type="red"
  on:click="{(e) => {
    console.log(e); // MouseEvent
  }}"
>
  text
</Tag>

<TagSkeleton class="class-name" style="color: red;" />

Currently, the "class", "style" attributes will cause an error, and the event from on:click would be "Event" instead of the more specific "MouseEvent."

@metonym
Copy link
Collaborator Author

metonym commented Nov 21, 2020

@semenodp The rewritten TypeScript definitions are released in v0.23.

Notable changes

  • account for intrinsic element attributes if $$restProps is spread to a native HTML Element (this issue)
  • accurately represent the shape of the library so direct imports work
  • improve typings for prop default values by deferring to the Svelte compiler instead of JSDocs
  • improve typings for forwarded/dispatched events
  • improve typings for slots, slot props

Tests that validate the TS definitions are located in the tests folder. There should be no errors when running svelte-check or from the official Svelte VS Code language server extension.


Also, for anyone reading this in the future:

I walk back what I said about manually maintaining type definitions. They should be auto-generated along with the COMPONENT_INDEX.md and COMPONENT_API.json

@metonym metonym closed this as completed Nov 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants