-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Data collections and references #6850
Conversation
🦋 Changeset detectedLatest commit: c8a715a 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 |
4f5503b
to
e34e28a
Compare
!preview data-collections |
This PR is blocked because it contains a |
573c4f0
to
a1abd14
Compare
!preview data-collections |
|
I'm concerned about the fact that each file format might become a new folder name is this the only folder for data that is not markdown? What about other files? |
I know you guys said you were going to support content collections. But I wonder what other formats are you going to support? I wonder whether or not other folders would be taken from people. I think from now on it's better to stop taking top level folders from people and instead just take folders in content collections. I'm saying this because with this PR four folders are taken from people. One of them being |
@louiss0 To clarify, we do not require or own an As for the proliferation of folders, that is a good concern to keep in mind. As we see it, there won't be further "collection" types beyond content and data, so I'd expect |
|
Oh, I'm pro moving the |
I'm also concerned about Markdoc. They are written here. |
fadee16
to
9cc4539
Compare
@louiss0 I'm also the active maintainer of |
That thing has problems I tried to file an issue but that was shut down by Matt. I think you are the one to solve the problems from that issue. He told me to spilt up each of the problems into different issues But I think they are too small. Here's the link to the issues #6829 (comment). |
Keep going bud. I'm in no rush to use Markdoc. I care more about Assets right now. I just decided to put my thoughts out there. I don't know how much you are going to extend it or if Remark is going to be involved but I just hope we don't lose the DX of using mdx too much. |
65aa7e1
to
b2b98b6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These error messages are super-nicely done, @bholmesdev! I would be honoured to make a mistake with my collections! Just a couple of thoughts for you to consider below!
Co-authored-by: Sarah Rainsberger <[email protected]>
Co-authored-by: Sarah Rainsberger <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried my best to review this huge PR 😄 Code looks good to me. Have a couple extra suggestions below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some TODO
in the code, but I don't understand when you aim to address them. This PR? Next PR? Maybe a mention in the description of the PR could shed some light;
There are a few places where we need to catch errors:
- I/O operations, it's minor but you never the permissions of the file system
JSON.parse
, very important. If the contents of a file are not a valid JSON object, the function will throw a runtime error.
if (entryType !== 'content' && entryType !== 'data') return; | ||
|
||
const collection = getEntryCollectionName({ contentDir, entry: pathToFileURL(filePath) }); | ||
if (!collection) throw UnexpectedLookupMapError; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love it! ❤️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Learn from the best ❤️
* See withastro/astro#7607 for the most recent exported types * See withastro/astro#6850 for `reference()` * See `packages/astro/types/content.d.ts` for `schema` type (might need a refactor) * `getDataEntryById` is missing but since `getEntryBySlug` is announced as deprecated I don't know if we should add it (so no change here)
* See withastro/astro#7607 for the most recent exported types * See withastro/astro#6850 for `reference()` * See `packages/astro/types/content.d.ts` for `schema` type (might need a refactor) * `getDataEntryById` is missing but since `getEntryBySlug` is announced as deprecated I don't know if we should add it (so no change here)
Demo
Visual.Studio.Code.-.config.ts.with-data.-.2.May.2023.mp4
Implementation for the data collection and collection reference RFCs: withastro/roadmap#569
Changes
Introducing data collections! These are JSON-based collections you can query throughout your app, and "reference" from existing content collections.
src/data/
directory for storing collections of JSON entries. This uses the same file-based conventions as content collections.type: 'data'
property todefineCollection
to configure a data collection from yoursrc/content/config.ts
.getEntry()
. Data collections do not have slugs, so we cannot use the existinggetEntryBySlug()
helper.reference()
extension function to any configured collection. This can be used like a table join in a database, referencing other collections by key.Testing
sync
Docs
withastro/docs#3233