-
-
Notifications
You must be signed in to change notification settings - Fork 578
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
Expose StarlightIcon
type
#2805
Conversation
🦋 Changeset detectedLatest commit: e8634e3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
✅ Deploy Preview for astro-starlight ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
I like that the documentation is changed this way. Also nice catch with the changes from Icon to Badge 👌 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this all looks great and very thorough — I had no idea how often we used keyof typeof Icons
😅 — thank you @HiDeoo!
Had one microscopic query but this PR looks good to go!
@@ -10,6 +10,19 @@ Starlight provides a set of built-in icons that you can display in your content | |||
Icons can be displayed using the [`<Icon>`](/components/icons/) component. | |||
They are also often used in other components, such as [cards](/components/cards/) or settings like [hero actions](/reference/frontmatter/#hero). | |||
|
|||
## Icon type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this maybe be this? Not 100% sure, but “Icon type” suggests overlap with the component itself. I also considered a heading that used “name” somehow, but didn’t come up with a wording I was satisfied with.
## Icon type | |
## `StarlightIcon` type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call, just updated the heading and its associated links 👍
Co-authored-by: Chris Swithinbank <[email protected]>
Oops, should have checked first that this was a patch — I thought it was 😅 I think it’s OK if I make it one? I know it’s technically a “feature”, but do you think there’s a risk in releasing it in a patch @HiDeoo? |
Definitely, go ahead, I went by the definition of semver but I cannot see a risk so I think it should be fine. |
Done in bb8e631 — apologies for not being more attentive. |
Description
As discussed on Discord, this PR exposes a new TypeScript type for built-in icon names:
StarlightIcon
. This type is a union of all built-in icon names.When building an Astro component accepting an icon name, you could already use the
ComponentProps<typeof Icon>['name']
approach, but importing the Astro component in a TypeScript file, which could also use ESLint with TypeScript type-aware rules, is not trivial.A common use case is for example a plugin defining its configuration through a Zod schema where an icon name is required. This can now be validated as a string but still provide autocompletion and type checking for the end user using
icon: z.string() as z.Schema<StarlightIcon>
.Regarding the documentation change, I think I did the maximum number of changes so it's easy to remove some of them if there are not judged required or good.