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

Added generateId option for file loader #12308

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

marceloverdijk
Copy link

Background: withastro/roadmap#1045

Changes

  • Adds a generateId option to the file loader (similar as the glob loader already has)
  • Don't forget a changeset! pnpm exec changeset

Testing

Docs

/cc @withastro/maintainers-docs for feedback!

Copy link

changeset-bot bot commented Oct 25, 2024

⚠️ No Changeset found

Latest commit: 3ca422f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Oct 25, 2024
@ascorbic ascorbic self-assigned this Nov 2, 2024
@ascorbic
Copy link
Contributor

ascorbic commented Nov 2, 2024

This is great. Thanks! Could you add a changeset that can form the basis for updated docs?

@marceloverdijk
Copy link
Author

Sure will do it later today 👍

@github-actions github-actions bot added the semver: minor Change triggers a `minor` release label Nov 2, 2024
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This PR is blocked because it contains a minor changeset. A reviewer will merge this at the next release if approved.

@marceloverdijk
Copy link
Author

@ascorbic I've added the changeset, is this sufficient?

Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Thank you @marceloverdijk for the contribution!

There are three things I ask you to implement:

  • Change the changeset to be patch. Content Layer is experimental, so we can land new APIs under patch.
  • Create at least a new test to verify that your implementation works. Let me know if you need any help with that. You can start from here
    it('Returns `file()` loader collection', async () => {
  • Docs. For now our docs are here:
    * The `file()` loader creates multiple entries from a single local file. Use this when all your entries are stored in an array of objects.
    *
    * ```ts {3,8,19}
    * // src/content/config.ts
    * import { defineCollection, z } from 'astro:content';
    * import { glob, file } from 'astro/loaders';
    *
    * const blog = defineCollection({
    * // By default the ID is a slug generated from
    * // the path of the file relative to `base`
    * loader: glob({ pattern: "**\/*.md", base: "./src/data/blog" }),
    * schema: z.object({
    * title: z.string(),
    * description: z.string(),
    * pubDate: z.coerce.date(),
    * updatedDate: z.coerce.date().optional(),
    * })
    * });
    *
    * const dogs = defineCollection({
    * // The path is relative to the project root, or an absolute path.
    * loader: file("src/data/dogs.json"),
    * schema: z.object({
    * id: z.string(),
    * breed: z.string(),
    * temperament: z.array(z.string()),
    * }),
    * });
    *
    * export const collections = { blog, dogs };
    . You'll have to provide a new example with the new API

@sarah11918
Copy link
Member

Let me know if I can be of any help with docs here! 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope) semver: minor Change triggers a `minor` release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants