docs: clarify noExternals path patterns - #4460
Conversation
|
@vittorioexp is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughUpdated ChangesConfiguration documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This clarifies configuration matching, Nitro migration guidance, and TypeScript bundler configuration without changing runtime behavior. No current merge-readiness risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/3.config/0.index.md`:
- Line 1068: Update the documentation sentence about Nitro’s default dependency
externalization to qualify it as applying only to Node-compatible builds, or
explicitly state that it depends on nitro.options.node being true; keep the
existing explanation of inlined modules and alias configuration unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 089aebfe-2390-467d-8060-3bb70d8452c3
📒 Files selected for processing (2)
docs/3.config/0.index.mdsrc/types/config.ts
| - Default: `false`{lang=ts} | ||
|
|
||
| Prevent specific packages from being externalized. Set to `true` to bundle all dependencies, or pass an array of package names/patterns. | ||
| By default, Nitro externalizes dependencies from `node_modules` for faster builds. Inlined modules are bundled into the output and can use your `alias` configuration. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Qualify the default externalization claim for non-Node builds.
src/build/plugins.ts:59-76 only enables the externals plugin when nitro.options.node is true; for edge/non-Node builds, this default externalization path is not enabled. Please say “Node-compatible builds” or document the node condition.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/3.config/0.index.md` at line 1068, Update the documentation sentence
about Nitro’s default dependency externalization to qualify it as applying only
to Node-compatible builds, or explicitly state that it depends on
nitro.options.node being true; keep the existing explanation of inlined modules
and alias configuration unchanged.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
commit: |
Document that noExternals array entries match resolved module paths, with monorepo and node_modules examples. Closes nitrojs#3269.
Patterns are matched against the import specifier as well as the resolved path, and v3 production builds already bundle by default.
d9fdd8e to
cbeba72
Compare
This comment was written by an AI assistant on behalf of the Nitro maintainers. Please double-check anything that looks wrong. |
Summary
Closes #3269.
Documents how
noExternalsarray patterns work: they match resolved absolute module paths, not bare import specifiers. Adds monorepo andnode_modulesexamples and a Nitro 2externals.inlinemigration note.Also updates the
noExternalsJSDoc insrc/types/config.tsto match.Test plan
src/build/config.tsandsrc/build/plugins/externals.ts