From b5456ba1b51e9eb2c0788738370ea9e55ee6828f Mon Sep 17 00:00:00 2001 From: Princesseuh Date: Thu, 2 Nov 2023 13:39:21 +0100 Subject: [PATCH 1/5] fix(assets): Propagate `sizes` attribute on all sources --- packages/astro/components/Image.astro | 4 ++-- packages/astro/components/Picture.astro | 21 +++++++++++++++++---- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/packages/astro/components/Image.astro b/packages/astro/components/Image.astro index a11efd4f9769..13d4e2df5117 100644 --- a/packages/astro/components/Image.astro +++ b/packages/astro/components/Image.astro @@ -1,6 +1,7 @@ --- import { getImage, type LocalImageProps, type RemoteImageProps } from 'astro:assets'; import { AstroError, AstroErrorData } from '../dist/core/errors/index.js'; +import { HTMLAttributes } from '../types'; // The TypeScript diagnostic for JSX props uses the last member of the union to suggest props, so it would be better for // LocalImageProps to be last. Unfortunately, when we do this the error messages that remote images get are complete nonsense @@ -24,8 +25,7 @@ if (typeof props.height === 'string') { const image = await getImage(props); -const additionalAttributes: Record = {}; - +const additionalAttributes: HTMLAttributes<'img'> = {}; if (image.srcSet.values.length > 0) { additionalAttributes.srcset = image.srcSet.attribute; } diff --git a/packages/astro/components/Picture.astro b/packages/astro/components/Picture.astro index f6c7686e8790..d4327840a554 100644 --- a/packages/astro/components/Picture.astro +++ b/packages/astro/components/Picture.astro @@ -49,9 +49,16 @@ const fallbackImage = await getImage({ densities: props.densities, }); -const additionalAttributes: Record = {}; +const imgAdditionalAttributes: HTMLAttributes<'img'> = {}; +const sourceAdditionaAttributes: HTMLAttributes<'source'> = {}; + +// Propagate the `sizes` attribute to the `source` elements +if (props.sizes) { + sourceAdditionaAttributes.sizes = props.sizes; +} + if (fallbackImage.srcSet.values.length > 0) { - additionalAttributes.srcset = fallbackImage.srcSet.attribute; + imgAdditionalAttributes.srcset = fallbackImage.srcSet.attribute; } --- @@ -62,8 +69,14 @@ if (fallbackImage.srcSet.values.length > 0) { props.densities || (!props.densities && !props.widths) ? `${image.src}${image.srcSet.values.length > 0 ? ', ' + image.srcSet.attribute : ''}` : image.srcSet.attribute; - return ; + return ( + + ); }) } - + From c60bf2b951bca62a34e9e248b6ae4cb5896a72e8 Mon Sep 17 00:00:00 2001 From: Princesseuh Date: Thu, 2 Nov 2023 15:59:44 +0100 Subject: [PATCH 2/5] refactor: small refactor exposed srcSet types --- packages/astro/src/assets/internal.ts | 2 ++ packages/astro/src/assets/services/service.ts | 12 +++--------- packages/astro/src/assets/types.ts | 15 +++++++++++---- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/packages/astro/src/assets/internal.ts b/packages/astro/src/assets/internal.ts index f7df11f69218..1c26ac6b5dfa 100644 --- a/packages/astro/src/assets/internal.ts +++ b/packages/astro/src/assets/internal.ts @@ -102,6 +102,7 @@ export async function getImage( let imageURL = await service.getURL(validatedOptions, imageConfig); let srcSets: SrcSetValue[] = await Promise.all( srcSetTransforms.map(async (srcSet) => ({ + transform: srcSet.transform, url: await service.getURL(srcSet.transform, imageConfig), descriptor: srcSet.descriptor, attributes: srcSet.attributes, @@ -115,6 +116,7 @@ export async function getImage( ) { imageURL = globalThis.astroAsset.addStaticImage(validatedOptions); srcSets = srcSetTransforms.map((srcSet) => ({ + transform: srcSet.transform, url: globalThis.astroAsset.addStaticImage!(srcSet.transform), descriptor: srcSet.descriptor, attributes: srcSet.attributes, diff --git a/packages/astro/src/assets/services/service.ts b/packages/astro/src/assets/services/service.ts index 7d2f6afb9098..8d77442c7370 100644 --- a/packages/astro/src/assets/services/service.ts +++ b/packages/astro/src/assets/services/service.ts @@ -3,7 +3,7 @@ import { AstroError, AstroErrorData } from '../../core/errors/index.js'; import { isRemotePath, joinPaths } from '../../core/path.js'; import { DEFAULT_OUTPUT_FORMAT, VALID_SUPPORTED_FORMATS } from '../consts.js'; import { isESMImportedImage, isRemoteAllowed } from '../internal.js'; -import type { ImageOutputFormat, ImageTransform } from '../types.js'; +import type { ImageOutputFormat, ImageTransform, UnresolvedSrcSetValue } from '../types.js'; export type ImageService = LocalImageService | ExternalImageService; @@ -28,12 +28,6 @@ type ImageConfig = Omit & { service: { entrypoint: string; config: T }; }; -type SrcSetValue = { - transform: ImageTransform; - descriptor?: string; - attributes?: Record; -}; - interface SharedServiceProps = Record> { /** * Return the URL to the endpoint or URL your images are generated from. @@ -53,7 +47,7 @@ interface SharedServiceProps = Record getSrcSet?: ( options: ImageTransform, imageConfig: ImageConfig - ) => SrcSetValue[] | Promise; + ) => UnresolvedSrcSetValue[] | Promise; /** * Return any additional HTML attributes separate from `src` that your service requires to show the image properly. * @@ -233,7 +227,7 @@ export const baseService: Omit = { }; }, getSrcSet(options) { - const srcSet: SrcSetValue[] = []; + const srcSet: UnresolvedSrcSetValue[] = []; const { targetWidth } = getTargetDimensions(options); const { widths, densities } = options; const targetFormat = options.format ?? DEFAULT_OUTPUT_FORMAT; diff --git a/packages/astro/src/assets/types.ts b/packages/astro/src/assets/types.ts index ec7393e50430..c11f58b25e00 100644 --- a/packages/astro/src/assets/types.ts +++ b/packages/astro/src/assets/types.ts @@ -33,11 +33,18 @@ export interface ImageMetadata { orientation?: number; } -export interface SrcSetValue { - url: string; +/** + * A yet to be completed with an url `SrcSetValue`. Other hooks will only see a resolved value, where the URL of the image has been added. + */ +export type UnresolvedSrcSetValue = { + transform: ImageTransform; descriptor?: string; - attributes?: Record; -} + attributes?: Record; +}; + +export type SrcSetValue = UnresolvedSrcSetValue & { + url: string; +}; /** * A yet to be resolved image transform. Used by `getImage` From 6cc6d89587ade6f304246d90127c347aebd1683f Mon Sep 17 00:00:00 2001 From: Princesseuh Date: Thu, 2 Nov 2023 16:33:42 +0100 Subject: [PATCH 3/5] test: update test with a sizes --- packages/astro/test/core-image.test.js | 3 +++ .../test/fixtures/core-image/src/pages/picturecomponent.astro | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/astro/test/core-image.test.js b/packages/astro/test/core-image.test.js index ade9799186b6..fb7c7c828d16 100644 --- a/packages/astro/test/core-image.test.js +++ b/packages/astro/test/core-image.test.js @@ -222,6 +222,9 @@ describe('astro:image', () => { expect($img).to.have.a.lengthOf(1); expect($picture).to.have.a.lengthOf(1); expect($source).to.have.a.lengthOf(1); + expect($source.attr('sizes')).to.equal( + '(max-width: 448px) 400px, (max-width: 810px) 750px, 1050px' + ); const srcset2 = parseSrcset($source.attr('srcset')); expect(srcset2.every((src) => src.url.startsWith('/_image'))).to.equal(true); diff --git a/packages/astro/test/fixtures/core-image/src/pages/picturecomponent.astro b/packages/astro/test/fixtures/core-image/src/pages/picturecomponent.astro index 713990d86fd8..2fcf4e06c248 100644 --- a/packages/astro/test/fixtures/core-image/src/pages/picturecomponent.astro +++ b/packages/astro/test/fixtures/core-image/src/pages/picturecomponent.astro @@ -8,7 +8,7 @@ import myImage from "../assets/penguin1.jpg";
- +
From 1b953d8a281529d462804c211131d94b563713ca Mon Sep 17 00:00:00 2001 From: Princesseuh Date: Thu, 2 Nov 2023 20:57:59 +0100 Subject: [PATCH 4/5] chore: changeset --- .changeset/tasty-oranges-bathe.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/tasty-oranges-bathe.md diff --git a/.changeset/tasty-oranges-bathe.md b/.changeset/tasty-oranges-bathe.md new file mode 100644 index 000000000000..b155cd4d89c1 --- /dev/null +++ b/.changeset/tasty-oranges-bathe.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fix `sizes` attribute not being present on `source` elements when using it on the Picture component From cbdacfc915eb75595d9be6596af60a994678b8d4 Mon Sep 17 00:00:00 2001 From: Princesseuh Date: Thu, 2 Nov 2023 21:33:43 +0100 Subject: [PATCH 5/5] fix: use a type import --- packages/astro/components/Image.astro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/astro/components/Image.astro b/packages/astro/components/Image.astro index 13d4e2df5117..9c4fcf7e9305 100644 --- a/packages/astro/components/Image.astro +++ b/packages/astro/components/Image.astro @@ -1,7 +1,7 @@ --- import { getImage, type LocalImageProps, type RemoteImageProps } from 'astro:assets'; import { AstroError, AstroErrorData } from '../dist/core/errors/index.js'; -import { HTMLAttributes } from '../types'; +import type { HTMLAttributes } from '../types'; // The TypeScript diagnostic for JSX props uses the last member of the union to suggest props, so it would be better for // LocalImageProps to be last. Unfortunately, when we do this the error messages that remote images get are complete nonsense