feat: add cache hint support to live loaders #13767
Conversation
|
github-actions
bot
added
pkg: astro
| `**${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}`), |
There was a problem hiding this comment.
When adding the new error I noticed that these were missing the path, which could cause unhelpful errors
There was a problem hiding this comment.
2 things:
- Why do you need
String(...)when the type is alreadystring? - 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
There was a problem hiding this comment.
Can you provide two screenshots: one before and after the change?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Before:
After:
See the field name title has been added
There was a problem hiding this comment.
Ah sorry I didn't notice it was an old one
| * **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. |
There was a problem hiding this comment.
| * 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
tagsandmaxAgeappear 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!
There was a problem hiding this comment.
I suppose what I was trying to get at was "Your hint is invalid. Fix it or remove it", because it's optional
There was a problem hiding this comment.
How about Make sure that cacheHint is an object with the correct properties, or is undefined.
4 participants
Changes
Adds support for returning a
cacheHintobject with entries and collections. This can be used to suggest cache tags or ttl for the data, and in future could be integrated with route cachingTesting
Docs
Added new error for invalid cache hints