Prevent Cloudflare build issues due to Node.js builtins usage #3341
Conversation
This file contains hidden or 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: 2168fc1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
✅ Deploy Preview for astro-starlight ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
delucis
reviewed
Jul 28, 2025
Comment on lines
26
to
28
| export function getCollectionPath(collection: StarlightCollection, srcDir: URL) { | ||
| return getCollectionUrl(collection, srcDir).pathname; | ||
| } |
There was a problem hiding this comment.
Wonder if anyone might mistakenly think this returns a file system “path” they can use?
Oh… and just checked, looks like this is completely unused? Was going to suggest renaming/documenting, but I guess we can just delete it! 😤
Suggested change
| export function getCollectionPath(collection: StarlightCollection, srcDir: URL) { | |
| return getCollectionUrl(collection, srcDir).pathname; | |
| } |
There was a problem hiding this comment.
Great catch, totally missed that during the refactor 🙈
To explain why this helper is no longer needed after this PR:
- After splitting collection helpers into 2 files (using
fsand not usingfs), thegetCollectionPathhelper was only used in theabsolutePathToLanghelper. - As this was an issue to receive an URL-encoded pathname in the
absolutePathToLanghelper (which was recently introduced in FixabsolutePathToLang()issue #3298), it was refactored to now accept thedocscontent collection path as an argument and compare POSIX paths. - This new argument is now passed from the new and explicit
getCollectionPosixPathhelper makinggetCollectionPathobsolete.
Co-authored-by: delucis <[email protected]>
Co-authored-by: delucis <[email protected]>
Co-authored-by: delucis <[email protected]>
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
The linked issue and its reproduction showcase build errors with the Cloudflare adapter due to some Node.js builtins ending up in the final bundle. Using updated dependencies seems to no longer cause this issue with some modules being automatically externalized by Vite but still logging some warnings, for example:
While this may work now, this PR aims to fix such issues and prevent regressions in the future by adding a Vite plugin (copied from Astro and slightly tweaked) to one of our e2e fixtures that checks that the final bundle does not have a dependencies on Node.js builtins due to Starlight and throws an error if it does.
Regarding the issue, 3 main reasons for the issue were identified:
<Aside>component. This caused some Node.js builtins used in the remark plugin to end up in the final bundle. To fix this, the shared error was moved to a dedicated file.collection.tsfile which contains various helpers to get paths to collections used in Starlight and used a lot throughout the codebase, contained 1 function relying on Node.js builtins which ended up leaking due to many places importing this file without using the function. To fix this, the function was moved to a dedicated file.absolutePathToLanghelper shared between many integrations was recently updated in FixabsolutePathToLang()issue #3298 to use a Node.js builtin (pathToFileURL) to convert a path to a URL and avoid issues with paths with some encoded characters, e.g. a space. This helper used by Expressive Code can end up in the final bundle due to it's usage in the preprocessor config which is also exposed in a virtual module throughpackages/starlight/internal.tsfor the<Code>component. To fix this, the helper has been updated to no longer be in charge of figuring out thedocscollection path, and instead the POSIX path to this collection is passed down to this helper.An additional test for these changes is also to install and build the documentation using the Cloudflare adapter and making sure that no log warning is emitted about Node.js builtins being automatically externalized.
Remaining tasks