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

fix(images): Improve error handling around the new assets feature #6649

Merged
merged 18 commits into from
Mar 29, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
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
5 changes: 5 additions & 0 deletions .changeset/lemon-snails-kiss.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Improve error handling when using `astro:assets`
7 changes: 7 additions & 0 deletions packages/astro/src/assets/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ interface GetImageResult {
* This is functionally equivalent to using the `<Image />` component, as the component calls this function internally.
*/
export async function getImage(options: ImageTransform): Promise<GetImageResult> {
if (!options || typeof options !== 'object') {
throw new AstroError({
...AstroErrorData.ExpectedImageOptions,
message: AstroErrorData.ExpectedImageOptions.message(JSON.stringify(options)),
});
}

const service = await getConfiguredImageService();
const validatedOptions = service.validateOptions ? service.validateOptions(options) : options;

Expand Down
7 changes: 7 additions & 0 deletions packages/astro/src/assets/services/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,13 @@ export type BaseServiceTransform = {
*/
export const baseService: Omit<LocalImageService, 'transform'> = {
validateOptions(options) {
if (!options.src || (typeof options.src !== 'string' && typeof options.src !== 'object')) {
throw new AstroError({
...AstroErrorData.ExpectedImage,
message: AstroErrorData.ExpectedImage.message(JSON.stringify(options.src)),
});
}

if (!isESMImportedImage(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;
Expand Down
23 changes: 19 additions & 4 deletions packages/astro/src/content/runtime-assets.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,38 @@
import { pathToFileURL } from 'url';
import { z } from 'zod';
import { imageMetadata, type Metadata } from '../assets/utils/metadata.js';
import {
imageMetadata as internalGetImageMetadata,
type Metadata,
} from '../assets/utils/metadata.js';

export function createImage(options: { assetsDir: string; relAssetsDir: string }) {
return () => {
if (options.assetsDir === 'undefined') {
throw new Error('Enable `experimental.assets` in your Astro config to use image()');
}

return z.string({ description: '__image' }).transform(async (imagePath) => {
return await getImageMetadata(pathToFileURL(imagePath));
return z.string({ description: '__image' }).transform(async (imagePath, ctx) => {
const imageMetadata = await getImageMetadata(pathToFileURL(imagePath));

if (!imageMetadata) {
ctx.addIssue({
code: 'custom',
message: `Image ${imagePath} does not exist. Is the path correct?`,
fatal: true,
});

return z.NEVER;
}

return imageMetadata;
});
};
}

async function getImageMetadata(
imagePath: URL
): Promise<(Metadata & { __astro_asset: true }) | undefined> {
const meta = await imageMetadata(imagePath);
const meta = await internalGetImageMetadata(imagePath);

if (!meta) {
return undefined;
Expand Down
7 changes: 5 additions & 2 deletions packages/astro/src/content/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,13 @@ export async function getEntryData(
object[schemaName] = z.preprocess(
async (value: unknown) => {
if (!value || typeof value !== 'string') return value;
return (await resolver(value))?.id;
return (
(await resolver(value))?.id ??
path.join(path.dirname(entry._internal.filePath), value)
);
},
schema,
{ description: undefined }
{ description: '__image' }
Copy link
Member Author

Choose a reason for hiding this comment

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

Unrelated bug fix. Since we resetted the description, the paths would always be relative to the first collection loaded instead of the one currently being rendered

);
} else if ('shape' in schema) {
await preprocessAssetPaths(schema.shape);
Expand Down
70 changes: 68 additions & 2 deletions packages/astro/src/core/errors/errors-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ See https://docs.astro.build/en/guides/server-side-rendering/ for more informati
* If the image is merely decorative (i.e. doesn’t contribute to the understanding of the page), set `alt=""` so that screen readers know to ignore the image.
*/
ImageMissingAlt: {
title: 'Missing alt property',
title: 'Missing alt property.',
code: 3022,
message: 'The alt property is required.',
hint: "The `alt` property is important for the purpose of accessibility, without it users using screen readers or other assistive technologies won't be able to understand what your image is supposed to represent. See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img#attr-alt for more information.",
Expand All @@ -479,7 +479,7 @@ See https://docs.astro.build/en/guides/server-side-rendering/ for more informati
* If you believe that your service is properly configured and this error is wrong, please [open an issue](https://astro.build/issues/).
*/
InvalidImageService: {
title: 'Error while loading image service',
title: 'Error while loading image service.',
code: 3023,
message:
'There was an error loading the configured image service. Please see the stack trace for more information.',
Expand Down Expand Up @@ -530,6 +530,72 @@ See https://docs.astro.build/en/guides/server-side-rendering/ for more informati
)} are supported for optimization.`,
hint: "If you do not need optimization, using an `img` tag directly instead of the `Image` component might be what you're looking for.",
},
/**
* @docs
* @see
* - [Assets (Experimental)](https://docs.astro.build/en/guides/assets/)
* @description
* The `src` property, either on `getImage`'s first parameter or on the Image component needs to be either an image that has been ESM imported or a string.
Princesseuh marked this conversation as resolved.
Show resolved Hide resolved
*
* ```astro
* ---
* import { Image } from "astro:assets";
* import myImage from "../assets/my_image.png";
* ---
*
* <Image src={myImage} alt="..." />
* <Image src="https://example.com/logo.png" width={300} height={300} alt="..." />
* ```
*
* In most cases, this error happens when the value passed to `src` is undefined.
*/
ExpectedImage: {
title: 'Expected src to be an image.',
code: 3026,
message: (options: string) =>
`Expected \`src\` property to be either an ESM imported image or a string with the path of a remote image. Received \`${options}\`.`,
hint: 'This error can often happen because of a wrong path. Make sure the path to your image is correct.',
},
/**
* @docs
* @see
* - [Assets (Experimental)](https://docs.astro.build/en/guides/assets/)
* @description
* `getImage()`'s first parameter should be an object with the different properties to apply to your image.
*
* ```ts
* import { getImage } from "astro:assets";
* import myImage from "../assets/my_image.png";
*
* const optimizedImage = await getImage({src: myImage, width: 300, height: 300});
* ```
*
* In most cases, this error happens because parameters were passed directly instead of inside an object.
*/
ExpectedImageOptions: {
title: 'Expected image options.',
code: 3027,
message: (options: string) =>
`Expected getImage() parameter to be an object. Received \`${options}\`.`,
},
/**
* @docs
* @see
* - [Assets (Experimental)](https://docs.astro.build/en/guides/assets/)
* @description
* Astro could not find an image you included in your Markdown content. Usually, this is simply caused by a typo in the path.
*
* Images in Markdown are relative to the current file, as such, to refer to an image that is directly next to the `.md` file, your path should start with `./`
Princesseuh marked this conversation as resolved.
Show resolved Hide resolved
*/
MarkdownImageNotFound: {
title: 'Image not found.',
code: 3028,
message: (imagePath: string, fullImagePath: string | undefined) =>
`Could not find requested image \`${imagePath}\`${
fullImagePath ? ` at \`${fullImagePath}\`.` : '.'
}`,
hint: 'This is often caused by a typo in the image path. Please make sure the file exists.',
Princesseuh marked this conversation as resolved.
Show resolved Hide resolved
},
// 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
11 changes: 6 additions & 5 deletions packages/astro/src/core/errors/overlay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ const style = /* css */ `
--shiki-token-function: #4ca48f;
--shiki-token-string-expression: #9f722a;
--shiki-token-punctuation: #ffffff;
--shiki-token-link: #ee0000;
--shiki-token-link: #9f722a;
}

:host(.astro-dark) {
Expand Down Expand Up @@ -131,9 +131,9 @@ const style = /* css */ `
--shiki-token-function: #90f4e3;
--shiki-token-string-expression: #f4cf90;
--shiki-token-punctuation: #ffffff;
--shiki-token-link: #ee0000;
--shiki-token-link: #f4cf90;
}

#theme-toggle-wrapper{
position: relative;
display: inline-block
Expand All @@ -144,7 +144,7 @@ const style = /* css */ `
right: 3px;
margin-top: 3px;
}

.theme-toggle-checkbox {
opacity: 0;
position: absolute;
Expand Down Expand Up @@ -250,7 +250,7 @@ const style = /* css */ `
padding: 12px;
margin-top: 12px;
}

#theme-toggle-wrapper > div{
position: absolute;
right: 22px;
Expand Down Expand Up @@ -372,6 +372,7 @@ const style = /* css */ `
background-color: var(--border);
padding: 4px;
border-radius: var(--roundiness);
white-space: nowrap;
}

.link {
Expand Down
34 changes: 29 additions & 5 deletions packages/astro/src/vite-plugin-markdown/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,17 @@ import {
InvalidAstroDataError,
safelyGetAstroData,
} from '@astrojs/markdown-remark/dist/internal.js';
import fs from 'fs';
import matter from 'gray-matter';
import fs from 'node:fs';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import type { Plugin } from 'vite';
import { normalizePath } from 'vite';
import type { AstroSettings } from '../@types/astro';
import { AstroError, AstroErrorData, MarkdownError } from '../core/errors/index.js';
import type { LogOptions } from '../core/logger/core.js';
import { warn } from '../core/logger/core.js';
import { isMarkdownFile } from '../core/util.js';
import { isMarkdownFile, rootRelativePath } from '../core/util.js';
import type { PluginMetadata } from '../vite-plugin-astro/types.js';
import { escapeViteEnvReferences, getFileInfo } from '../vite-plugin-utils/index.js';

Expand Down Expand Up @@ -58,6 +59,10 @@ const astroServerRuntimeModulePath = normalizePath(
fileURLToPath(new URL('../runtime/server/index.js', import.meta.url))
);

const astroErrorModulePath = normalizePath(
fileURLToPath(new URL('../core/errors/index.js', import.meta.url))
);

export default function markdown({ settings, logging }: AstroPluginOptions): Plugin {
return {
enforce: 'pre',
Expand All @@ -81,14 +86,15 @@ export default function markdown({ settings, logging }: AstroPluginOptions): Plu

let html = renderResult.code;
const { headings } = renderResult.metadata;
let imagePaths: { raw: string; absolute: string }[] = [];
let imagePaths: { raw: string; resolved: string }[] = [];
if (settings.config.experimental.assets) {
let paths = (renderResult.vfile.data.imagePaths as string[]) ?? [];
imagePaths = await Promise.all(
paths.map(async (imagePath) => {
return {
raw: imagePath,
absolute: (await this.resolve(imagePath, id))?.id ?? imagePath,
resolved:
(await this.resolve(imagePath, id))?.id ?? path.join(path.dirname(id), imagePath),
};
})
);
Expand All @@ -113,17 +119,35 @@ export default function markdown({ settings, logging }: AstroPluginOptions): Plu
const code = escapeViteEnvReferences(`
import { Fragment, jsx as h } from ${JSON.stringify(astroJsxRuntimeModulePath)};
import { spreadAttributes } from ${JSON.stringify(astroServerRuntimeModulePath)};
import { AstroError, AstroErrorData } from ${JSON.stringify(astroErrorModulePath)};

${layout ? `import Layout from ${JSON.stringify(layout)};` : ''}
${settings.config.experimental.assets ? 'import { getImage } from "astro:assets";' : ''}

export const images = {
${imagePaths.map(
(entry) =>
`'${entry.raw}': await getImage({src: (await import("${entry.absolute}")).default})`
`'${entry.raw}': await getImageSafely((await import("${entry.raw}")).default, "${
entry.raw
}", "${rootRelativePath(settings.config, entry.resolved)}")`
)}
}

async function getImageSafely(imageSrc, imagePath, resolvedImagePath) {
if (!imageSrc) {
throw new AstroError({
...AstroErrorData.MarkdownImageNotFound,
message: AstroErrorData.MarkdownImageNotFound.message(
imagePath,
resolvedImagePath
),
location: { file: "${id}" },
});
}

return await getImage({src: imageSrc})
}

function updateImageReferences(html) {
return html.replaceAll(
/__ASTRO_IMAGE_=\"(.+)\"/gm,
Expand Down
Loading