Implement legacy collections using glob #11976
Conversation
🦋 Changeset detectedLatest commit: 4eae23f The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
pkg: astro
ascorbic
changed the title
There was a problem hiding this comment.
Thanks @ascorbic ! I've left a bunch of comments here, most of which boil down to that because this is a major feature people know and use, I think more of this could be geared towards helping people update with actions to follow to keep their projects in working order. You'll see what I mean!
Some are suggestions, some are questions to prompt towards the kind of guidance I'd expect to find here!
.changeset/quick-onions-leave.md
Outdated
|
|
||
| While these emulate most of the features of legacy collections, they have these differences: | ||
|
|
||
| - Implicit collections for folders in `src/content` are only defined if no other collections use content layer. If no content layer collections are defined, and there are folders in `src/content` that don't match collections in `src/content/config.ts` then collections will be auto-generated for them. This is not recommended, and a warning will be logged that this is deprecated. A collection should always be defined in `config.ts`. For legacy collections these can just be empty declarations: e.g.`const blog = defineCollection({})`. |
There was a problem hiding this comment.
I am honestly having a hard time making sense of this from a usage/what happens to my existing project standpoint:
- I don't know what an "implicit" collection is. This phrase hasn't been introduced.
- Why would I have existing collections in
src/content/that do not match my collections insrc/content/config.ts. Wouldn't that have already caused a problem for me? - It feels like we're describing a slightly convoluted thing that will happen, then saying it's not recommended? We could be a lot more explicit about telling people what they need to do, vs describing things that might be done to them.
I think the "what would make this a problem for someone to warrant even saying anything here?" question is the biggest one I have.
I get that you can now autogenerate collection schemas in certain cases, but existing legacy projects probably already have schemas? (So focusing on ones that don't is distracting when that's not most people's experience.) Is not having a schema the only time this surfaces as a problem? And if so, what fixes that: not having a mix of both old and new collections?
It is also OK to instruct people that "If condition X applies to you, you must now do X" if that is the happy path. These collections will be phased out eventually anyway. They don't need to know workarounds, or things that might be OK for a while. Just decide what you want them to do and tell them they need to do it. If other stuff happens to be possible, that's fine. This is to help people deal with a breaking change in the most straightforward way possible.
For example, if you want to straight up say that you simply can't mix new and legacy collections in a project anymore: that's fine! Say that this is the point at which you either update all your collections accordingly, or you enable the legacy flag if you're unable to do so yet. That is what the flag is for! That is very easy advice to follow, and doesn't require parsing a lot of "if" cases.
There was a problem hiding this comment.
Thanks! I've rewritten this whole section. The reason I included this behaviour at all is because I do see a lot of people using these auto-generated collections without config. I didn't want everything to break as they upgraded, and I don;t want them to just immediately enabled the legacy flag! Adding this fallback behaviour helped get us to working with most sites, while also loudly warning that they should upgrade.
The only reason I've included the fact that it exists in the docs is because of the scenario where it works, but then stops working if they add new collections. I want the behaviour to be:
- your config is untouched, most things will Just Work with no changes
- you start to upgrade, you then should add the one-liner for all of your previously auto-generated collections
I'd love your thoughts on how best to communicate this (or if you think this is totally the wrong approach anyway)
Co-authored-by: Sarah Rainsberger <[email protected]>
|
Thanks, @sarah11918! I've made lots of changes based on your feedback. |
There was a problem hiding this comment.
Looks great @ascorbic ! Thank you for taking the time to do a little rework here. I think it really does make a difference!
Just some tiny formatting things, and one suggestion to consider for the most problematic of the issues (which is now handled spot on, I think!) but I leave it to you what you feel makes the most sense!
I also left a suggestion for the flag's config reference, and am noting that an error was mentioned but I don't see one here in this PR, just fyi!
Co-authored-by: Sarah Rainsberger <[email protected]>
* feat: support pattern arrays with glob * wip * feat: emulate legacy content collections * Fixes * Lint * Correctly handle legacy data * Fix tests * Switch flag handling * Fix warnings * Add layout warning * Update fixtures * More tests! * Handle empty md files * Lockfile * Dedupe name * Handle data ID unslug * Fix e2e * Clean build * Clean builds in tests * Test fixes * Fix test * Fix typegen * Fix tests * Fixture updates * Test updates * Update changeset * Test * Remove wait in test * Handle race condition * Lock * chore: changes from review * Handle folders without config * lint * Fix test * Update wording for auto-collections * Delete legacyId * Sort another fixture * Rename flag to `legacy.collections` * Apply suggestions from code review Co-authored-by: Sarah Rainsberger <[email protected]> * Changes from review * Apply suggestions from code review Co-authored-by: Sarah Rainsberger <[email protected]> * lockfile * lock --------- Co-authored-by: Sarah Rainsberger <[email protected]>
Changes
Breaking change to legacy content collections
Implements legacy content and data collections using
glob()loader. By default, collections that use the the old types (content or data) are now implemented using the glob loader, with extra backward-compat handling. This includes any collection without aloaderdefined.Any legacy content collections are handled like this:
globloader collection is defined, with patterns that match the previous handling (matchessrc/content/<collection name>/**/*.mdand other content extensions depending on installed integrations, with underscore-prefixed files and folders ignored)slugfield is added with the same format as beforerender()method is added to the entry, so they can be called usingentry.render()getEntryBySlugis supportedLegacy data collections are handled like this:
globloader collection is defined, with patterns that match the previous handling (matchessrc/content/<collection name>/**/*{.json,.yaml}and other data extensions, with underscore-prefixed files and folders ignored)getDataEntryByIdis supportedWhile these emulate most of the features of legacy collections, they have these differences:
src/contentare only generated if no other collections use content layer. If no content layer collections are defined, and there are folders insrc/contentthat don't match collections insrc/content/config.tsthen collections will be auto-generated for them. This is not recommended, and a warning will be logged that this is deprecated and that a collection should always be defined inconfig.ts. For legacy collections these can just be empty declarations: e.g.const blog = defineCollection({}).layoutfield is not supported in Markdownimage().refine()is not supportedgetEntryis typed asstring, rather than having types for every entry.A new config flag
legacy.collectionsis added for users that need the old behavior. When set, collections insrc/contentare processed in the same way as before rather than being implemented with glob. When set, content layer collections are forbidden insrc/content, and will fail a build if defined.Testing
Lots of tests have been updated. Where possible they use the new implementation, but when they need to test unsupported features then they have the legacy flag applied.
Docs