fix: image proxy not working correctly (#9659)

* fix: image proxy not working correctly

* fix: only take in valid images

* test: add tests

* Create slimy-mayflies-vanish.md

* nit: remove erika-ism

Author: Princesseuh

.changeset/slimy-mayflies-vanish.md

@@ -0,0 +1,5 @@
+---
+"astro": patch
+---
+
+Fix Astro wrongfully deleting certain images imported with `?url` when used in tandem with `astro:assets`

packages/astro/src/assets/internal.ts

@@ -61,6 +61,10 @@ export async function getImage(
 				: options.src,
 	};
 
+	const originalPath = isESMImportedImage(resolvedOptions.src)
+		? resolvedOptions.src.fsPath
+		: resolvedOptions.src;
+
 	// Clone the `src` object if it's an ESM import so that we don't refer to any properties of the original object
 	// Causing our generate step to think the image is used outside of the image optimization pipeline
 	const clonedSrc = isESMImportedImage(resolvedOptions.src)
@@ -95,10 +99,10 @@ export async function getImage(
 		!(isRemoteImage(validatedOptions.src) && imageURL === validatedOptions.src)
 	) {
 		const propsToHash = service.propertiesToHash ?? DEFAULT_HASH_PROPS;
-		imageURL = globalThis.astroAsset.addStaticImage(validatedOptions, propsToHash);
+		imageURL = globalThis.astroAsset.addStaticImage(validatedOptions, propsToHash, originalPath);
 		srcSets = srcSetTransforms.map((srcSet) => ({
 			transform: srcSet.transform,
-			url: globalThis.astroAsset.addStaticImage!(srcSet.transform, propsToHash),
+			url: globalThis.astroAsset.addStaticImage!(srcSet.transform, propsToHash, originalPath),
 			descriptor: srcSet.descriptor,
 			attributes: srcSet.attributes,
 		}));

packages/astro/src/assets/types.ts

@@ -20,7 +20,9 @@ declare global {
 	// eslint-disable-next-line no-var
 	var astroAsset: {
 		imageService?: ImageService;
-		addStaticImage?: ((options: ImageTransform, hashProperties: string[]) => string) | undefined;
+		addStaticImage?:
+			| ((options: ImageTransform, hashProperties: string[], fsPath: string) => string)
+			| undefined;
 		staticImages?: AssetsGlobalStaticImagesList;
 		referencedImages?: Set<string>;
 	};

packages/astro/src/assets/utils/emitAsset.ts

@@ -33,7 +33,7 @@ export async function emitESMImage(
 	Object.defineProperty(emittedImage, 'fsPath', {
 		enumerable: false,
 		writable: false,
-		value: url,
+		value: id,
 	});
 
 	// Build

packages/astro/src/assets/utils/proxy.ts

@@ -1,11 +1,17 @@
-export function getProxyCode(options: Record<string, any>, isSSR: boolean): string {
+import type { ImageMetadata } from '../types.js';
+
+export function getProxyCode(options: ImageMetadata, isSSR: boolean): string {
+	const stringifiedFSPath = JSON.stringify(options.fsPath);
 	return `
 						new Proxy(${JSON.stringify(options)}, {
 						get(target, name, receiver) {
 							if (name === 'clone') {
 								return structuredClone(target);
 							}
-							${!isSSR ? 'globalThis.astroAsset.referencedImages.add(target.fsPath);' : ''}
+							if (name === 'fsPath') {
+								return ${stringifiedFSPath};
+							}
+							${!isSSR ? `globalThis.astroAsset.referencedImages.add(${stringifiedFSPath});` : ''}
 							return target[name];
 						}
 					})

packages/astro/src/assets/vite-plugin-assets.ts

@@ -19,7 +19,8 @@ import { hashTransform, propsToFilename } from './utils/transformToPath.js';
 
 const resolvedVirtualModuleId = '\0' + VIRTUAL_MODULE_ID;
 
-const assetRegex = new RegExp(`\\.(${VALID_INPUT_FORMATS.join('|')})$`, 'i');
+const assetRegex = new RegExp(`\\.(${VALID_INPUT_FORMATS.join('|')})`, 'i');
+const assetRegexEnds = new RegExp(`\\.(${VALID_INPUT_FORMATS.join('|')})$`, 'i');
 
 export default function assets({
 	settings,
@@ -81,7 +82,7 @@ export default function assets({
 					return;
 				}
 
-				globalThis.astroAsset.addStaticImage = (options, hashProperties) => {
+				globalThis.astroAsset.addStaticImage = (options, hashProperties, originalPath) => {
 					if (!globalThis.astroAsset.staticImages) {
 						globalThis.astroAsset.staticImages = new Map<
 							string,
@@ -97,11 +98,6 @@ export default function assets({
 						isESMImportedImage(options.src) ? options.src.src : options.src
 					).replace(settings.config.build.assetsPrefix || '', '');
 
-					// This, however, is the real original path, in `src` and all.
-					const originalSrcPath = isESMImportedImage(options.src)
-						? options.src.fsPath
-						: options.src;
-
 					const hash = hashTransform(
 						options,
 						settings.config.image.service.entrypoint,
@@ -120,7 +116,7 @@ export default function assets({
 
 						if (!transformsForPath) {
 							globalThis.astroAsset.staticImages.set(finalOriginalImagePath, {
-								originalSrcPath: originalSrcPath,
+								originalSrcPath: originalPath,
 								transforms: new Map(),
 							});
 							transformsForPath = globalThis.astroAsset.staticImages.get(finalOriginalImagePath)!;
@@ -178,15 +174,25 @@ export default function assets({
 				resolvedConfig = viteConfig;
 			},
 			async load(id, options) {
-				// If our import has any query params, we'll let Vite handle it
-				// See https://github.com/withastro/astro/issues/8333
-				if (id !== removeQueryString(id)) {
-					return;
-				}
 				if (assetRegex.test(id)) {
-					const meta = await emitESMImage(id, this.meta.watchMode, this.emitFile);
+					if (!globalThis.astroAsset.referencedImages)
+						globalThis.astroAsset.referencedImages = new Set();
+
+					if (id !== removeQueryString(id)) {
+						// If our import has any query params, we'll let Vite handle it, nonetheless we'll make sure to not delete it
+						// See https://github.com/withastro/astro/issues/8333
+						globalThis.astroAsset.referencedImages.add(removeQueryString(id));
+						return;
+					}
 
-					if (!meta) {
+					// If the requested ID doesn't end with a valid image extension, we'll let Vite handle it
+					if (!assetRegexEnds.test(id)) {
+						return;
+					}
+
+					const imageMetadata = await emitESMImage(id, this.meta.watchMode, this.emitFile);
+
+					if (!imageMetadata) {
 						throw new AstroError({
 							...AstroErrorData.ImageNotFound,
 							message: AstroErrorData.ImageNotFound.message(id),
@@ -197,13 +203,13 @@ export default function assets({
 					// Since you cannot use image optimization on the client anyway, it's safe to assume that if the user imported
 					// an image on the client, it should be present in the final build.
 					if (options?.ssr) {
-						return `export default ${getProxyCode(meta, isServerLikeOutput(settings.config))}`;
+						return `export default ${getProxyCode(
+							imageMetadata,
+							isServerLikeOutput(settings.config)
+						)}`;
 					} else {
-						if (!globalThis.astroAsset.referencedImages)
-							globalThis.astroAsset.referencedImages = new Set();
-
-						globalThis.astroAsset.referencedImages.add(meta.fsPath);
-						return `export default ${JSON.stringify(meta)}`;
+						globalThis.astroAsset.referencedImages.add(imageMetadata.fsPath);
+						return `export default ${JSON.stringify(imageMetadata)}`;
 					}
 				}
 			},

packages/astro/src/content/runtime-assets.ts

@@ -22,7 +22,7 @@ export function createImage(pluginContext: PluginContext, entryFilePath: string)
 				return z.never();
 			}
 
-			return { ...metadata, ASTRO_ASSET: true };
+			return { ...metadata, ASTRO_ASSET: metadata.fsPath };
 		});
 	};
 }

packages/astro/src/content/vite-plugin-content-imports.ts

@@ -364,6 +364,7 @@ function stringifyEntryData(data: Record<string, any>, isSSR: boolean): string {
 			// For Astro assets, add a proxy to track references
 			if (typeof value === 'object' && 'ASTRO_ASSET' in value) {
 				const { ASTRO_ASSET, ...asset } = value;
+				asset.fsPath = ASTRO_ASSET;
 				return getProxyCode(asset, isSSR);
 			}
 		});

packages/astro/test/fixtures/core-image-deletion/package.json

@@ -0,0 +1,11 @@
+{
+  "name": "@test/core-image-deletion",
+  "version": "0.0.0",
+  "private": true,
+  "dependencies": {
+    "astro": "workspace:*"
+  },
+  "scripts": {
+    "dev": "astro dev"
+  }
+}

packages/astro/test/fixtures/core-image-deletion/public/penguin3.jpg

@@ -0,0 +1,15 @@
+---
+import { Image } from "astro:assets";
+import onlyOne from "../assets/onlyone.jpg";
+import twoOfUs from "../assets/twoofus.jpg";
+import twoFromURL from "../assets/url.jpg";
+import twoFromURL_URL from "../assets/url.jpg?url";
+---
+
+<Image src={onlyOne} alt="Only one of me exists at the end of the build" />
+
+<Image src={twoOfUs} alt="Two of us will exist, because I'm also used as a normal image" /></Image>
+<img src={twoOfUs.src} alt="Two of us will exist, because I'm also used as a normal image" />
+
+<Image src={twoFromURL} alt="Two of us will exist, because I'm also imported using ?url" /></Image>
+<img src={twoFromURL_URL} alt="Two of us will exist, because I'm also used as a normal image" />

packages/astro/test/fixtures/core-image-deletion/src/assets/onlyone.jpg

@@ -0,0 +1,9 @@
+{
+  "extends": "astro/tsconfigs/base",
+  "compilerOptions": {
+    "baseUrl": ".",
+    "paths": {
+      "~/assets/*": ["src/assets/*"]
+    },
+  }
+}

packages/astro/test/fixtures/core-image-deletion/src/assets/twoofus.jpg

@@ -1,5 +1,4 @@
 ---
-import { getImage } from 'astro:assets';
 import { getCollection } from 'astro:content';
 
 export async function getStaticPaths() {
@@ -11,7 +10,6 @@ export async function getStaticPaths() {
 
 const { entry } = Astro.props;
 const { Content } = await entry.render();
-const myImage = await getImage(entry.data.image);
 ---
 <html>
 	<head>

packages/astro/test/fixtures/core-image-deletion/src/assets/url.jpg

@@ -0,0 +1,36 @@
+import { expect } from 'chai';
+import { testImageService } from './test-image-service.js';
+import { loadFixture } from './test-utils.js';
+
+describe('astro:assets - delete images that are unused', () => {
+	/** @type {import('./test-utils.js').Fixture} */
+	let fixture;
+
+	describe('build ssg', () => {
+		before(async () => {
+			fixture = await loadFixture({
+				root: './fixtures/core-image-deletion/',
+				image: {
+					service: testImageService(),
+				},
+			});
+
+			await fixture.build();
+		});
+
+		it('should delete images that are only used for optimization', async () => {
+			const imagesOnlyOptimized = await fixture.glob('_astro/onlyone.*.*');
+			expect(imagesOnlyOptimized).to.have.lengthOf(1);
+		});
+
+		it('should not delete images that are used in other contexts', async () => {
+			const imagesUsedElsewhere = await fixture.glob('_astro/twoofus.*.*');
+			expect(imagesUsedElsewhere).to.have.lengthOf(2);
+		});
+
+		it('should not delete images that are also used through query params', async () => {
+			const imagesUsedElsewhere = await fixture.glob('_astro/url.*.*');
+			expect(imagesUsedElsewhere).to.have.lengthOf(2);
+		});
+	});
+});