Skip to content

Commit

Permalink
feat: support images outside the project (#7139)
Browse files Browse the repository at this point in the history
  • Loading branch information
Princesseuh authored May 26, 2023
1 parent 1c77779 commit f2f18b4
Show file tree
Hide file tree
Showing 11 changed files with 107 additions and 59 deletions.
5 changes: 5 additions & 0 deletions .changeset/wise-cars-hear.md
Original file line number Diff line number Diff line change
@@ -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.
9 changes: 9 additions & 0 deletions packages/astro/src/assets/services/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ export type BaseServiceTransform = {
*/
export const baseService: Omit<LocalImageService, 'transform'> = {
validateOptions(options) {
// `src` is missing or is `undefined`.
if (!options.src || (typeof options.src !== 'string' && typeof options.src !== 'object')) {
throw new AstroError({
...AstroErrorData.ExpectedImage,
Expand All @@ -117,6 +118,14 @@ export const baseService: Omit<LocalImageService, 'transform'> = {
}

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) {
Expand Down
27 changes: 3 additions & 24 deletions packages/astro/src/assets/utils/emitAsset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<AstroSettings, 'config'>
fileEmitter: any
): Promise<Metadata | undefined> {
if (!id) {
return undefined;
Expand Down Expand Up @@ -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<AstroConfig, 'root'>, 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;
}
9 changes: 4 additions & 5 deletions packages/astro/src/assets/vite-plugin-assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)}`;
}
},
Expand Down
10 changes: 2 additions & 8 deletions packages/astro/src/content/runtime-assets.ts
Original file line number Diff line number Diff line change
@@ -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<AstroSettings, 'config'>,
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) {
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/content/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export async function getEntryData(
}

schema = schema({
image: createImage({ config }, pluginContext, entry._internal.filePath),
image: createImage(pluginContext, entry._internal.filePath),
});
}

Expand Down
47 changes: 35 additions & 12 deletions packages/astro/src/core/errors/errors-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
* ---
*
* <img src={rocket.src} width={rocket.width} height={rocket.height} alt="A rocketship in space." />
Expand Down Expand Up @@ -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.',
},
Expand All @@ -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.',
Expand All @@ -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.',
},
Expand All @@ -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";
* ---
*
* <!-- GOOD: `src` is the full imported image. -->
* <Image src={myImage} alt="Cool image" />
*
* <!-- BAD: `src` is an image's `src` path instead of the full image. -->
* <Image src={myImage.src} alt="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
/**
Expand Down Expand Up @@ -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
Expand All @@ -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.',
},

Expand All @@ -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.',
},
/**
Expand Down
42 changes: 34 additions & 8 deletions packages/astro/test/core-image.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -297,20 +323,20 @@ 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', () => {
let $img = $('#array-of-images img');
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', () => {
Expand All @@ -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);
});
});

Expand All @@ -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);
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
import { Image } from "astro:assets";
import myImage from "../assets/penguin1.jpg";
---

<Image src={myImage.src} alt="hello"/>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
import { Image } from "astro:assets";
import imageOutsideProject from "../../../core-image-base/src/assets/penguin1.jpg";
---

<Image src={imageOutsideProject} alt="outside project" />

<img src={imageOutsideProject.src} />

0 comments on commit f2f18b4

Please sign in to comment.