diff options
18 files changed, 129 insertions, 30 deletions
diff --git a/.changeset/slimy-mayflies-vanish.md b/.changeset/slimy-mayflies-vanish.md new file mode 100644 index 000000000..64f61aec9 --- /dev/null +++ b/.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` diff --git a/packages/astro/src/assets/internal.ts b/packages/astro/src/assets/internal.ts index 7a08da15f..45699285e 100644 --- a/packages/astro/src/assets/internal.ts +++ b/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, })); diff --git a/packages/astro/src/assets/types.ts b/packages/astro/src/assets/types.ts index 34ded257f..fb11becb0 100644 --- a/packages/astro/src/assets/types.ts +++ b/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>; }; diff --git a/packages/astro/src/assets/utils/emitAsset.ts b/packages/astro/src/assets/utils/emitAsset.ts index 0f996691b..545299605 100644 --- a/packages/astro/src/assets/utils/emitAsset.ts +++ b/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 diff --git a/packages/astro/src/assets/utils/proxy.ts b/packages/astro/src/assets/utils/proxy.ts index 0dcaffb82..2b4389e4d 100644 --- a/packages/astro/src/assets/utils/proxy.ts +++ b/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]; } }) diff --git a/packages/astro/src/assets/vite-plugin-assets.ts b/packages/astro/src/assets/vite-plugin-assets.ts index 85868f6e1..5a56e76a6 100644 --- a/packages/astro/src/assets/vite-plugin-assets.ts +++ b/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)}`; } } }, diff --git a/packages/astro/src/content/runtime-assets.ts b/packages/astro/src/content/runtime-assets.ts index 0709933de..a34d71bab 100644 --- a/packages/astro/src/content/runtime-assets.ts +++ b/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 }; }); }; } diff --git a/packages/astro/src/content/vite-plugin-content-imports.ts b/packages/astro/src/content/vite-plugin-content-imports.ts index 9410e4076..36194e45e 100644 --- a/packages/astro/src/content/vite-plugin-content-imports.ts +++ b/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); } }); diff --git a/packages/astro/test/fixtures/core-image-deletion/package.json b/packages/astro/test/fixtures/core-image-deletion/package.json new file mode 100644 index 000000000..40a2ee1a6 --- /dev/null +++ b/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" + } +} diff --git a/packages/astro/test/fixtures/core-image-deletion/public/penguin3.jpg b/packages/astro/test/fixtures/core-image-deletion/public/penguin3.jpg Binary files differnew file mode 100644 index 000000000..e859ac3c9 --- /dev/null +++ b/packages/astro/test/fixtures/core-image-deletion/public/penguin3.jpg diff --git a/packages/astro/test/fixtures/core-image-deletion/src/assets/onlyone.jpg b/packages/astro/test/fixtures/core-image-deletion/src/assets/onlyone.jpg Binary files differnew file mode 100644 index 000000000..1a8986ac5 --- /dev/null +++ b/packages/astro/test/fixtures/core-image-deletion/src/assets/onlyone.jpg diff --git a/packages/astro/test/fixtures/core-image-deletion/src/assets/twoofus.jpg b/packages/astro/test/fixtures/core-image-deletion/src/assets/twoofus.jpg Binary files differnew file mode 100644 index 000000000..e859ac3c9 --- /dev/null +++ b/packages/astro/test/fixtures/core-image-deletion/src/assets/twoofus.jpg diff --git a/packages/astro/test/fixtures/core-image-deletion/src/assets/url.jpg b/packages/astro/test/fixtures/core-image-deletion/src/assets/url.jpg Binary files differnew file mode 100644 index 000000000..6479e9212 --- /dev/null +++ b/packages/astro/test/fixtures/core-image-deletion/src/assets/url.jpg diff --git a/packages/astro/test/fixtures/core-image-deletion/src/pages/index.astro b/packages/astro/test/fixtures/core-image-deletion/src/pages/index.astro new file mode 100644 index 000000000..a7aa45081 --- /dev/null +++ b/packages/astro/test/fixtures/core-image-deletion/src/pages/index.astro @@ -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" /> diff --git a/packages/astro/test/fixtures/core-image-deletion/tsconfig.json b/packages/astro/test/fixtures/core-image-deletion/tsconfig.json new file mode 100644 index 000000000..b5bf6a715 --- /dev/null +++ b/packages/astro/test/fixtures/core-image-deletion/tsconfig.json @@ -0,0 +1,9 @@ +{ + "extends": "astro/tsconfigs/base", + "compilerOptions": { + "baseUrl": ".", + "paths": { + "~/assets/*": ["src/assets/*"] + }, + } +} diff --git a/packages/astro/test/fixtures/core-image-ssg/src/pages/blog/[...slug].astro b/packages/astro/test/fixtures/core-image-ssg/src/pages/blog/[...slug].astro index dc25493e8..1173b0fd1 100644 --- a/packages/astro/test/fixtures/core-image-ssg/src/pages/blog/[...slug].astro +++ b/packages/astro/test/fixtures/core-image-ssg/src/pages/blog/[...slug].astro @@ -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> diff --git a/packages/astro/test/image-deletion.test.js b/packages/astro/test/image-deletion.test.js new file mode 100644 index 000000000..2c92f2cec --- /dev/null +++ b/packages/astro/test/image-deletion.test.js @@ -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); + }); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ab455435e..f80477671 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2508,6 +2508,12 @@ importers: specifier: workspace:* version: link:../../.. + packages/astro/test/fixtures/core-image-deletion: + dependencies: + astro: + specifier: workspace:* + version: link:../../.. + packages/astro/test/fixtures/core-image-errors: dependencies: astro: |
