-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
feat: initial live loader implementation #13688
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
Conversation
|
| function searchLiveConfig(fs: typeof fsMod, srcDir: URL): { exists: boolean; url: URL } { | ||
| const paths = [ | ||
| 'live-content.config.mjs', | ||
| 'live-content.config.js', | ||
| 'live-content.config.mts', | ||
| 'live-content.config.ts', | ||
| ]; | ||
| return search(fs, srcDir, paths); | ||
| } |
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.
Some feedback:
content.configdisappears at the end of he build, but what about thelive.configfile? This isn't clear from the PR and the RFC.- nit: not very fond of the
live-contentname, because it could beliveContenttoo. Maybelive.config.*works better
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.
It's imported into the virtual module here, so it's included as a chunk in the build:
astro/packages/astro/src/content/vite-plugin-content-virtual-mod.ts
Lines 254 to 255 in ff5d8ce
| ? // Dynamic import so it extracts the chunk and avoids a circular import | |
| `const liveCollections = (await import(${JSON.stringify(fileURLToPath(contentPaths.liveConfig.url))})).collections;` |
Yeah, I'm not sure about the name either. I'm going to keep it like this for now. I may yet be able to use one file for them, which would be the ideal.
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.
Got it.
I'm going to leave a comment here based on my knowledge, however take it with a grain of salt because I don't know the future design of the feature. This isn't a blocker comment
I believe we will need a refactor of this code: the current content collections aren't optimised for SSR. When built, they generate a lookup table for all collections. We usually discourage the use of collections in SSR. Which means the live collections will need to be smarter I suppose, and won't need much of the code that is generated now by our internals. Essentially, if we have live collections and content collections, in SSR we should only have the relevant code that belong to live collections.
Also, since this configuration is a production file, very much like actions and middleware (that's what I understood), we should evaluate it's loading via vite and rollup.
Changes
First draft of live loaders, implementing withastro/roadmap#1151. Merging into #13685
This implementation requires the user to define the collections in
src/live-content.config.ts. It creates a new collection type calledlive, which takes a new type of loader calledLiveLoader. This has the following signature:The collection is currently typed by setting a generic for
LiveLoader. There isn't any schema implementation yet.If we're happy with this approach I will propose for the RFC to go to stage 3, and will write up the full RFC.
Testing
Currently, just a fixture with manual tests.
Docs