Refactor Props of Image and Picture component to support type checking
#5788
Conversation
🦋 Changeset detectedLatest commit: c245aca 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
the
pkg: integration
MoustaphaDev
changed the title
Props of Image and Picture component for better type checkingProps of Image and Picture component to support type checking
|
|
||
| interface LocalImageProps | ||
| extends Omit<TransformOptions, 'src'>, |
There was a problem hiding this comment.
For some reason, when generics are used directly inside the astro component, its props type become Record<string, any>. When extracted to another file, it works as expected
There was a problem hiding this comment.
Our language server doesn't support generics at the moment, but it will soon. Your workaround seems valid, though!
cc @Princesseuh for a TS review 😄
| @@ -59,6 +32,7 @@ const { image, sources } = await getPicture({ | |||
| fit, | |||
| background, | |||
| position, | |||
| alt, | |||
There was a problem hiding this comment.
Ommiting alt was a mistake I guess?
|
|
||
| export interface ImageComponentLocalImageProps | ||
| extends Omit<TransformOptions, 'src'>, | ||
| Omit<ImgHTMLAttributes, 'src' | 'width' | 'height'> { |
There was a problem hiding this comment.
Should be astroHTML.JSX.ImgHTMLAttributes too?
There was a problem hiding this comment.
Maybe HTMLAttributes<'img'> from astro/types?
There was a problem hiding this comment.
Maybe
HTMLAttributes<'img'>fromastro/types?
Sounds good to me!
There was a problem hiding this comment.
Should be
astroHTML.JSX.ImgHTMLAttributestoo?
Probably not? I don't think we want astro directives here
|
Left a few comments! Let me know if you'd rather I push this PR over the line myself, as you've done plenty work already! |
Co-authored-by: Erika <3019731+Princesseuh@users.noreply.github.com>
Thanks! Your suggestions where helpful enough, I just need approval now 🙂 |
Changes
ImageandPicturecomponents.altprop to thegetPicturefunctionTesting
Tested locally. There's only a small runtime change (see
altprop passed to thegetPicturefunction). All tests should passDocs
N/A