Fixes type definitions @astrojs/image and adds more documentation to the README
#4045
Conversation
🦋 Changeset detectedLatest commit: 01d18e0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
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
github-actions
bot
added
the
pkg: astro
|
|
||
| // Built-in asset types | ||
| // see `src/constants.ts` | ||
| /// <reference path="./client-base.d.ts" /> | ||
|
|
||
| // images |
There was a problem hiding this comment.
Image types are still included here, using types: ["astro/client"] will get all types from client-base.d.ts plus basic image types
| /// <reference types="vite/types/importMeta" /> | ||
|
|
||
| // CSS modules | ||
| type CSSModuleClasses = { readonly [key: string]: string }; |
There was a problem hiding this comment.
This was the real magic sauce for ESM import types
Moving out all the types except images into a client-base.d.ts allows the main use case of astro/client to work. The image integration now uses only this base file, avoiding the issue of duplicating *.jpg module definitions
|
|
||
| export default integration; | ||
| export { getImage } from './lib/get-image.js'; |
There was a problem hiding this comment.
Turns out moving the actual integration out of index.ts was sometimes confusing the editor when working with astro.config.mjs files
tony-sull
changed the title
@astrojs/image and adds more documentation to the README@astrojs/image and adds more documentation to the README
There was a problem hiding this comment.
A few comments there and there, but nothing blocking. LGTM!
Ultimately, if astro add gets the possiblity of doing tsconfig changes, it'd be cool if it added the types for you as I can see users missing this step fairly easily.
Thank you for improving the editor experience 🥰
There was a problem hiding this comment.
I had one issue following this guide, but this may be due to me doing something wrong (see below).
Also I had one more idea, what I would like to see: The responsive image section should point out how to use this to support higher resolution screens (retina) which look better with 2x/3x scaling.
| ```astro | ||
| --- | ||
| import { getImage } from '@astrojs/image'; | ||
|
|
||
| const { src } = await getImage('../assets/hero.png'); | ||
| --- |
There was a problem hiding this comment.
This does not work for me. Maybe I configured something incorrectly, but I tried to follow the README as it was stated here. Any idea why this happens? (same for getPicture)
'getImage' is not exported by node_modules/@astrojs/image/components/index.ts, imported by src/components/Picture.astro
file: /Users/fidge123/dev/private/project/src/components/Picture.astro:1:3
1: ---
^
2: import { getImage } from '@astrojs/image';
error 'getImage' is not exported by node_modules/@astrojs/image/components/index.ts, imported by src/components/Picture.astro
File:
/Users/fidge123/dev/private/project/src/components/Picture.astro
Code:
1: ---
^
2: import { getImage } from '@astrojs/image';
error Command failed with exit code 1.
There was a problem hiding this comment.
getImage and getPicture aren't currently public, this PR will make them available in the next release 👍
The responsive image section should point out how to use this to support higher resolution screens (retina) which look better with 2x/3x scaling.
Definitely agree we can build up some really nice content for best practices here! I've actually been leaning towards writing a few blog posts that dives deeper into responsive images and how to use these components, the README could link to those for full details
There was a problem hiding this comment.
Thank you for the clarification! I'm looking forward to those blog posts 😊
Changes
types.d.tsfor integration typingsgetImage()andgetPicture()helpers publicly<Image />and<Picture />component propertiesgetImage()andgetPicture()ESM Import Types 🪄
<Image />component prop errorsComponent prop auto-complete
Testing
All existing tests should pass
Docs
integration README updated