fix(assets): Fallback format not being taken into account properly (#…

…8842) Co-authored-by: Florian Lefebvre <[email protected]> Co-authored-by: Sarah Rainsberger <[email protected]> Co-authored-by: Emanuele Stoppa <[email protected]>
withastro · Oct 16, 2023 · b405b03 · b405b03
1 parent 84d7c14
commit b405b03
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 9 deletions.
diff --git a/.changeset/dull-bikes-decide.md 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
@@ -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
@@ -198,6 +198,13 @@ export const baseService: Omit<LocalImageService, 'transform'> = {
 			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
@@ -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
@@ -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
@@ -10,3 +10,7 @@ import myImage from "../assets/penguin1.jpg";
 <div id="picture-widths">
 <Picture src={myImage} width={Math.round(myImage.width / 2)} alt="A penguin" widths={[myImage.width]} />
 </div>
+
+<div id="picture-fallback">
+<Picture src={myImage} fallbackFormat="jpeg" alt="A penguin" />
+</div>