Content Collection cache (experimental) #8854
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 3d61b49 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
the
pkg: astro
natemoo-re
commented
Oct 17, 2023
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
natemoo-re
force-pushed
the
refactor/content-collections
branch
from
af787e4
to
ba83398
Compare
github-actions
bot
added
the
pr: docs
natemoo-re
force-pushed
the
refactor/content-collections
branch
from
6636df2
to
aba219d
Compare
|
!preview content-cache |
|
natemoo-re
force-pushed
the
refactor/content-collections
branch
from
f38cdb9
to
b1f127e
Compare
|
!preview content-cache |
|
natemoo-re
force-pushed
the
refactor/content-collections
branch
4 times, most recently
from
f97ffa7
to
7870d0c
Compare
|
Working on a refactor to be stacked against this PR refactor/content-collections...refactor/collections-build |
natemoo-re
force-pushed
the
refactor/content-collections
branch
from
7b52b92
to
5466fb6
Compare
natemoo-re
changed the title
|
/preview content-cache |
|
Snapshots have been released for the following packages:
Publish LogBuild Log |
natemoo-re
force-pushed
the
refactor/content-collections
branch
2 times, most recently
from
84dd3a2
to
b14a2bc
Compare
|
/preview content-cache |
|
Snapshots have been released for the following packages:
Publish LogBuild Log |
sarah11918
approved these changes
Nov 8, 2023
Co-authored-by: Sarah Rainsberger <[email protected]>
matthewp
reviewed
Nov 8, 2023
matthewp
reviewed
Nov 8, 2023
matthewp
reviewed
Nov 8, 2023
matthewp
reviewed
Nov 8, 2023
matthewp
reviewed
Nov 8, 2023
matthewp
reviewed
Nov 8, 2023
matthewp
approved these changes
Nov 9, 2023
Merged
ematipico
reviewed
Nov 9, 2023
Comment on lines
+42
to
+44
| configResolved(config) { | ||
| IS_DEV = config.mode === 'development' | ||
| }, |
There was a problem hiding this comment.
settings already contains the config; why do we need configResolved in this case?
| return; | ||
| } | ||
| if (code.includes(RESOLVED_VIRTUAL_MODULE_ID)) { | ||
| const depth = chunk.fileName.split('/').length - 1; |
| function getStringifiedCollectionFromLookup(wantedType: 'content' | 'data' | 'render', relContentDir: string, lookupMap: ContentLookupMap) { | ||
| let str = '{'; | ||
| // In dev, we don't need to normalize the import specifier at all. Vite handles it. | ||
| let normalize = (slug: string) => slug; |
There was a problem hiding this comment.
This can be let normalize because if/else both reach the point where we do normalize =
| let suffix = ''; | ||
| if (wantedType === 'content') suffix = CONTENT_FLAG; | ||
| else if (wantedType === 'data') suffix = DATA_FLAG; | ||
| else if (wantedType === 'render') suffix = CONTENT_RENDER_FLAG; |
There was a problem hiding this comment.
This can be else because we checked all types
| `lookupMap = ${stringifiedLookupMap};` | ||
| ), | ||
| }; | ||
| function getStringifiedCollectionFromLookup(wantedType: 'content' | 'data' | 'render', relContentDir: string, lookupMap: ContentLookupMap) { |
There was a problem hiding this comment.
It seems we introduced some new logic: 'content' | 'data' | 'render'. Maybe it's worth having a comment that explains what we do.
If you think it's too technical, we could also have a README.md
Comment on lines
+180
to
+191
| await fsMod.promises.mkdir(contentCacheDir, { recursive: true }); | ||
| await fsMod.promises.writeFile(contentManifestFile, JSON.stringify(newManifest), { encoding: 'utf8' }); | ||
|
|
||
| const cacheExists = fsMod.existsSync(cache); | ||
| fsMod.mkdirSync(cache, { recursive: true }) | ||
| await fsMod.promises.mkdir(cacheTmp, { recursive: true }); | ||
| await copyFiles(distContentRoot, cacheTmp, true); | ||
| if (cacheExists) { | ||
| await copyFiles(contentCacheDir, distContentRoot, false); | ||
| } | ||
| await copyFiles(cacheTmp, contentCacheDir); | ||
| await fsMod.promises.rm(cacheTmp, { recursive: true, force: true }); |
There was a problem hiding this comment.
Can we enclose all these FS operations in a try/catch block? There might be remote situations where we don't have write access
| ]); | ||
| newManifest.serverEntries = Array.from(serverComponents); | ||
| newManifest.clientEntries = Array.from(clientComponents); | ||
| await fsMod.promises.mkdir(contentCacheDir, { recursive: true }); |
There was a problem hiding this comment.
Should we check if contentCacheDir exists already?
|
|
||
| async function generateContentManifest(opts: StaticBuildOptions, lookupMap: ContentLookupMap): Promise<ContentManifest> { | ||
| let manifest: ContentManifest = { version: CONTENT_MANIFEST_VERSION, entries: [], serverEntries: [], clientEntries: [] }; | ||
| const limit = pLimit(10); |
| const key: ContentManifestKey = { collection, type, entry }; | ||
| const fileURL = new URL(encodeURI(joinPaths(opts.settings.config.root.toString(), entry))); | ||
| promises.push(limit(async () => { | ||
| const data = await fsMod.promises.readFile(fileURL, { encoding: 'utf8' }); |
There was a problem hiding this comment.
Is this operation safe? Should we check if fileURL exists and if we can read the file?
| @@ -36,3 +36,19 @@ export function i18nHasFallback(config: AstroConfig): boolean { | |||
|
|
|||
| return false; | |||
| } | |||
|
|
|||
| export function encodeName(name: string): string { | |||
There was a problem hiding this comment.
We should at least explain what kind of encoding this function does. Or maybe an example.
Merged
natemoo-re
added a commit
that referenced
this pull request
Nov 22, 2023
Co-authored-by: Sarah Rainsberger <[email protected]> Co-authored-by: Matthew Phillips <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
experimental.contentCollectionCacheflag. This flag changes a few things about our build process, which allows us to share Content Collection assets between the build.astro build --forceflag to clear the cache if needed.import.meta.glob, which will invalidate a collection every time one of the modules it references changes, this implements a custom build for content collections where individual modules can be built separately.In order to try this PR out locally, run:
Real-world results from this PR:
Note that the build still renders every page, which can take a significant time with large projects.
2.5K items
Uncached 33.31s
Cached 10.53s
5K items
Uncached 86.70s
Cached 44.17s
Docs Repo (Real World)
Uncached 133.20s
Cached 10.46s
Testing
Added a few initial test suites for the experimental behavior. To remove the experimental flag, we'll need more comprehensive test coverage.
Docs
For review:
.changeset/lovely-pianos-build.mdpackages/astro/src/@types/astro.ts