Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support images outside the project #7139

Merged
merged 8 commits into from
May 26, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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));
Comment on lines -43 to +42
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A note on this: Vite alternates between a root relative path and /@fs depending on whether the path is inside the root or not. Here it's always using /@fs which is and should fine, but there are small bugs now and then because Vite's rule is not applied by frameworks. Something to keep an eye on.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm willing to take the risk. I think the consistency of it is great, even though I don't necessarily want users to rely on it. Will definitely reconsider if it becomes an issue!

}

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;
}
6 changes: 3 additions & 3 deletions packages/astro/src/assets/vite-plugin-assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,13 @@ export default function assets({
}

const url = new URL(req.url, 'file:');
const filePath = url.searchParams.get('href');
const filePath = url.searchParams.get('href')?.slice('/@fs'.length);

if (!filePath) {
return next();
}

const filePathURL = new URL('.' + filePath, settings.config.root);
const filePathURL = new URL('.' + filePath, 'file:');
Princesseuh marked this conversation as resolved.
Show resolved Hide resolved
const file = await fs.readFile(filePathURL);

// Get the file's metadata from the URL
Expand Down Expand Up @@ -243,7 +243,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
28 changes: 17 additions & 11 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 @@ -715,9 +715,18 @@ See https://docs.astro.build/en/guides/server-side-rendering/ for more informati
LocalsNotSerializable: {
title: '`Astro.locals` is not serializable',
code: 3034,
message: (href: string) => {
return `The information stored in \`Astro.locals\` for the path "${href}" is not serializable.\nMake sure you store only serializable data.`;
},
message: (href: string) =>
`The information stored in \`Astro.locals\` for the path "${href}" is not serializable.\nMake sure you store only serializable data.`,
},
/**
* @docs
*/
ImageOutsideProject: {
title: 'Image is located outside project.',
code: 3035,
message: (imagePath: string) =>
`Image ${imagePath} is located outside the Astro project and could not be processed.`,
hint: 'We recommend having all your images and content inside of your Astro project.',
},
// 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 @@ -987,7 +996,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 @@ -996,9 +1004,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 @@ -1011,9 +1018,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
18 changes: 9 additions & 9 deletions packages/astro/test/core-image.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -228,9 +228,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 @@ -295,22 +295,22 @@ describe('astro:image', () => {
expect($img).to.have.a.lengthOf(7);
});

it('has proper source for directly used image', () => {
it('has proper source for directly used image ssss', () => {
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 +330,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 +348,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