fix(astro): handle symlinked content collection directories #11236
Conversation
🦋 Changeset detectedLatest commit: 35d50f6 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
|
Hm. Looks like it's broken with caching. |
packages/astro/src/content/utils.ts
Outdated
| const contentDirEntries = await fsMod.promises.readdir(contentDir, { withFileTypes: true }); | ||
| for (const entry of contentDirEntries) { | ||
| if (entry.isSymbolicLink()) { | ||
| const entryPath = path.join(contentDirPath, entry.name); | ||
| const realPath = await fsMod.promises.realpath(entryPath); | ||
| contentPaths.set(normalizePath(realPath), entry.name); | ||
| } | ||
| } |
There was a problem hiding this comment.
nit: file system operations can come with runtime errors (permissions, etc.), I tend to wrap these operations in a try/catch to be more safe
There was a problem hiding this comment.
Good point. As a rule, would you silently ignore any errors there, log a warning, or fail the build?
There was a problem hiding this comment.
I'd lean towards silently ignoring them here, because we're just looking for symlinks and a failure probably means it's not a symlink.
There was a problem hiding this comment.
It really depends on the use case, really. In this case we could just ignore the error, and maybe log and error/warning using the logger (you should have access to it).
There was a problem hiding this comment.
Where would I find a logger instance? I'm calling this in vite-plugin-content-imports.
There was a problem hiding this comment.
It seems we don't pass it down to this plugin (the logger arrived after this plugin was created). You'll have to pass it like in this case:
astro/packages/astro/src/core/create-vite.ts
Line 149 in 2851b0a
There was a problem hiding this comment.
Aha, thanks. I just pushed an update with error handling and renamed funciton, but no logging. I'll add the logs.
There was a problem hiding this comment.
OK, I've added logging now
packages/astro/src/content/utils.ts
Outdated
| return contentPaths; | ||
| } | ||
|
|
||
| export function reverseSymlinks({ |
There was a problem hiding this comment.
nit: reverseSymlinks -> reverseSymlink singular, because it returns a string, and not a stringp[]
There was a problem hiding this comment.
The reason I did the plural is because there are multiple symlinks. But I can see what you mean.
Co-authored-by: Emanuele Stoppa <[email protected]>
Changes
Currently if you try to use a symlink in a content collection directory it will break the build. This is because Vite resolves the path to the symlink target, which is then outside of the content directory.
This PR tracks symlinks in the content directory, and then reverses them when transforming the content. This works out as a more robust way than intercepting the module resolution, which seemed to break many things in various places.
Fixes #9088
Testing
Adds fixtures and test cases with symlinks
Docs
This should be documented. I'm not sure if we document the limitation currently. This only supports symlinks at the top level (i.e. the link is
src/content/blog -> /Path/to/real/content/blog).