From f2f18b44055c6334a39d6379de88fe41e518aa1e Mon Sep 17 00:00:00 2001 From: Erika <3019731+Princesseuh@users.noreply.github.com> Date: Fri, 26 May 2023 22:03:06 +0200 Subject: [PATCH] feat: support images outside the project (#7139) --- .changeset/wise-cars-hear.md | 5 ++ packages/astro/src/assets/services/service.ts | 9 ++++ packages/astro/src/assets/utils/emitAsset.ts | 27 ++--------- .../astro/src/assets/vite-plugin-assets.ts | 9 ++-- packages/astro/src/content/runtime-assets.ts | 10 +--- packages/astro/src/content/utils.ts | 2 +- packages/astro/src/core/errors/errors-data.ts | 47 ++++++++++++++----- packages/astro/test/core-image.test.js | 42 +++++++++++++---- .../fixtures/core-image-ssr/src/pages/api.ts | 1 - .../src/pages/error-image-src-passed.astro | 6 +++ .../core-image/src/pages/outsideProject.astro | 8 ++++ 11 files changed, 107 insertions(+), 59 deletions(-) create mode 100644 .changeset/wise-cars-hear.md create mode 100644 packages/astro/test/fixtures/core-image/src/pages/error-image-src-passed.astro create mode 100644 packages/astro/test/fixtures/core-image/src/pages/outsideProject.astro diff --git a/.changeset/wise-cars-hear.md b/.changeset/wise-cars-hear.md new file mode 100644 index 000000000000..21019c26f31a --- /dev/null +++ b/.changeset/wise-cars-hear.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +The `src` property returned by ESM importing images with `astro:assets` is now an absolute path, unlocking support for importing images outside the project. diff --git a/packages/astro/src/assets/services/service.ts b/packages/astro/src/assets/services/service.ts index 910f0f0a829c..9706c3ed0893 100644 --- a/packages/astro/src/assets/services/service.ts +++ b/packages/astro/src/assets/services/service.ts @@ -109,6 +109,7 @@ export type BaseServiceTransform = { */ export const baseService: Omit = { validateOptions(options) { + // `src` is missing or is `undefined`. if (!options.src || (typeof options.src !== 'string' && typeof options.src !== 'object')) { throw new AstroError({ ...AstroErrorData.ExpectedImage, @@ -117,6 +118,14 @@ export const baseService: Omit = { } if (!isESMImportedImage(options.src)) { + // User passed an `/@fs/` path instead of the full image. + if (options.src.startsWith('/@fs/')) { + throw new AstroError({ + ...AstroErrorData.LocalImageUsedWrongly, + message: AstroErrorData.LocalImageUsedWrongly.message(options.src), + }); + } + // For remote images, width and height are explicitly required as we can't infer them from the file let missingDimension: 'width' | 'height' | 'both' | undefined; if (!options.width && !options.height) { diff --git a/packages/astro/src/assets/utils/emitAsset.ts b/packages/astro/src/assets/utils/emitAsset.ts index 79775b96d088..9b8d6fc08b3c 100644 --- a/packages/astro/src/assets/utils/emitAsset.ts +++ b/packages/astro/src/assets/utils/emitAsset.ts @@ -2,14 +2,13 @@ import fs from 'node:fs'; import path from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; import slash from 'slash'; -import type { AstroConfig, AstroSettings } from '../../@types/astro'; +import { prependForwardSlash } from '../../core/path.js'; import { imageMetadata, type Metadata } from './metadata.js'; export async function emitESMImage( id: string | undefined, watchMode: boolean, - fileEmitter: any, - settings: Pick + fileEmitter: any ): Promise { if (!id) { return undefined; @@ -40,34 +39,14 @@ export async function emitESMImage( url.searchParams.append('origHeight', meta.height.toString()); url.searchParams.append('origFormat', meta.format); - meta.src = rootRelativePath(settings.config, url); + meta.src = `/@fs` + prependForwardSlash(fileURLToNormalizedPath(url)); } return meta; } -/** - * Utilities inlined from `packages/astro/src/core/util.ts` - * Avoids ESM / CJS bundling failures when accessed from integrations - * due to Vite dependencies in core. - */ - -function rootRelativePath(config: Pick, url: URL): string { - const basePath = fileURLToNormalizedPath(url); - const rootPath = fileURLToNormalizedPath(config.root); - return prependForwardSlash(basePath.slice(rootPath.length)); -} - -function prependForwardSlash(filePath: string): string { - return filePath[0] === '/' ? filePath : '/' + filePath; -} - function fileURLToNormalizedPath(filePath: URL): string { // Uses `slash` package instead of Vite's `normalizePath` // to avoid CJS bundling issues. return slash(fileURLToPath(filePath) + filePath.search).replace(/\\/g, '/'); } - -export function emoji(char: string, fallback: string): string { - return process.platform !== 'win32' ? char : fallback; -} diff --git a/packages/astro/src/assets/vite-plugin-assets.ts b/packages/astro/src/assets/vite-plugin-assets.ts index 9a8f3e3ad9d3..4088c7ec53c4 100644 --- a/packages/astro/src/assets/vite-plugin-assets.ts +++ b/packages/astro/src/assets/vite-plugin-assets.ts @@ -107,13 +107,12 @@ export default function assets({ } const url = new URL(req.url, 'file:'); - const filePath = url.searchParams.get('href'); - - if (!filePath) { + if (!url.searchParams.has('href')) { return next(); } - const filePathURL = new URL('.' + filePath, settings.config.root); + const filePath = url.searchParams.get('href')?.slice('/@fs'.length); + const filePathURL = new URL('.' + filePath, 'file:'); const file = await fs.readFile(filePathURL); // Get the file's metadata from the URL @@ -243,7 +242,7 @@ export default function assets({ const cleanedUrl = removeQueryString(id); if (/\.(jpeg|jpg|png|tiff|webp|gif|svg)$/.test(cleanedUrl)) { - const meta = await emitESMImage(id, this.meta.watchMode, this.emitFile, settings); + const meta = await emitESMImage(id, this.meta.watchMode, this.emitFile); return `export default ${JSON.stringify(meta)}`; } }, diff --git a/packages/astro/src/content/runtime-assets.ts b/packages/astro/src/content/runtime-assets.ts index b24dac8b4913..122e00aa6d4b 100644 --- a/packages/astro/src/content/runtime-assets.ts +++ b/packages/astro/src/content/runtime-assets.ts @@ -1,21 +1,15 @@ import type { PluginContext } from 'rollup'; import { z } from 'zod'; -import type { AstroSettings } from '../@types/astro.js'; import { emitESMImage } from '../assets/index.js'; -export function createImage( - settings: Pick, - pluginContext: PluginContext, - entryFilePath: string -) { +export function createImage(pluginContext: PluginContext, entryFilePath: string) { return () => { return z.string().transform(async (imagePath, ctx) => { const resolvedFilePath = (await pluginContext.resolve(imagePath, entryFilePath))?.id; const metadata = await emitESMImage( resolvedFilePath, pluginContext.meta.watchMode, - pluginContext.emitFile, - settings + pluginContext.emitFile ); if (!metadata) { diff --git a/packages/astro/src/content/utils.ts b/packages/astro/src/content/utils.ts index 831b25fb468e..686176096763 100644 --- a/packages/astro/src/content/utils.ts +++ b/packages/astro/src/content/utils.ts @@ -111,7 +111,7 @@ export async function getEntryData( } schema = schema({ - image: createImage({ config }, pluginContext, entry._internal.filePath), + image: createImage(pluginContext, entry._internal.filePath), }); } diff --git a/packages/astro/src/core/errors/errors-data.ts b/packages/astro/src/core/errors/errors-data.ts index a97c18d0e819..e50000c5651f 100644 --- a/packages/astro/src/core/errors/errors-data.ts +++ b/packages/astro/src/core/errors/errors-data.ts @@ -517,7 +517,7 @@ See https://docs.astro.build/en/guides/server-side-rendering/ for more informati * For unsupported formats such as SVGs and GIFs, you may be able to use an `img` tag directly: * ```astro * --- - * import rocket from '../assets/images/rocket.svg' + * import rocket from '../assets/images/rocket.svg'; * --- * * A rocketship in space. @@ -626,7 +626,7 @@ See https://docs.astro.build/en/guides/server-side-rendering/ for more informati * Making changes to the response, such as setting headers, cookies, and the status code cannot be done outside of page components. */ ResponseSentError: { - title: 'Unable to set response', + title: 'Unable to set response.', code: 3030, message: 'The response has already been sent to the browser and cannot be altered.', }, @@ -646,7 +646,7 @@ See https://docs.astro.build/en/guides/server-side-rendering/ for more informati * ``` */ MiddlewareNoDataOrNextCalled: { - title: "The middleware didn't return a response or call `next`", + title: "The middleware didn't return a response or call `next`.", code: 3031, message: 'The middleware needs to either return a `Response` object or call the `next` function.', @@ -666,7 +666,7 @@ See https://docs.astro.build/en/guides/server-side-rendering/ for more informati * ``` */ MiddlewareNotAResponse: { - title: 'The middleware returned something that is not a `Response` object', + title: 'The middleware returned something that is not a `Response` object.', code: 3032, message: 'Any data returned from middleware must be a valid `Response` object.', }, @@ -687,12 +687,38 @@ See https://docs.astro.build/en/guides/server-side-rendering/ for more informati * ``` */ LocalsNotAnObject: { - title: 'Value assigned to `locals` is not accepted', + title: 'Value assigned to `locals` is not accepted.', code: 3033, message: '`locals` can only be assigned to an object. Other values like numbers, strings, etc. are not accepted.', hint: 'If you tried to remove some information from the `locals` object, try to use `delete` or set the property to `undefined`.', }, + /** + * @docs + * @see + * - [Assets (Experimental)](https://docs.astro.build/en/guides/assets/) + * @description + * When using the default image services, `Image`'s and `getImage`'s `src` parameter must be either an imported image or an URL, it cannot be a filepath. + * + * ```astro + * --- + * import { Image } from "astro:assets"; + * import myImage from "../my_image.png"; + * --- + * + * + * Cool image + * + * + * Cool image + * ``` + */ + LocalImageUsedWrongly: { + title: 'ESM imported images must be passed as-is.', + code: 3034, + message: (imageFilePath: string) => + `\`Image\`'s and \`getImage\`'s \`src\` parameter must be an imported image or an URL, it cannot be a filepath. Received \`${imageFilePath}\`.`, + }, // No headings here, that way Vite errors are merged with Astro ones in the docs, which makes more sense to users. // Vite Errors - 4xxx /** @@ -961,7 +987,6 @@ See https://docs.astro.build/en/guides/server-side-rendering/ for more informati }, /** * @docs - * @message A content collection schema should not contain `slug` since it is reserved for slug generation. Remove this from your `COLLECTION_NAME` collection schema. * @see * - [The reserved entry `slug` field](https://docs.astro.build/en/guides/content-collections/) * @description @@ -970,9 +995,8 @@ See https://docs.astro.build/en/guides/server-side-rendering/ for more informati ContentSchemaContainsSlugError: { title: 'Content Schema should not contain `slug`.', code: 9003, - message: (collection: string) => { - return `A content collection schema should not contain \`slug\` since it is reserved for slug generation. Remove this from your ${collection} collection schema.`; - }, + message: (collectionName: string) => + `A content collection schema should not contain \`slug\` since it is reserved for slug generation. Remove this from your ${collectionName} collection schema.`, hint: 'See https://docs.astro.build/en/guides/content-collections/ for more on the `slug` field.', }, @@ -985,9 +1009,8 @@ See https://docs.astro.build/en/guides/server-side-rendering/ for more informati CollectionDoesNotExistError: { title: 'Collection does not exist', code: 9004, - message: (collection: string) => { - return `The collection **${collection}** does not exist. Ensure a collection directory with this name exists.`; - }, + message: (collectionName: string) => + `The collection **${collectionName}** does not exist. Ensure a collection directory with this name exists.`, hint: 'See https://docs.astro.build/en/guides/content-collections/ for more on creating collections.', }, /** diff --git a/packages/astro/test/core-image.test.js b/packages/astro/test/core-image.test.js index b6b48270eebc..4fec4828471c 100644 --- a/packages/astro/test/core-image.test.js +++ b/packages/astro/test/core-image.test.js @@ -114,6 +114,32 @@ describe('astro:image', () => { expect(logs).to.have.a.lengthOf(1); expect(logs[0].message).to.contain('Received unsupported format'); }); + + it("errors when an ESM imported image's src is passed to Image/getImage instead of the full import ssss", async () => { + logs.length = 0; + let res = await fixture.fetch('/error-image-src-passed'); + await res.text(); + + expect(logs).to.have.a.lengthOf(1); + expect(logs[0].message).to.contain('must be an imported image or an URL'); + }); + + it('supports images from outside the project', async () => { + let res = await fixture.fetch('/outsideProject'); + let html = await res.text(); + $ = cheerio.load(html); + + let $img = $('img'); + expect($img).to.have.a.lengthOf(2); + expect( + $img.toArray().every((img) => { + return ( + img.attribs['src'].startsWith('/@fs/') || + img.attribs['src'].startsWith('/_image?href=%2F%40fs%2F') + ); + }) + ).to.be.true; + }); }); describe('vite-isms', () => { @@ -228,9 +254,9 @@ describe('astro:image', () => { expect($img).to.have.a.lengthOf(1); // Verbose test for the full URL to make sure the image went through the full pipeline - expect($img.attr('src')).to.equal( - '/_image?href=%2Fsrc%2Fassets%2Fpenguin1.jpg%3ForigWidth%3D207%26origHeight%3D243%26origFormat%3Djpg&f=webp' - ); + expect( + $img.attr('src').startsWith('/_image') && $img.attr('src').endsWith('f=webp') + ).to.equal(true); }); it('has width and height attributes', () => { @@ -297,12 +323,12 @@ describe('astro:image', () => { it('has proper source for directly used image', () => { let $img = $('#direct-image img'); - expect($img.attr('src').startsWith('/src/')).to.equal(true); + expect($img.attr('src').startsWith('/')).to.equal(true); }); it('has proper source for refined image', () => { let $img = $('#refined-image img'); - expect($img.attr('src').startsWith('/src/')).to.equal(true); + expect($img.attr('src').startsWith('/')).to.equal(true); }); it('has proper sources for array of images', () => { @@ -310,7 +336,7 @@ describe('astro:image', () => { const imgsSrcs = []; $img.each((i, img) => imgsSrcs.push(img.attribs['src'])); expect($img).to.have.a.lengthOf(2); - expect(imgsSrcs.every((img) => img.startsWith('/src/'))).to.be.true; + expect(imgsSrcs.every((img) => img.startsWith('/'))).to.be.true; }); it('has proper attributes for optimized image through getImage', () => { @@ -330,7 +356,7 @@ describe('astro:image', () => { it('properly handles nested images', () => { let $img = $('#nested-image img'); - expect($img.attr('src').startsWith('/src/')).to.equal(true); + expect($img.attr('src').startsWith('/')).to.equal(true); }); }); @@ -348,7 +374,7 @@ describe('astro:image', () => { }); it('includes /src in the path', async () => { - expect($('img').attr('src').startsWith('/src')).to.equal(true); + expect($('img').attr('src').includes('/src')).to.equal(true); }); }); }); diff --git a/packages/astro/test/fixtures/core-image-ssr/src/pages/api.ts b/packages/astro/test/fixtures/core-image-ssr/src/pages/api.ts index 9534383db893..a5526e1a161e 100644 --- a/packages/astro/test/fixtures/core-image-ssr/src/pages/api.ts +++ b/packages/astro/test/fixtures/core-image-ssr/src/pages/api.ts @@ -2,7 +2,6 @@ import { APIRoute } from "../../../../../src/@types/astro"; export const get = (async ({ params, request }) => { const url = new URL(request.url); - console.log(url) const src = url.searchParams.get("src"); return { diff --git a/packages/astro/test/fixtures/core-image/src/pages/error-image-src-passed.astro b/packages/astro/test/fixtures/core-image/src/pages/error-image-src-passed.astro new file mode 100644 index 000000000000..71b34101ea14 --- /dev/null +++ b/packages/astro/test/fixtures/core-image/src/pages/error-image-src-passed.astro @@ -0,0 +1,6 @@ +--- +import { Image } from "astro:assets"; +import myImage from "../assets/penguin1.jpg"; +--- + +hello diff --git a/packages/astro/test/fixtures/core-image/src/pages/outsideProject.astro b/packages/astro/test/fixtures/core-image/src/pages/outsideProject.astro new file mode 100644 index 000000000000..268b44741302 --- /dev/null +++ b/packages/astro/test/fixtures/core-image/src/pages/outsideProject.astro @@ -0,0 +1,8 @@ +--- +import { Image } from "astro:assets"; +import imageOutsideProject from "../../../core-image-base/src/assets/penguin1.jpg"; +--- + +outside project + +