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: better errors for images #8536

Merged
merged 4 commits into from
Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from all 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/slow-mirrors-provide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Improved error messages around `astro:assets`
4 changes: 2 additions & 2 deletions packages/astro/src/assets/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,12 @@ export async function getImage(

const service = await getConfiguredImageService();

// If the user inlined an import, something fairly common especially in MDX, await it for them
// If the user inlined an import, something fairly common especially in MDX, or passed a function that returns an Image, await it for them
const resolvedOptions: ImageTransform = {
...options,
src:
typeof options.src === 'object' && 'then' in options.src
? (await options.src).default
? (await options.src).default ?? (await options.src)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a small behaviour change to support using non-eager import.meta.glob better. Saw some users confused that they couldn't pass the result of it to Image without awaiting multiple times and calling multiple functions in some cases. This was particularly relevant in MDX where you can't await everything the same way as in Astro files

: options.src,
};

Expand Down
15 changes: 11 additions & 4 deletions packages/astro/src/assets/services/service.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { AstroConfig } from '../../@types/astro.js';
import { AstroError, AstroErrorData } from '../../core/errors/index.js';
import { joinPaths } from '../../core/path.js';
import { isRemotePath, joinPaths } from '../../core/path.js';
import { VALID_SUPPORTED_FORMATS } from '../consts.js';
import { isESMImportedImage, isRemoteAllowed } from '../internal.js';
import type { ImageOutputFormat, ImageTransform } from '../types.js';
Expand Down Expand Up @@ -126,13 +126,20 @@ export const baseService: Omit<LocalImageService, 'transform'> = {
if (!options.src || (typeof options.src !== 'string' && typeof options.src !== 'object')) {
throw new AstroError({
...AstroErrorData.ExpectedImage,
message: AstroErrorData.ExpectedImage.message(JSON.stringify(options.src)),
message: AstroErrorData.ExpectedImage.message(
JSON.stringify(options.src),
typeof options.src,
JSON.stringify(options, (_, v) => (v === undefined ? null : v))
),
});
}

if (!isESMImportedImage(options.src)) {
// User passed an `/@fs/` path instead of the full image.
if (options.src.startsWith('/@fs/')) {
// User passed an `/@fs/` path or a filesystem path instead of the full image.
if (
options.src.startsWith('/@fs/') ||
(!isRemotePath(options.src) && !options.src.startsWith('/'))
) {
throw new AstroError({
...AstroErrorData.LocalImageUsedWrongly,
message: AstroErrorData.LocalImageUsedWrongly.message(options.src),
Expand Down
12 changes: 6 additions & 6 deletions packages/astro/src/core/errors/dev/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,21 +227,21 @@ export function getDocsForError(err: ErrorWithMetadata): string | undefined {
* Render a subset of Markdown to HTML or a CLI output
*/
export function renderErrorMarkdown(markdown: string, target: 'html' | 'cli') {
const linkRegex = /\[(.+)\]\((.+)\)/gm;
const linkRegex = /\[([^\[]+)\]\((.*)\)/gm;
const boldRegex = /\*\*(.+)\*\*/gm;
const urlRegex = /(\b(https?|ftp):\/\/[-A-Z0-9+&@#\\/%?=~_|!:,.;]*[-A-Z0-9+&@#\\/%=~_|])/gim;
const urlRegex = / (\b(https?|ftp):\/\/[-A-Z0-9+&@#\\/%?=~_|!:,.;]*[-A-Z0-9+&@#\\/%=~_|])/gim;
Copy link
Member Author

@Princesseuh Princesseuh Sep 13, 2023

Choose a reason for hiding this comment

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

A small regression fix in this regex. I changed it not too long ago and it broke some links in subtle ways, this fixes that

const codeRegex = /`([^`]+)`/gim;

if (target === 'html') {
return escape(markdown)
.replace(linkRegex, `<a href="$2" target="_blank">$1</a>`)
.replace(boldRegex, '<b>$1</b>')
.replace(urlRegex, ' <a href="$1" target="_blank">$1</a> ')
.replace(urlRegex, ' <a href="$1" target="_blank">$1</a>')
.replace(codeRegex, '<code>$1</code>');
} else {
return markdown
.replace(linkRegex, (fullMatch, m1, m2) => `${bold(m1)} ${underline(m2)}`)
.replace(urlRegex, (fullMatch) => ` ${underline(fullMatch.trim())} `)
.replace(boldRegex, (fullMatch, m1) => `${bold(m1)}`);
.replace(linkRegex, (_, m1, m2) => `${bold(m1)} ${underline(m2)}`)
.replace(urlRegex, (fullMatch) => ` ${underline(fullMatch.trim())}`)
.replace(boldRegex, (_, m1) => `${bold(m1)}`);
}
}
24 changes: 17 additions & 7 deletions packages/astro/src/core/errors/errors-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -594,9 +594,9 @@ export const PrerenderDynamicEndpointPathCollide = {
export const ExpectedImage = {
name: 'ExpectedImage',
title: 'Expected src to be an image.',
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.',
message: (src: string, typeofOptions: string, fullOptions: string) =>
`Expected \`src\` property for \`getImage\` or \`<Image />\` to be either an ESM imported image or a string with the path of a remote image. Received \`${src}\` (type: \`${typeofOptions}\`).\n\nFull serialized options received: \`${fullOptions}\`.`,
hint: "This error can often happen because of a wrong path. Make sure the path to your image is correct. If you're passing an async function, make sure to call and await it.",
} satisfies ErrorData;
/**
* @docs
Expand Down Expand Up @@ -712,12 +712,15 @@ export const LocalsNotAnObject = {
'`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`.',
} satisfies ErrorData;

/**
* @docs
* @see
* - [Images](https://docs.astro.build/en/guides/images/)
* @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.
* 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 string of a filepath.
*
* For local images from content collections, you can use the [image() schema helper](https://docs.astro.build/en/guides/images/#images-in-content-collections) to resolve the images.
*
* ```astro
* ---
Expand All @@ -728,15 +731,22 @@ export const LocalsNotAnObject = {
* <!-- 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. -->
* <!-- GOOD: `src` is a URL. -->
* <Image src="https://example.com/my_image.png" alt="Cool image" />
*
* <!-- BAD: `src` is an image's `src` path instead of the full image object. -->
* <Image src={myImage.src} alt="Cool image" />
*
* <!-- BAD: `src` is a string filepath. -->
* <Image src="../my_image.png" alt="Cool image" />
* ```
*/
export const LocalImageUsedWrongly = {
name: 'LocalImageUsedWrongly',
title: 'ESM imported images must be passed as-is.',
title: 'Local images must be imported.',
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}\`.`,
`\`Image\`'s and \`getImage\`'s \`src\` parameter must be an imported image or an URL, it cannot be a string filepath. Received \`${imageFilePath}\`.`,
hint: 'If you want to use an image from your `src` folder, you need to either import it or if the image is coming from a content collection, use the [image() schema helper](https://docs.astro.build/en/guides/images/#images-in-content-collections) See https://docs.astro.build/en/guides/images/#src-required for more information on the `src` property.',
} satisfies ErrorData;

/**
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/src/core/errors/overlay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ const style = /* css */ `
#message-content,
#hint-content {
white-space: pre-wrap;
line-height: 24px;
line-height: 26px;
flex-grow: 1;
}

Expand Down Expand Up @@ -369,7 +369,7 @@ const style = /* css */ `
#message-hints code {
font-family: var(--font-monospace);
background-color: var(--border);
padding: 4px;
padding: 2px 4px;
border-radius: var(--roundiness);
white-space: nowrap;
}
Expand Down