diff --git a/.changeset/few-avocados-notice.md b/.changeset/few-avocados-notice.md new file mode 100644 index 0000000000..72c8ba2499 --- /dev/null +++ b/.changeset/few-avocados-notice.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +Fixes `astro:assets` not working when using complex config with `vite.build.rollupOptions.output.assetFileNames` diff --git a/packages/astro/e2e/dev-toolbar-audits.test.js b/packages/astro/e2e/dev-toolbar-audits.test.js index 5066376e45..9a628a1c06 100644 --- a/packages/astro/e2e/dev-toolbar-audits.test.js +++ b/packages/astro/e2e/dev-toolbar-audits.test.js @@ -44,7 +44,7 @@ test.describe('Dev Toolbar - Audits', () => { await appButton.click(); }); - test('can handle mutations zzz', async ({ page, astro }) => { + test('can handle mutations', async ({ page, astro }) => { await page.goto(astro.resolveUrl('/audits-mutations')); const toolbar = page.locator('astro-dev-toolbar'); diff --git a/packages/astro/src/assets/build/generate.ts b/packages/astro/src/assets/build/generate.ts index 496dc1e6e4..f209345387 100644 --- a/packages/astro/src/assets/build/generate.ts +++ b/packages/astro/src/assets/build/generate.ts @@ -1,5 +1,5 @@ import fs, { readFileSync } from 'node:fs'; -import { basename, join } from 'node:path/posix'; +import { basename } from 'node:path/posix'; import { dim, green } from 'kleur/colors'; import type PQueue from 'p-queue'; import type { AstroConfig } from '../../@types/astro.js'; @@ -9,7 +9,7 @@ import { getTimeStat } from '../../core/build/util.js'; import { AstroError } from '../../core/errors/errors.js'; import { AstroErrorData } from '../../core/errors/index.js'; import type { Logger } from '../../core/logger/core.js'; -import { isRemotePath, prependForwardSlash } from '../../core/path.js'; +import { isRemotePath, removeLeadingForwardSlash } from '../../core/path.js'; import { isServerLikeOutput } from '../../prerender/utils.js'; import type { MapValue } from '../../type-utils.js'; import { getConfiguredImageService } from '../internal.js'; @@ -89,10 +89,7 @@ export async function prepareAssetsGenerationEnv( } function getFullImagePath(originalFilePath: string, env: AssetEnv): URL { - return new URL( - '.' + prependForwardSlash(join(env.assetsFolder, basename(originalFilePath))), - env.serverRoot - ); + return new URL(removeLeadingForwardSlash(originalFilePath), env.serverRoot); } export async function generateImagesForPath( @@ -115,7 +112,7 @@ export async function generateImagesForPath( // For instance, the same image could be referenced in both a server-rendered page and build-time-rendered page if ( !env.isSSR && - !isRemotePath(originalFilePath) && + transformsAndPath.originalSrcPath && !globalThis.astroAsset.referencedImages?.has(transformsAndPath.originalSrcPath) ) { try { diff --git a/packages/astro/src/assets/internal.ts b/packages/astro/src/assets/internal.ts index 5cd3cc7bf5..6a3becde3d 100644 --- a/packages/astro/src/assets/internal.ts +++ b/packages/astro/src/assets/internal.ts @@ -74,9 +74,9 @@ export async function getImage( } } - const originalPath = isESMImportedImage(resolvedOptions.src) + const originalFilePath = isESMImportedImage(resolvedOptions.src) ? resolvedOptions.src.fsPath - : resolvedOptions.src; + : undefined; // Only set for ESM imports, where we do have a file path // 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 @@ -112,10 +112,14 @@ export async function getImage( !(isRemoteImage(validatedOptions.src) && imageURL === validatedOptions.src) ) { const propsToHash = service.propertiesToHash ?? DEFAULT_HASH_PROPS; - imageURL = globalThis.astroAsset.addStaticImage(validatedOptions, propsToHash, originalPath); + imageURL = globalThis.astroAsset.addStaticImage( + validatedOptions, + propsToHash, + originalFilePath + ); srcSets = srcSetTransforms.map((srcSet) => ({ transform: srcSet.transform, - url: globalThis.astroAsset.addStaticImage!(srcSet.transform, propsToHash, originalPath), + url: globalThis.astroAsset.addStaticImage!(srcSet.transform, propsToHash, originalFilePath), descriptor: srcSet.descriptor, attributes: srcSet.attributes, })); diff --git a/packages/astro/src/assets/types.ts b/packages/astro/src/assets/types.ts index b77180e209..29cf1ef9ee 100644 --- a/packages/astro/src/assets/types.ts +++ b/packages/astro/src/assets/types.ts @@ -11,7 +11,7 @@ export type ImageOutputFormat = (typeof VALID_OUTPUT_FORMATS)[number] | (string export type AssetsGlobalStaticImagesList = Map< string, { - originalSrcPath: string; + originalSrcPath: string | undefined; transforms: Map; } >; @@ -21,7 +21,7 @@ declare global { var astroAsset: { imageService?: ImageService; addStaticImage?: - | ((options: ImageTransform, hashProperties: string[], fsPath: string) => string) + | ((options: ImageTransform, hashProperties: string[], fsPath: string | undefined) => string) | undefined; staticImages?: AssetsGlobalStaticImagesList; referencedImages?: Set; diff --git a/packages/astro/src/assets/utils/transformToPath.ts b/packages/astro/src/assets/utils/transformToPath.ts index 0cac1a6715..9a91098cc8 100644 --- a/packages/astro/src/assets/utils/transformToPath.ts +++ b/packages/astro/src/assets/utils/transformToPath.ts @@ -1,19 +1,18 @@ -import { basename, extname } from 'node:path'; +import { basename, dirname, extname } from 'node:path'; import { deterministicString } from 'deterministic-object-hash'; import { removeQueryString } from '../../core/path.js'; import { shorthash } from '../../runtime/server/shorthash.js'; import type { ImageTransform } from '../types.js'; import { isESMImportedImage } from './imageKind.js'; -export function propsToFilename(transform: ImageTransform, hash: string) { - let filename = removeQueryString( - isESMImportedImage(transform.src) ? transform.src.src : transform.src - ); +export function propsToFilename(filePath: string, transform: ImageTransform, hash: string) { + let filename = decodeURIComponent(removeQueryString(filePath)); const ext = extname(filename); - filename = decodeURIComponent(basename(filename, ext)); + filename = basename(filename, ext); + const prefixDirname = isESMImportedImage(transform.src) ? dirname(filePath) : ''; let outputExt = transform.format ? `.${transform.format}` : ext; - return `/${filename}_${hash}${outputExt}`; + return decodeURIComponent(`${prefixDirname}/${filename}_${hash}${outputExt}`); } export function hashTransform( diff --git a/packages/astro/src/assets/vite-plugin-assets.ts b/packages/astro/src/assets/vite-plugin-assets.ts index 563874a561..fe92b2538e 100644 --- a/packages/astro/src/assets/vite-plugin-assets.ts +++ b/packages/astro/src/assets/vite-plugin-assets.ts @@ -9,6 +9,7 @@ import { appendForwardSlash, joinPaths, prependForwardSlash, + removeBase, removeQueryString, } from '../core/path.js'; import { isServerLikeOutput } from '../prerender/utils.js'; @@ -91,7 +92,7 @@ export default function assets({ return; } - globalThis.astroAsset.addStaticImage = (options, hashProperties, originalPath) => { + globalThis.astroAsset.addStaticImage = (options, hashProperties, originalFSPath) => { if (!globalThis.astroAsset.staticImages) { globalThis.astroAsset.staticImages = new Map< string, @@ -102,13 +103,18 @@ export default function assets({ >(); } - // Rollup will copy the file to the output directory, this refer to this final path, not to the original path + // Rollup will copy the file to the output directory, as such this is the path in the output directory, including the asset prefix / base const ESMImportedImageSrc = isESMImportedImage(options.src) ? options.src.src : options.src; const fileExtension = extname(ESMImportedImageSrc); - const pf = getAssetsPrefix(fileExtension, settings.config.build.assetsPrefix); - const finalOriginalImagePath = ESMImportedImageSrc.replace(pf, ''); + const assetPrefix = getAssetsPrefix(fileExtension, settings.config.build.assetsPrefix); + + // This is the path to the original image, from the dist root, without the base or the asset prefix (e.g. /_astro/image.hash.png) + const finalOriginalPath = removeBase( + removeBase(ESMImportedImageSrc, settings.config.base), + assetPrefix + ); const hash = hashTransform( options, @@ -117,21 +123,26 @@ export default function assets({ ); let finalFilePath: string; - let transformsForPath = globalThis.astroAsset.staticImages.get(finalOriginalImagePath); + let transformsForPath = globalThis.astroAsset.staticImages.get(finalOriginalPath); let transformForHash = transformsForPath?.transforms.get(hash); + + // If the same image has already been transformed with the same options, we'll reuse the final path if (transformsForPath && transformForHash) { finalFilePath = transformForHash.finalPath; } else { finalFilePath = prependForwardSlash( - joinPaths(settings.config.build.assets, propsToFilename(options, hash)) + joinPaths( + isESMImportedImage(options.src) ? '' : settings.config.build.assets, + prependForwardSlash(propsToFilename(finalOriginalPath, options, hash)) + ) ); if (!transformsForPath) { - globalThis.astroAsset.staticImages.set(finalOriginalImagePath, { - originalSrcPath: originalPath, + globalThis.astroAsset.staticImages.set(finalOriginalPath, { + originalSrcPath: originalFSPath, transforms: new Map(), }); - transformsForPath = globalThis.astroAsset.staticImages.get(finalOriginalImagePath)!; + transformsForPath = globalThis.astroAsset.staticImages.get(finalOriginalPath)!; } transformsForPath.transforms.set(hash, { @@ -143,7 +154,7 @@ export default function assets({ // The paths here are used for URLs, so we need to make sure they have the proper format for an URL // (leading slash, prefixed with the base / assets prefix, encoded, etc) if (settings.config.build.assetsPrefix) { - return encodeURI(joinPaths(pf, finalFilePath)); + return encodeURI(joinPaths(assetPrefix, finalFilePath)); } else { return encodeURI(prependForwardSlash(joinPaths(settings.config.base, finalFilePath))); } diff --git a/packages/astro/test/core-image-unconventional-settings.test.js b/packages/astro/test/core-image-unconventional-settings.test.js new file mode 100644 index 0000000000..9ecf6aab6b --- /dev/null +++ b/packages/astro/test/core-image-unconventional-settings.test.js @@ -0,0 +1,187 @@ +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; +import * as cheerio from 'cheerio'; + +import { testImageService } from './test-image-service.js'; +import { loadFixture } from './test-utils.js'; + +/** + ** @typedef {import('../src/@types/astro').AstroInlineConfig & { root?: string | URL }} AstroInlineConfig + */ + +/** @type {AstroInlineConfig} */ +const defaultSettings = { + root: './fixtures/core-image-unconventional-settings/', + image: { + service: testImageService(), + }, +}; + +describe('astro:assets - Support unconventional build settings properly', () => { + /** @type {import('./test-utils').Fixture} */ + let fixture; + + it('supports assetsPrefix', async () => { + fixture = await loadFixture({ + ...defaultSettings, + build: { + assetsPrefix: 'https://cdn.example.com/', + }, + }); + await fixture.build(); + + const html = await fixture.readFile('/index.html'); + const $ = cheerio.load(html); + const src = $('#walrus-img').attr('src'); + assert.equal(src.startsWith('https://cdn.example.com/'), true); + + const data = await fixture.readFile(src.replace('https://cdn.example.com/', ''), null); + assert.equal(data instanceof Buffer, true); + }); + + it('supports base', async () => { + fixture = await loadFixture({ + ...defaultSettings, + build: { + base: '/subdir/', + }, + }); + await fixture.build(); + + const html = await fixture.readFile('/index.html'); + const $ = cheerio.load(html); + const src = $('#walrus-img').attr('src'); + const data = await fixture.readFile(src.replace('/subdir/', ''), null); + assert.equal(data instanceof Buffer, true); + }); + + // This test is a bit of a stretch, but it's a good sanity check, `assetsPrefix` should take precedence over `base` in this context + it('supports assetsPrefix + base', async () => { + fixture = await loadFixture({ + ...defaultSettings, + build: { + assetsPrefix: 'https://cdn.example.com/', + base: '/subdir/', + }, + }); + await fixture.build(); + + const html = await fixture.readFile('/index.html'); + const $ = cheerio.load(html); + const src = $('#walrus-img').attr('src'); + assert.equal(src.startsWith('https://cdn.example.com/'), true); + + const data = await fixture.readFile(src.replace('https://cdn.example.com/', ''), null); + assert.equal(data instanceof Buffer, true); + }); + + it('supports custom build.assets', async () => { + fixture = await loadFixture({ + ...defaultSettings, + build: { + assets: 'assets', + }, + }); + await fixture.build(); + + const html = await fixture.readFile('/index.html'); + const $ = cheerio.load(html); + + const unoptimizedSrc = $('#walrus-img-unoptimized').attr('src'); + assert.equal(unoptimizedSrc.startsWith('/assets/'), true); + + const src = $('#walrus-img').attr('src'); + const data = await fixture.readFile(src, null); + + assert.equal(data instanceof Buffer, true); + }); + + it('supports custom vite.build.rollupOptions.output.assetFileNames', async () => { + fixture = await loadFixture({ + ...defaultSettings, + vite: { + build: { + rollupOptions: { + output: { + assetFileNames: 'images/hello_[name].[ext]', + }, + }, + }, + }, + }); + await fixture.build(); + + const html = await fixture.readFile('/index.html'); + const $ = cheerio.load(html); + const unoptimizedSrc = $('#walrus-img-unoptimized').attr('src'); + assert.equal(unoptimizedSrc, '/images/hello_light_walrus.avif'); + + const src = $('#walrus-img').attr('src'); + const data = await fixture.readFile(src, null); + + assert.equal(data instanceof Buffer, true); + }); + + it('supports complex vite.build.rollupOptions.output.assetFileNames', async () => { + fixture = await loadFixture({ + ...defaultSettings, + vite: { + build: { + rollupOptions: { + output: { + assetFileNames: 'assets/[hash]/[name][extname]', + }, + }, + }, + }, + }); + await fixture.build(); + + const html = await fixture.readFile('/index.html'); + const $ = cheerio.load(html); + const unoptimizedSrc = $('#walrus-img-unoptimized').attr('src'); + const originalData = await fixture.readFile(unoptimizedSrc, null); + assert.equal(originalData instanceof Buffer, true); + + const src = $('#walrus-img').attr('src'); + const data = await fixture.readFile(src, null); + + assert.equal(data instanceof Buffer, true); + }); + + it('supports custom vite.build.rollupOptions.output.assetFileNames with assetsPrefix', async () => { + fixture = await loadFixture({ + ...defaultSettings, + vite: { + build: { + rollupOptions: { + output: { + assetFileNames: 'images/hello_[name].[ext]', + }, + }, + }, + }, + build: { + assetsPrefix: 'https://cdn.example.com/', + }, + }); + await fixture.build(); + + const html = await fixture.readFile('/index.html'); + const $ = cheerio.load(html); + const unoptimizedSrc = $('#walrus-img-unoptimized').attr('src'); + assert.equal(unoptimizedSrc, 'https://cdn.example.com/images/hello_light_walrus.avif'); + + const unoptimizedData = await fixture.readFile( + unoptimizedSrc.replace('https://cdn.example.com/', ''), + null + ); + assert.equal(unoptimizedData instanceof Buffer, true); + + const src = $('#walrus-img').attr('src'); + assert.equal(src.startsWith('https://cdn.example.com/'), true); + + const data = await fixture.readFile(src.replace('https://cdn.example.com/', ''), null); + assert.equal(data instanceof Buffer, true); + }); +}); diff --git a/packages/astro/test/fixtures/core-image-unconventional-settings/package.json b/packages/astro/test/fixtures/core-image-unconventional-settings/package.json new file mode 100644 index 0000000000..c3a0834e8a --- /dev/null +++ b/packages/astro/test/fixtures/core-image-unconventional-settings/package.json @@ -0,0 +1,8 @@ +{ + "name": "@test/core-image-unconventional-settings", + "version": "0.0.0", + "private": true, + "dependencies": { + "astro": "workspace:*" + } +} diff --git a/packages/astro/test/fixtures/core-image-unconventional-settings/src/assets/light_walrus.avif b/packages/astro/test/fixtures/core-image-unconventional-settings/src/assets/light_walrus.avif new file mode 100644 index 0000000000..89e1c3a143 Binary files /dev/null and b/packages/astro/test/fixtures/core-image-unconventional-settings/src/assets/light_walrus.avif differ diff --git a/packages/astro/test/fixtures/core-image-unconventional-settings/src/pages/index.astro b/packages/astro/test/fixtures/core-image-unconventional-settings/src/pages/index.astro new file mode 100644 index 0000000000..f896aad064 --- /dev/null +++ b/packages/astro/test/fixtures/core-image-unconventional-settings/src/pages/index.astro @@ -0,0 +1,7 @@ +--- +import { Image } from "astro:assets"; +import walrus from "../assets/light_walrus.avif"; +--- + +A walrus +A walrus diff --git a/packages/astro/test/fixtures/core-image-unconventional-settings/tsconfig.json b/packages/astro/test/fixtures/core-image-unconventional-settings/tsconfig.json new file mode 100644 index 0000000000..b5bf6a715e --- /dev/null +++ b/packages/astro/test/fixtures/core-image-unconventional-settings/tsconfig.json @@ -0,0 +1,9 @@ +{ + "extends": "astro/tsconfigs/base", + "compilerOptions": { + "baseUrl": ".", + "paths": { + "~/assets/*": ["src/assets/*"] + }, + } +} diff --git a/packages/astro/test/image-deletion.test.js b/packages/astro/test/image-deletion.test.js index cb4b464bf6..9af94a7543 100644 --- a/packages/astro/test/image-deletion.test.js +++ b/packages/astro/test/image-deletion.test.js @@ -3,7 +3,7 @@ import { before, describe, it } from 'node:test'; import { testImageService } from './test-image-service.js'; import { loadFixture } from './test-utils.js'; -describe('astro:assets - delete images that are unused zzz', () => { +describe('astro:assets - delete images that are unused', () => { /** @type {import('./test-utils.js').Fixture} */ let fixture; diff --git a/packages/internal-helpers/src/path.ts b/packages/internal-helpers/src/path.ts index 50daf97bce..6b1c981257 100644 --- a/packages/internal-helpers/src/path.ts +++ b/packages/internal-helpers/src/path.ts @@ -97,3 +97,10 @@ export function fileExtension(path: string) { const ext = path.split('.').pop(); return ext !== path ? `.${ext}` : ''; } + +export function removeBase(path: string, base: string) { + if (path.startsWith(base)) { + return path.slice(removeTrailingForwardSlash(base).length); + } + return path; +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e939aac94e..5e3f64bcd7 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2553,6 +2553,12 @@ importers: specifier: workspace:* version: link:../../.. + packages/astro/test/fixtures/core-image-unconventional-settings: + dependencies: + astro: + specifier: workspace:* + version: link:../../.. + packages/astro/test/fixtures/css-assets: dependencies: '@test/astro-font-awesome-package':