Skip to content

feat: add cache hint support to live loaders #13767

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.

Sign up for GitHub

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

Merged
Parent: feat: live content collections
merged 3 commits into from
May 16, 2025
Merged

feat: add cache hint support to live loaders #13767

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b2ea0a0
feat: add cache hint support to live loaders
ascorbic May 8, 2025
4316005
Tidy loader in fixture
ascorbic May 8, 2025
9c8beb8
Error description
ascorbic May 15, 2025
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
36 changes: 36 additions & 0 deletions packages/astro/src/content/runtime.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,11 @@ export function createCollectionToGlobResultMap({
return collectionToGlobResultMap;
}

const cacheHintSchema = z.object({
tags: z.array(z.string()).optional(),
maxAge: z.number().optional(),
});

async function parseLiveEntry(
entry: LiveDataEntry,
schema: z.ZodType,
Expand All @@ -137,6 +142,21 @@ async function parseLiveEntry(
),
});
}
if (entry.cacheHint) {
const cacheHint = cacheHintSchema.safeParse(entry.cacheHint);

if (!cacheHint.success) {
throw new AstroError({
...AstroErrorData.InvalidCacheHintError,
message: AstroErrorData.InvalidCacheHintError.message(
collection,
entry.id,
cacheHint.error,
),
});
}
entry.cacheHint = cacheHint.data;
}
return {
...entry,
data: parsed.data,
Expand Down Expand Up @@ -184,6 +204,22 @@ export function createGetCollection({
);
}

if (response.cacheHint) {
const cacheHint = cacheHintSchema.safeParse(response.cacheHint);

if (!cacheHint.success) {
throw new AstroError({
...AstroErrorData.InvalidCacheHintError,
message: AstroErrorData.InvalidCacheHintError.message(
collection,
undefined,
cacheHint.error,
),
});
}
response.cacheHint = cacheHint.data;
}

return {
...response,
collection,
Expand Down
35 changes: 31 additions & 4 deletions packages/astro/src/core/errors/errors-data.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1614,8 +1614,9 @@ export const InvalidContentEntryDataError = {
title: 'Content entry data does not match schema.',
message(collection: string, entryId: string, error: ZodError) {
return [
`**${String(collection)} → ${String(entryId)}** data does not match collection schema.`,
...error.errors.map((zodError) => zodError.message),
`**${String(collection)} → ${String(entryId)}** data does not match collection schema.\n`,
...error.errors.map((zodError) => ` **${zodError.path.join('.')}**: ${zodError.message}`),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When adding the new error I noticed that these were missing the path, which could cause unhelpful errors

Copy link
Member

Choose a reason for hiding this comment

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

2 things:

  1. Why do you need String(...) when the type is already string?
  2. I think the formatting of the error should happen where the throw happens rather than inside the error data, see https://github.com/withastro/astro/blob/main/packages/astro/src/core/errors/README.md#shape

Copy link
Member

Choose a reason for hiding this comment

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

Can you provide two screenshots: one before and after the change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a very old error - I just added the zodError.path.join('.') bit. I followed the same pattern for the one I added. You're right the string cast isn't needed, but I think that standardising the formatting of the zod error does make sense

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before:
image

After:
image

See the field name title has been added

Copy link
Member

Choose a reason for hiding this comment

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

Ah sorry I didn't notice it was an old one

'',
].join('\n');
},
hint: 'See https://docs.astro.build/en/guides/content-collections/ for more information on content schemas.',
Expand Down Expand Up @@ -1665,13 +1666,39 @@ export const ContentEntryDataError = {
title: 'Content entry data does not match schema.',
message(collection: string, entryId: string, error: ZodError) {
return [
`**${String(collection)} → ${String(entryId)}** data does not match collection schema.`,
...error.errors.map((zodError) => zodError.message),
`**${String(collection)} → ${String(entryId)}** data does not match collection schema.\n`,
...error.errors.map((zodError) => ` **${zodError.path.join('.')}**: ${zodError.message}`),
'',
].join('\n');
},
hint: 'See https://docs.astro.build/en/guides/content-collections/ for more information on content schemas.',
} satisfies ErrorData;

/**
* @docs
* @message
* **Example error message:**<br/>
* **blog** → **post** returned an invalid cache hint.<br/>
* **maxAge**: Expected number, received string
* @description
* The loader for a live content collection returned an invalid cache hint.
* Make sure that if you are using a cache hint, it is a valid object.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Make sure that if you are using a cache hint, it is a valid object.
* Make sure that your cache hint object is properly formatted.

Just two quick things jump to mind:

  • if an invalid cache hint was received, then presumably they are using a cache hint? So we can avoid that extra wording? (Or at least, safe enough? If someone reacts, "Wait, I didn't think I provided a cache hint..." then this message is also helpful to show them where to look for their error.)

  • "It's invalid. Make it valid" could be a tiny bit more helpful if instead of "make it valid", we can prompt with what's likely to make it invalid, or at the very least, not duplicate "valid" which can feel like a "duh!" kind of error message. 😅 Both tags and maxAge appear to be optional, so it's probably not e.g. that one is missing, but something like that the type or syntax was wrong?

The other alternative is to be more descriptive about the first sentence, like:

The loader for a live content collection could not determine a caching strategy (/how to cache your content). Make sure that your cache hint is a valid object.

I think either change to avoid duplication feels helpful!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose what I was trying to get at was "Your hint is invalid. Fix it or remove it", because it's optional

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about Make sure that cacheHint is an object with the correct properties, or is undefined.

* @see
* - [Experimental live content](https://astro.build/en/reference/experimental-flags/live-content/)
*/
export const InvalidCacheHintError = {
name: 'InvalidCacheHintError',
title: 'Invalid cache hint.',
message(collection: string, entryId: string | undefined, error: ZodError) {
return [
`**${String(collection)}${entryId ? ` → ${String(entryId)}` : ''}** returned an invalid cache hint.\n`,
...error.errors.map((zodError) => ` **${zodError.path.join('.')}**: ${zodError.message}`),
'',
].join('\n');
},
hint: 'See https://docs.astro.build/en/reference/experimental-flags/live-content/ for more information.',
} satisfies ErrorData;

/**
* @docs
* @message
Expand Down
12 changes: 11 additions & 1 deletion packages/astro/src/types/public/content.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,16 +125,26 @@ export interface DataEntryType {

export type GetDataEntryInfoReturnType = { data: Record<string, unknown>; rawData?: string };

export interface CacheHint {
/** Cache tags */
tags?: Array<string>;
/** Maximum age of the response in seconds */
maxAge?: number;
}

export interface LiveDataEntry<TData extends Record<string, unknown> = Record<string, unknown>> {
/** The ID of the entry. Unique per collection. */
id: string;
/** The parsed entry data */
data: TData;
/** A hint for how to cache this entry */
cacheHint?: CacheHint;
}

export interface LiveDataCollection<
TData extends Record<string, unknown> = Record<string, unknown>,
> {
entries: Array<LiveDataEntry<TData>>;
// TODO: pagination etc.
/** A hint for how to cache this collection. Individual entries can also have cache hints */
cacheHint?: CacheHint;
}
15 changes: 14 additions & 1 deletion packages/astro/test/fixtures/live-loaders/src/live.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,24 @@ const entries = {
const loader: LiveLoader<Entry, { id: keyof typeof entries }> = {
name: 'test-loader',
loadEntry: async (context) => {
return entries[context.filter.id] || null;
if(!entries[context.filter.id]) {
return;
}
return {
...entries[context.filter.id],
cacheHint: {
tags: [`page:${context.filter.id}`],
maxAge: 60,
},
};
},
loadCollection: async (context) => {
return {
entries: Object.values(entries),
cacheHint: {
tags: ['page'],
maxAge: 60,
},
};
},
};
Expand Down