Bun/Node: remove the --no-save which seems to cause issues sometimes#412
Bun/Node: remove the --no-save which seems to cause issues sometimes#412
Conversation
WalkthroughThe updates remove the Changes
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
go.mod(1 hunks)internal/bundler/bundler.go(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Test CLI Upgrade Path (windows-latest)
- GitHub Check: Test CLI Upgrade Path (blacksmith-4vcpu-ubuntu-2204-arm)
- GitHub Check: Test CLI Upgrade Path (macos-latest)
- GitHub Check: Test CLI Upgrade Path (blacksmith-4vcpu-ubuntu-2204)
- GitHub Check: Build and Test (blacksmith-4vcpu-ubuntu-2204-arm)
- GitHub Check: Test Install Script (blacksmith-4vcpu-ubuntu-2204)
- GitHub Check: Build and Test (blacksmith-4vcpu-ubuntu-2204)
- GitHub Check: Build and Test (macos-latest)
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
go.mod (1)
7-7: Dependency update verified and secure
- Confirmed that
github.com/agentuity/go-common v1.0.67exists (published 2025-06-17).- No known security advisories or CVEs affecting this release.
This minor version bump is safe to merge.
internal/bundler/bundler.go (1)
109-109: Ensuresource-map-jspersists in package.jsonRemoving
--no-savemeansbun add source-map-jswill now add the dependency to your project’s package.json (it isn’t present today). This change makessource-map-jsa permanent dependency, which can affect build reproducibility and CI/CD installs. Confirm this aligns with your dependency management strategy.
| install = exec.CommandContext(ctx.Context, "npm", "install", "--no-audit", "--no-fund", "--include=prod", "--ignore-scripts") | ||
| case "bunjs": | ||
| args := []string{"install", "--production", "--no-save", "--ignore-scripts"} | ||
| args := []string{"install", "--production", "--ignore-scripts"} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Behavioral change: bun install will now save dependencies to package.json.
Removing --no-save from the bun install command means dependencies will be saved to package.json. This change aligns with the npm install modification but could have significant implications.
Consider the following impacts:
- Dependencies will now be persisted in package.json
- This could affect build reproducibility across different environments
- CI/CD pipelines may need adjustment if they rely on the current behavior
- Team members will automatically install these dependencies
If the goal is to ensure dependencies are tracked, this change makes sense. However, if temporary installation was intended, consider using --no-save explicitly or documenting this behavioral change.
🤖 Prompt for AI Agents
In internal/bundler/bundler.go at line 152, the bun install command was changed
by removing the --no-save flag, causing dependencies to be saved to
package.json. To fix this, decide if you want dependencies to be persisted; if
not, re-add the --no-save flag to the args slice to prevent modifying
package.json and maintain the previous behavior. If saving dependencies is
intended, ensure this behavioral change is documented and communicated to the
team.
| switch theproject.Bundler.Runtime { | ||
| case "nodejs": | ||
| install = exec.CommandContext(ctx.Context, "npm", "install", "--no-save", "--no-audit", "--no-fund", "--include=prod", "--ignore-scripts") | ||
| install = exec.CommandContext(ctx.Context, "npm", "install", "--no-audit", "--no-fund", "--include=prod", "--ignore-scripts") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Behavioral change: npm install will now save dependencies to package.json.
Removing --no-save from the npm install command means dependencies will be saved to package.json. This is a significant change in dependency management behavior.
Since this command is used for installing production dependencies (note the --include=prod flag), consider:
- Whether saving dependencies to package.json is intentional
- If this might cause conflicts with existing package.json entries
- The impact on CI/CD pipelines that rely on the current behavior
If the intent is to install without saving, consider using --no-save explicitly or use a different approach like npm ci for production installs.
🤖 Prompt for AI Agents
In internal/bundler/bundler.go at line 150, the npm install command was changed
to remove the --no-save flag, causing dependencies to be saved to package.json,
which alters the intended behavior. To fix this, explicitly add the --no-save
flag back to the npm install command to prevent modifying package.json during
production dependency installation, or alternatively replace the command with
npm ci if the goal is to install exact dependencies from package-lock.json
without saving changes.
Summary by CodeRabbit
--no-saveflag from relevant install processes.