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

Improve errors for invalid IDs in content collections #12892

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
7 changes: 7 additions & 0 deletions .changeset/dry-dragons-shout.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'astro': patch
---

Added more verbose errors for content collections with invalid IDs.

Previously, when an entry in a content collection had an ID that wasn't a string, the error would omit the ID entirely. With this change, the error will now explicitly say that the ID has to be a string.
45 changes: 38 additions & 7 deletions packages/astro/src/content/content-layer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ import {
getEntryConfigByExtMap,
getEntryDataAndImages,
globalContentConfigObserver,
loaderReturnSchema,
safeStringify,
} from './utils.js';
import type { z } from 'zod';

export interface ContentLayerOptions {
store: MutableDataStore;
Expand All @@ -30,6 +32,12 @@ export interface ContentLayerOptions {
watcher?: FSWatcher;
}

type CollectionLoader<TData> = () =>
| Array<TData>
| Promise<Array<TData>>
| Record<string, Record<string, unknown>>
| Promise<Record<string, Record<string, unknown>>>;

export class ContentLayer {
#logger: Logger;
#store: MutableDataStore;
Expand Down Expand Up @@ -266,7 +274,7 @@ export class ContentLayer {
});

if (typeof collection.loader === 'function') {
return simpleLoader(collection.loader, context);
return simpleLoader(collection.loader as CollectionLoader<{ id: string }>, context);
}

if (!collection.loader.load) {
Expand Down Expand Up @@ -325,15 +333,38 @@ export class ContentLayer {
}

export async function simpleLoader<TData extends { id: string }>(
handler: () =>
| Array<TData>
| Promise<Array<TData>>
| Record<string, Record<string, unknown>>
| Promise<Record<string, Record<string, unknown>>>,
handler: CollectionLoader<TData>,
context: LoaderContext,
) {
const data = await handler();
const unsafeData = await handler();
const parsedData = loaderReturnSchema.safeParse(unsafeData);

if (!parsedData.success) {
const issue = parsedData.error.issues[0] as z.ZodInvalidUnionIssue;

// Due to this being a union, zod will always throw an "Expected array, received object" error along with the other errors.
// This error is in the second position if the data is an array, and in the first position if the data is an object.
const parseIssue = Array.isArray(unsafeData)
? issue.unionErrors[0]
: issue.unionErrors[1];

const error = parseIssue.errors[0];
const firstPathItem = error.path[0];

const entry = Array.isArray(unsafeData)
? unsafeData[firstPathItem as number]
: unsafeData[firstPathItem as string];

throw new AstroError({
...AstroErrorData.ContentLoaderReturnsInvalidResult,
message: AstroErrorData.ContentLoaderReturnsInvalidResult.message(context.collection, entry, error.message),
});
}

const data = parsedData.data;

context.store.clear();

if (Array.isArray(data)) {
for (const raw of data) {
if (!raw.id) {
Expand Down
55 changes: 17 additions & 38 deletions packages/astro/src/content/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ import {
} from './consts.js';
import { glob } from './loaders/glob.js';
import { createImage } from './runtime-assets.js';

/**
* Amap from a collection + slug to the local file path.
* A map from a collection + slug to the local file path.
* This is used internally to resolve entry imports when using `getEntry()`.
* @see `templates/content/module.mjs`
*/
Expand All @@ -41,10 +42,20 @@ const entryTypeSchema = z
.string({
invalid_type_error: 'Content entry `id` must be a string',
// Default to empty string so we can validate properly in the loader
})
.catch(''),
})
.catchall(z.unknown());
}),
}).passthrough();

export const loaderReturnSchema = z.union([
z.array(entryTypeSchema),
z.record(
z.string(),
z.object({
id: z.string({
invalid_type_error: 'Content entry `id` must be a string'
}).optional()
}).passthrough()
),
]);

const collectionConfigParser = z.union([
z.object({
Expand All @@ -59,39 +70,7 @@ const collectionConfigParser = z.union([
type: z.literal(CONTENT_LAYER_TYPE),
schema: z.any().optional(),
loader: z.union([
z.function().returns(
z.union([
z.array(entryTypeSchema),
z.promise(z.array(entryTypeSchema)),
z.record(
z.string(),
z
.object({
id: z
.string({
invalid_type_error: 'Content entry `id` must be a string',
})
.optional(),
})
.catchall(z.unknown()),
),

z.promise(
z.record(
z.string(),
z
.object({
id: z
.string({
invalid_type_error: 'Content entry `id` must be a string',
})
.optional(),
})
.catchall(z.unknown()),
),
),
]),
),
z.function(),
z.object({
name: z.string(),
load: z.function(
Expand Down
28 changes: 28 additions & 0 deletions packages/astro/src/core/errors/errors-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1563,6 +1563,34 @@ export const InvalidContentEntryDataError = {
hint: 'See https://docs.astro.build/en/guides/content-collections/ for more information on content schemas.',
} satisfies ErrorData;

/**
* @docs
* @message
* **Example error message:**<br/>
* The content loader for the collection **blog** returned an invalid result:<br/>
* Content entry `id` must be a string:<br/>
* {<br/>
* "id": 1,<br/>
* "title": "Hello, World!"<br/>
* }
* @description
* A content loader returned an invalid result.
louisescher marked this conversation as resolved.
Show resolved Hide resolved
* Make sure that the ID of the entry is a string.
* See the [Content collections documentation](https://docs.astro.build/en/guides/content-collections/) for more information.
*/
export const ContentLoaderReturnsInvalidResult = {
louisescher marked this conversation as resolved.
Show resolved Hide resolved
name: 'ContentLoaderReturnsInvalidResult',
title: 'Content loader returns invalid result.',
message(collection: string, entry: any, message: string) {
return [
`The content loader for the collection **${String(collection)}** returned an invalid result:`,
`${message}:`,
JSON.stringify(entry, null, 2),
].join('\n');
},
hint: 'See https://docs.astro.build/en/guides/content-collections/ for more information on content schemas.',
} satisfies ErrorData;

/**
* @docs
* @message
Expand Down
15 changes: 15 additions & 0 deletions packages/astro/test/content-collections.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,21 @@ describe('Content Collections', () => {
});
});

describe('With numbers for IDs', () => {
it('Throws the right error', async () => {
const fixture = await loadFixture({
root: './fixtures/content-collections-number-id/',
});
let error;
try {
await fixture.build({ force: true });
} catch (e) {
error = e.message;
}
assert.match(error, /Content entry `id` must be a string:/);
});
});

describe('With empty collections directory', () => {
it('Handles the empty directory correctly', async () => {
const fixture = await loadFixture({
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "@test/content-collections-number-id",
"version": "0.0.0",
"private": true,
"type": "module",
"dependencies": {
"astro": "workspace:*"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { defineCollection, z } from 'astro:content';

const data = defineCollection({
loader: async () => ([
{ id: 1, title: 'One!' },
{ id: 2, title: 'Two!' },
{ id: 3, title: 'Three!' },
]),
schema: z.object({
id: z.number(),
title: z.string(),
}),
});

export const collections = {
data,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
import { getEntry } from 'astro:content';
const data = await getEntry('blog', 1);
---

{JSON.stringify(data)}
7 changes: 6 additions & 1 deletion pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading