From b405b039a6824590e4ad63605f19f0925b4b88ce Mon Sep 17 00:00:00 2001 From: Erika <3019731+Princesseuh@users.noreply.github.com> Date: Mon, 16 Oct 2023 17:43:21 +0200 Subject: [PATCH] fix(assets): Fallback format not being taken into account properly (#8842) Co-authored-by: Florian Lefebvre Co-authored-by: Sarah Rainsberger Co-authored-by: Emanuele Stoppa --- .changeset/dull-bikes-decide.md | 5 ++++ packages/astro/components/Picture.astro | 27 +++++++++++++------ packages/astro/src/assets/services/service.ts | 7 +++++ packages/astro/src/core/errors/errors-data.ts | 15 +++++++++++ packages/astro/test/core-image.test.js | 10 ++++++- .../src/pages/picturecomponent.astro | 4 +++ 6 files changed, 59 insertions(+), 9 deletions(-) create mode 100644 .changeset/dull-bikes-decide.md diff --git a/.changeset/dull-bikes-decide.md b/.changeset/dull-bikes-decide.md new file mode 100644 index 0000000000..cc1fd1f960 --- /dev/null +++ b/.changeset/dull-bikes-decide.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes Picture component not taking into account the fallback format specified diff --git a/packages/astro/components/Picture.astro b/packages/astro/components/Picture.astro index 00907f9d39..1be6371d16 100644 --- a/packages/astro/components/Picture.astro +++ b/packages/astro/components/Picture.astro @@ -11,7 +11,16 @@ type Props = (LocalImageProps | RemoteImageProps) & { pictureAttributes?: HTMLAttributes<'picture'>; }; -const { formats = ['webp'], pictureAttributes = {}, ...props } = Astro.props; +const defaultFormats = ['webp'] as const; +const defaultFallbackFormat = 'png' as const; + +// Certain formats don't want PNG fallbacks: +// - GIF will typically want to stay as a gif, either for animation or for the lower amount of colors +// - SVGs can't be converted to raster formats in most cases +// For those, we'll use the original format as the fallback instead. +const specialFormatsFallback = ['gif', 'svg'] as const; + +const { formats = defaultFormats, pictureAttributes = {}, fallbackFormat, ...props } = Astro.props; if (props.alt === undefined || props.alt === null) { throw new AstroError(AstroErrorData.ImageMissingAlt); @@ -24,16 +33,18 @@ const optimizedImages: GetImageResult[] = await Promise.all( ) ); -const fallbackFormat = - props.fallbackFormat ?? isESMImportedImage(props.src) - ? ['svg', 'gif'].includes(props.src.format) - ? props.src.format - : 'png' - : 'png'; +let resultFallbackFormat = fallbackFormat ?? defaultFallbackFormat; +if ( + !fallbackFormat && + isESMImportedImage(props.src) && + specialFormatsFallback.includes(props.src.format) +) { + resultFallbackFormat = props.src.format; +} const fallbackImage = await getImage({ ...props, - format: fallbackFormat, + format: resultFallbackFormat, widths: props.widths, densities: props.densities, }); diff --git a/packages/astro/src/assets/services/service.ts b/packages/astro/src/assets/services/service.ts index 9812c95c33..455531dfd7 100644 --- a/packages/astro/src/assets/services/service.ts +++ b/packages/astro/src/assets/services/service.ts @@ -198,6 +198,13 @@ export const baseService: Omit = { if (options.src.format === 'svg') { options.format = 'svg'; } + + if ( + (options.src.format === 'svg' && options.format !== 'svg') || + (options.src.format !== 'svg' && options.format === 'svg') + ) { + throw new AstroError(AstroErrorData.UnsupportedImageConversion); + } } // If the user didn't specify a format, we'll default to `webp`. It offers the best ratio of compatibility / quality diff --git a/packages/astro/src/core/errors/errors-data.ts b/packages/astro/src/core/errors/errors-data.ts index 05ccc11259..b3849b150b 100644 --- a/packages/astro/src/core/errors/errors-data.ts +++ b/packages/astro/src/core/errors/errors-data.ts @@ -554,6 +554,21 @@ export const UnsupportedImageFormat = { )} are supported by our image services.`, hint: "Using an `img` tag directly instead of the `Image` component might be what you're looking for.", } satisfies ErrorData; + +/** + * @docs + * @see + * - [Images](https://docs.astro.build/en/guides/images/) + * @description + * Astro does not currently supporting converting between vector (such as SVGs) and raster (such as PNGs and JPEGs) images. + */ +export const UnsupportedImageConversion = { + name: 'UnsupportedImageConversion', + title: 'Unsupported image conversion', + message: + 'Converting between vector (such as SVGs) and raster (such as PNGs and JPEGs) images is not currently supported.', +} satisfies ErrorData; + /** * @docs * @see diff --git a/packages/astro/test/core-image.test.js b/packages/astro/test/core-image.test.js index 93c4fd0232..8e007edb4d 100644 --- a/packages/astro/test/core-image.test.js +++ b/packages/astro/test/core-image.test.js @@ -195,8 +195,16 @@ describe('astro:image', () => { let html = await res.text(); $ = cheerio.load(html); + // Fallback format + let $img = $('#picture-fallback img'); + expect($img).to.have.a.lengthOf(1); + + const imageURL = new URL($img.attr('src'), 'http://localhost'); + expect(imageURL.searchParams.get('f')).to.equal('jpeg'); + expect($img.attr('fallbackformat')).to.be.undefined; + // Densities - let $img = $('#picture-density-2-format img'); + $img = $('#picture-density-2-format img'); let $picture = $('#picture-density-2-format picture'); let $source = $('#picture-density-2-format source'); expect($img).to.have.a.lengthOf(1); 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 a849964bfa..713990d86f 100644 --- a/packages/astro/test/fixtures/core-image/src/pages/picturecomponent.astro +++ b/packages/astro/test/fixtures/core-image/src/pages/picturecomponent.astro @@ -10,3 +10,7 @@ import myImage from "../assets/penguin1.jpg";
+ +
+ +