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

Content collection loader examples are inconsistent #10300

Closed
anaxite opened this issue Dec 8, 2024 · 11 comments · Fixed by #10370
Closed

Content collection loader examples are inconsistent #10300

anaxite opened this issue Dec 8, 2024 · 11 comments · Fixed by #10370
Labels
improve or update documentation Enhance / update existing documentation (e.g. add example, improve description, update for changes)

Comments

@anaxite
Copy link
Contributor

anaxite commented Dec 8, 2024

📚 Subject area/topic

Content collections

📋 Page(s) affected (or suggested, for new content)

📋 Description of content that is out-of-date or incorrect

The Astro v5 loader examples have a different file pattern depending on the page. The different examples are confusing, and it's not sure which pattern is the recommended default.

For example, the upgrade guide shows a loader pattern that excludes underscore:

loader: glob({ pattern: '**/[^_]*.md', base: "./src/data/blog" }),

The astro:content reference removes the underscore check:

loader: glob({ pattern: '**/*.md', base: './src/data/blog' }),

The MDX integration guide adds a backslash:

loader: glob({ pattern: "**\/*.{md,mdx}", base: "./src/blog" }),

I am willing to submit a PR if I know the recommended format.

🖥️ Reproduction in StackBlitz (if reporting incorrect content or code samples)

No response

@anaxite anaxite added the improve or update documentation Enhance / update existing documentation (e.g. add example, improve description, update for changes) label Dec 8, 2024
@ArmandPhilippot
Copy link
Member

Hi, thanks for reporting this and volunteering to fix the docs!

Regarding the backslash:
It's an escape character which is not needed in this case. We tried to clean them in #10209 and #10213 but apparently we forgot some patterns... (well, I say we but I mean I 😅 ). So, yes, we should remove it to be consistent!

Regarding the consistency between the patterns:
Which one to use depends on your needs and I don't think we have a recommended default. There is not one more just than the other and you could even use another one in your project.
As you can see in the "Content collections" page, we also use pattern: ['*.md', '!voyager-*'] to illustrate that a pattern could be something else.
And the base property is not always the same because there too, it is customizable, you can put your files in the folder of your choice.
So I don't know what should be done here...

However, I agree, the pattern is something that should be better documented to reduce confusion and it's related to #10147.

@anaxite
Copy link
Contributor Author

anaxite commented Dec 8, 2024

Does the router exclude files with underscores even if the pattern is '**/*.md'?
If so, this may be the simplest pattern to suggest.
I could change most instances to this (except where you need to illustrate the pattern is different).

If the router behavior depends on this pattern, that complicates the issue.

@ArmandPhilippot
Copy link
Member

I don't think so, and that's probably why this pattern is used in the upgrade guide. So I think it's best to leave it there!

If patterns were to be standardized, it would probably be safer to use '**/[^_]*.md'. But I'm not sure it's necessary... because as you said the other pattern is simpler and it's maybe better to stay simple in some examples.
But you can wait for other opinions!

@anaxite
Copy link
Contributor Author

anaxite commented Dec 8, 2024

I think I know a way to make it clearer.

  1. Edit the upgrade guide to make it clear that underscores _ in front of files are no longer excluded automatically. This is a behavior change that is good to signal.
  2. Add both examples in the upgrade guide ("if you want to keep the old behavior", set this pattern or something).
  3. Edit the Content Collections page to explain the pattern, instead of mentioning an automatic underscore behavior.

I can see if anyone else has a different thought as well.

@ArmandPhilippot
Copy link
Member

underscores _ in front of files are no longer excluded automatically

Just a precision: It is still the case, but this applies to files in src/pages. When I was saying I don't think so I meant for files coming from content collections. I don't know if they were excluded before, I never used this pattern outside of src/pages. When your content comes from a content collection, your page template uses something like src/pages/posts/[id].astro: no underscore! So it seems to me that files with an underscore in your content collection are not excluded.

Regarding your suggestions:

  1. I think a comment above the pattern in the upgrade guide explaining that this pattern allows you to exclude files starting with an underscore could make sense and could help clear the confusion.
  2. I'm not sure adding two examples will help... Which one to put first? I think that could add another kind of confusion.
  3. I think what would make sense here is that once the pattern is better documented (still Improve built-in loader docs #10147), we add a ReadMore targeting that page.

I suggested to wait for another opinion because I am not a maintainer here, just a contributor so my opinion doesn't count more than yours and like I said I don't know what's best here! I just wanted to point out that standardization may not be that simple. It's still the weekend, I'm sure other people will be able to chime in later. 😉

In any case, a PR to remove the unnecessary escape character is very welcomed! And, thanks for your ideas! Let's see what others think!

@sarah11918
Copy link
Member

Firstly, thanks for catching the \ that we missed!

Just chiming in here that Armand has given a great summary of the position and why "consistent" examples are problematic:

  • if we show the same thing all the time then it can be interpreted as "the" pattern, when really, the pattern is meant to be extremely flexible, and showing it slightly different in places helps get that idea across. Removing the unnecessary character is helpful, but it's not super important that every example look the same, and in fact, it's probably more helpful if it doesn't so people realize that this is entirely custom!

  • If you compare v4 docs to the current content colletions docs, you'll see that the part about using an underscore to prevent pages has been removed for current docs. The previous Content Collections API had the benefit of only working with local entries, so using our underscore trick that matches our src/pages behaviour was built in at some point for convenience. With the new API, we can't know what your files look like as stored in some other source, nor that that system expects/supports file names working that way since it is our own convention. (Maybe that underscore is meaningful to a CMS you're using, for example, but doesn't mean "exclude from build.")

I'll take a peek at the upgrade guide and see where a very quick mention of this change might make sense. The part about excluding draft posts by filtering queries was present even in the old content collections docs, so I don't think we need to have an example of going back to old behavior. It's merely that the shortcut convenience no longer works, and in fact was possible to control more finely with the suggested filtering method anyway. But you're right, if this is a behaviour people were relying on previously, it makes sense to point it out in the upgrade guide! And, we could additionally maybe make it even more explicit on the routing page that this pattern is for only page/endpoint/route files within src/pages/.

I can see the PR for excluding the backslash, and I'll get that merged in shortly!

@ArmandPhilippot
Copy link
Member

It seems that the new behavior is not clear to everyone. There is a new issue in the core repo about this, see: withastro/astro#12730.

In Exclude pages, maybe we could:

  1. Update the first sentence (basically what you said)
-You can exclude pages or directories from being built by prefixing their names with an underscore (_).
+You can exclude pages or directories within `src/pages` from being built by prefixing their names with an underscore (_).
  1. Add a new paragraph under the code snippet related to Content Collections
To exclude pages from your content collections, you need to [adapt the `pattern` of your glob loader](https://docs.astro.build/en/guides/content-collections/#built-in-loaders).

This doesn't show exactly which pattern to use, but it might help to better understand how it works and where to look for...

@sarah11918
Copy link
Member

sarah11918 commented Dec 13, 2024

Yes, I think those are great changes! It is probably also still worth a shout out in the upgrade guide that the analogous behaviour that had been built in to the old collections API for local content files has been removed, and you can either update your pattern accordingly or filter your results after fetching.

(And just taking this moment to point out how "changing content collections" obviously doesn't just mean "updating the content collections page in docs"! We tried to anticipate as much as we could and update the entire docs for all these v5 changes, but there are a lot of interconnected pieces. Thank you everyone for helping us spot what we missed, participating in improving it, and having the patience while we get eventually get there! 💜 )

@anaxite
Copy link
Contributor Author

anaxite commented Dec 13, 2024

Sounds good. Shall I have a go at some of these updates?

@sarah11918
Copy link
Member

sarah11918 commented Dec 13, 2024

@anaxite I would love that! I think incorporating Armand's suggestions and running with those make sense. We can edit/polish final details in the PR. Once you start, I'll add something to the PR on the upgrade guide. (That one I'll probably want to word myself, since there's already so much info on that page I'll want to fit this in as compactly as I can.)

@anaxite
Copy link
Contributor Author

anaxite commented Dec 15, 2024

@sarah11918 Alright, PR is started! I'm leaving it in draft mode until I've had more of a look.

I started in the routing page with Armand's suggestion, and updated the file tree block at the bottom to make it look a bit more different to content collections.

sarah11918 added a commit to anaxite/withastro.docs that referenced this issue Dec 17, 2024
sarah11918 added a commit to anaxite/withastro.docs that referenced this issue Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improve or update documentation Enhance / update existing documentation (e.g. add example, improve description, update for changes)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants