feat(types): add SvgComponent type and update SVG module declaration #13651
Conversation
🦋 Changeset detectedLatest commit: bddf04c The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
pkg: astro
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Should this be
But we can change it to |
| @@ -1,3 +1,4 @@ | |||
| import type * as astroHTML from 'astro/jsx-runtime'; | |||
There was a problem hiding this comment.
@Princesseuh do you think this import could have weird TS implications? IIRC its always a bit tricky to import this one (at least in userland)
There was a problem hiding this comment.
Can we import the type from the actual file @florian-lefebvre ?
There was a problem hiding this comment.
That's what I'm looking at atm but it's tricky because it's only in a namespace inside a dts
There was a problem hiding this comment.
So one way is to do import '../../astro-jsx.d.ts'; (or using a triple slash reference) but then it overrides other type definitions, eg.
error TS5055: Cannot write file '/home/florian/Documents/github/withastro/astro/packages/astro/dist/transitions/events.d.ts' because it would overwrite input file.
error TS5055: Cannot write file '/home/florian/Documents/github/withastro/astro/packages/astro/dist/transitions/types.d.ts' because it would overwrite input file.
error TS5055: Cannot write file '/home/florian/Documents/github/withastro/astro/packages/astro/dist/type-utils.d.ts' because it would overwrite input file.
error TS5055: Cannot write file '/home/florian/Documents/github/withastro/astro/packages/astro/dist/types/public/elements.d.ts' because it would overwrite input file.
error TS5055: Cannot write file '/home/florian/Documents/github/withastro/astro/packages/astro/dist/types/public/view-transitions.d.ts' because it would overwrite input file.
There was a problem hiding this comment.
instead of exporting the type from here, personally I'd export it directly in client.d.ts, where it's trivial to import from astro-jsx without bundling issues
There was a problem hiding this comment.
We want the type to be used in *.svg but also to be exported from "astro". Would that be doable in client.d.ts without making the type global?
There was a problem hiding this comment.
ah in which case I think maybe it's fine that way
There was a problem hiding this comment.
@Princesseuh the way I did it here won't work, as it causes build errors. (I confirmed that main branch builds successfully.)
I have tried a few different things including "export it directly in client.d.ts" but nothing is working.
What exactly did you mean by "it's trivial to import from astro-jsx without bundling issues"?
|
I think this is good as a minor thanks! |
|
Changes done. @florian-lefebvre Note: I didn't use GitHub UI to accept your suggestions, but rather just replicated them in my local and made a commit to ensure there are no syntax errors and it has proper formatting. :) |
|
This is an expermental feature, so it can be a patch |
|
@ascorbic As per v5.7.0 release notes, the SVG feature is no longer experimental. Can you clarify again whether this change should be |
|
🤦♂️ You're right of course. In my defence is been a long week with lots of features |
|
@ADTC maintainers are responsible for documenting the features they implement. This means that you will have send a PR to Also, the Build is falling, so we can't merge it until this is fixed. |
|
Yes I know the build is failing but I don't know how to fix it. 😢 I was hoping @Princesseuh might have some insight. |
|
I'm not sure, sorry. It has been too much time since I've set up client.d.ts and all of that to remember the exact rules that make TypeScript happy. In retrospect, maybe astro-jsx.d.ts shouldn't have lived "outside" of the package, it's wonky, but I set that up 3 years ago now 😅 |
Changes
SvgComponenttype as per discussion here.Testing
No tests needed, I think?
Docs
Docs will need to mention that the
SvgComponentcomponent is available for use, similar to the mentions of other types likeImageMetadata./cc @withastro/maintainers-docs for feedback!