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

JSX types for SVG elements (SVGElementTags) wrongly use runtime DOM element types instead of attributes #1905

Closed
ymeine opened this issue Oct 8, 2023 · 2 comments
Labels
bug Something isn't working typescript relating to typescript or types
Milestone

Comments

@ymeine
Copy link

ymeine commented Oct 8, 2023

Describe the bug

As part of an SVG element I was building with solid, I wanted to use an <feDropShadow> element like this:

<feDropShadow in='SourceGraphic' dx='0.2' dy='0.4' stdDeviation='0.2' />

only to find out that TypeScript was complaining about dx and dy having wrong types, expecting an object of the shape SVGAnimatedNumber.

And that's because feDropShadow is defined as Partial<SVGFEDropShadowElement> in interface SVGElementTags, in file solid-js/types/jsx.d.ts

The issue here is that SVGFEDropShadowElement, defined in typescript/lib/lib.dom.d.ts, represents the runtime instance of the created element, not the input attributes type, which should define the inputs dx/dy I mentioned as numbers/strings (for an example, here's how it's defined in React: @types/react/index.d.ts).

Your Example Website or App

https://stackblitz.com/edit/solidjs-templates-xd2mqb?file=src%2FApp.tsx

Steps to Reproduce the Bug or Issue

Just write a solid component with JSX in TypeScript and try defining dx or dy with numbers or strings on a <feDropShadow> element: the tooling (IDE, compiler, etc.) will report an error.

Expected behavior

The type definition should accept numbers/strings and not an SVGAnimatedNumber object. But more importantly, all JSX SVG element types should be defined according to their input attributes, not to their corresponding runtime instance objects.

Screenshots or Videos

image

Platform

  • solid version: 1.7.12

Additional context

No response

@ryansolid ryansolid added typescript relating to typescript or types bug Something isn't working labels Oct 9, 2023
@ryansolid ryansolid added this to the 1.8.0 milestone Oct 9, 2023
ryansolid added a commit that referenced this issue Oct 9, 2023
@ymeine
Copy link
Author

ymeine commented Oct 10, 2023

That was so fast ❤️

By checking how this was resolved, I actually noticed two things:

  • I hadn't even paid attention to the fact that the other elements were properly typed, and that I had just stumbled upon one of the rare occurrences not yet completed - I feel a bit sorry for "explaining" how the typing should be done instead
  • I know now that the typings (and more) are developed in a separate package and repository

Sincere thanks for all the excellent work

@ryansolid
Copy link
Member

Yeah no problem. The separate repo trips people up all the time and I was honestly surprised that slipped by, probably part of a PR I didn't check closely enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working typescript relating to typescript or types
Projects
None yet
Development

No branches or pull requests

2 participants