refactor: enhance dependency management in bundle script by modulariz… #222
Conversation
…ing functions and improving local package handling
|
Caution Review failedThe pull request is closed. WalkthroughRefactors Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as CLI/Runner
participant Bundle as tools/bundle.ts
participant Graph as Package Graph
participant FS as FS/package.json
participant Bun as Bun.build
Note over Bundle: Modularized dependency collection
CLI->>Bundle: run()
Bundle->>Graph: getLocalPackages(packages)
Graph-->>Bundle: localPackageNames
par Collect dependency maps
Bundle->>FS: getDependencies(pkg, "dependencies")
FS-->>Bundle: deps
Bundle->>FS: getDependencies(pkg, "devDependencies")
FS-->>Bundle: devDeps
Bundle->>FS: getDependencies(pkg, "peerDependencies")
FS-->>Bundle: peerDeps
end
Bundle->>Bundle: getDependenciesFromPackages(...)\nfilterWorkspace(...)
Bundle->>Bundle: externals = union(allDeps) - localPackageNames
Bundle->>FS: merge filtered deps into package.json
Bundle->>Bun: Bun.build({ external: externals })
Bun-->>Bundle: artifacts
Bundle-->>CLI: artifacts + updated package.json
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #222 +/- ##
=======================================
Coverage 81.07% 81.07%
=======================================
Files 63 63
Lines 4719 4719
=======================================
Hits 3826 3826
Misses 893 893 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
bundle.ts(2 hunks)
| external: Object.keys(dependencies).filter( | ||
| (dep) => !localPackagesNames.has(dep), | ||
| ), |
There was a problem hiding this comment.
Include peer dependencies in Bun externals
Line 70 currently ignores peerDependencies, so Bun tries to bundle them. That breaks builds whenever a peer isn’t installed locally (common) and defeats the purpose of peer deps.
- external: Object.keys(dependencies).filter(
- (dep) => !localPackagesNames.has(dep),
- ),
+ external: [
+ ...new Set([
+ ...Object.keys(dependencies),
+ ...Object.keys(peerDependencies),
+ ]),
+ ].filter((dep) => !localPackagesNames.has(dep)),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| external: Object.keys(dependencies).filter( | |
| (dep) => !localPackagesNames.has(dep), | |
| ), | |
| external: [ | |
| ...new Set([ | |
| ...Object.keys(dependencies), | |
| ...Object.keys(peerDependencies), | |
| ]), | |
| ].filter((dep) => !localPackagesNames.has(dep)), |
🤖 Prompt for AI Agents
In bundle.ts around lines 70-72, the externals array is built only from
Object.keys(dependencies) which omits peerDependencies and causes Bun to attempt
bundling peers; update the externals construction to include keys from both
dependencies and peerDependencies (guarding for undefined) and then filter out
localPackagesNames, e.g. combine Object.keys(dependencies || {}) with
Object.keys(peerDependencies || {}) before filtering so peer deps are treated as
externals.
| packageJson.dependencies = filterWorkspace({ | ||
| ...packageJson.dependencies, | ||
| ...dependencies, | ||
| }); | ||
| packageJson.devDependencies = filterWorkspace({ | ||
| ...packageJson.devDependencies, | ||
| ...devDependencies, | ||
| }); | ||
| packageJson.peerDependencies = filterWorkspace({ | ||
| ...packageJson.peerDependencies, | ||
| ...peerDependencies, | ||
| }); |
There was a problem hiding this comment.
Guard against spreading undefined dependency maps
Line 81 will throw TypeError: Cannot convert undefined or null to object whenever the source package.json omits one of these sections (peer/dev deps are optional). Please coalesce to {} before spreading.
- packageJson.dependencies = filterWorkspace({
- ...packageJson.dependencies,
- ...dependencies,
- });
- packageJson.devDependencies = filterWorkspace({
- ...packageJson.devDependencies,
- ...devDependencies,
- });
- packageJson.peerDependencies = filterWorkspace({
- ...packageJson.peerDependencies,
- ...peerDependencies,
- });
+ packageJson.dependencies = filterWorkspace({
+ ...(packageJson.dependencies ?? {}),
+ ...dependencies,
+ });
+ packageJson.devDependencies = filterWorkspace({
+ ...(packageJson.devDependencies ?? {}),
+ ...devDependencies,
+ });
+ packageJson.peerDependencies = filterWorkspace({
+ ...(packageJson.peerDependencies ?? {}),
+ ...peerDependencies,
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| packageJson.dependencies = filterWorkspace({ | |
| ...packageJson.dependencies, | |
| ...dependencies, | |
| }); | |
| packageJson.devDependencies = filterWorkspace({ | |
| ...packageJson.devDependencies, | |
| ...devDependencies, | |
| }); | |
| packageJson.peerDependencies = filterWorkspace({ | |
| ...packageJson.peerDependencies, | |
| ...peerDependencies, | |
| }); | |
| packageJson.dependencies = filterWorkspace({ | |
| ...(packageJson.dependencies ?? {}), | |
| ...dependencies, | |
| }); | |
| packageJson.devDependencies = filterWorkspace({ | |
| ...(packageJson.devDependencies ?? {}), | |
| ...devDependencies, | |
| }); | |
| packageJson.peerDependencies = filterWorkspace({ | |
| ...(packageJson.peerDependencies ?? {}), | |
| ...peerDependencies, | |
| }); |
🤖 Prompt for AI Agents
In bundle.ts around lines 81 to 92, spreading
packageJson.dependencies/devDependencies/peerDependencies will throw when those
properties are undefined; coalesce each source section to an empty object before
spreading (e.g., use packageJson.dependencies || {} etc.) so you merge into a
safe object, then pass the result to filterWorkspace.
…ing functions and improving local package handling
Summary by CodeRabbit
Refactor
Chores