Replace npm prune with npm ci --omit=dev for consistency#1218
Replace npm prune with npm ci --omit=dev for consistency#1218
Conversation
Now that `prepare` has been replaced with `prepack` (which only runs on npm pack/publish), `npm ci --omit=dev` no longer triggers an unwanted build. This allows a cleaner approach: install all deps, build, then reinstall with only production dependencies via `npm ci --omit=dev`. https://claude.ai/code/session_01WorTDgSv8c9jvVpa1YsSeo
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR updates the dependency installation strategy in the CI/CD workflow and Docker build process, replacing Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Code ReviewOverall: Approve - This is a clean, well-motivated change. SummaryReplacing The commit message correctly notes that this is now safe because DockerfileThe reordering of Previously, The updated comment ("all steps" instead of "npm ci and npm link") is accurate. CI WorkflowThe CI change is straightforward - same substitution. Since ConsiderationsDetailsPerformance: Docker layer size: Both Potential failure scenario: If VerdictNo issues found. The change is correct, well-scoped, and the rationale is clearly documented in both the PR description and commit message. Reviewed by Claude |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1218 +/- ##
=======================================
Coverage 87.14% 87.14%
=======================================
Files 115 115
Lines 4310 4310
Branches 998 998
=======================================
Hits 3756 3756
Misses 554 554 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
The previous commit unnecessarily changed the order of npm link and the dev dependency removal step. Since npm link only creates a global symlink to the package directory, it should come before npm ci --omit=dev, matching the original order when npm prune --omit=dev was used. https://claude.ai/code/session_01WorTDgSv8c9jvVpa1YsSeo
Deploying repomix with
|
| Latest commit: |
007a886
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d60ea11a.repomix.pages.dev |
| Branch Preview URL: | https://claude-remove-omit-dev-flag.repomix.pages.dev |
Code Review (Follow-up)Overall: Approve - Clean, well-scoped change. Agreeing with the previous review's conclusion. Correction to Previous ReviewThe previous Claude review described the Dockerfile step order as This order is correct. Key Behavioral Difference from
|
| Scenario | Risk | Mitigation |
|---|---|---|
prepack changed back to prepare |
npm ci --omit=dev would trigger a build without devDeps, causing failure |
Commit message documents this dependency; CI would catch it immediately |
| Lockfile drift | npm ci fails if package-lock.json doesn't match package.json |
This is a feature, not a bug — surfaces issues earlier than npm prune |
npm link after npm ci --omit=dev (order accidentally swapped) |
Would still work, but npm link might warn about missing devDeps in node_modules |
Current order (link first) avoids this; second commit specifically preserved this |
Verdict
No issues found. The change is correct and well-documented across both commits.
Reviewed by Claude
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Docker build process by standardizing the method for installing production dependencies. By switching from Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces npm prune with npm ci for removing development dependencies in the Docker build, aiming for improved reliability. While the change is functionally correct, it introduces a significant inefficiency by running npm ci twice. My review includes a suggestion to refactor the Dockerfile to use a multi-stage build, which is a more efficient and standard practice for this scenario, resulting in faster builds and a smaller final image.
| RUN npm ci \ | ||
| && npm run build \ | ||
| && npm link \ | ||
| && npm prune --omit=dev \ | ||
| && npm ci --omit=dev \ | ||
| && npm cache clean --force |
There was a problem hiding this comment.
While using npm ci --omit=dev is a valid way to remove development dependencies, running npm ci twice in the same RUN command is inefficient. The first npm ci installs all dependencies (including dev), and the second one removes the node_modules directory entirely before reinstalling only the production dependencies. This means you're downloading and installing dev dependencies only to throw them away immediately after the build step.
A more efficient and idiomatic approach in Docker is to use a multi-stage build. This would allow you to build the application with dev dependencies in a 'builder' stage, and then copy only the necessary production artifacts to a clean final stage, where you would run npm ci --omit=dev only once.
This would result in a faster build process and a smaller final image. Here's an example of how it could look:
# --- Builder Stage ---
FROM node:22-slim AS builder
WORKDIR /repomix
# Install all dependencies
COPY package.json package-lock.json ./
RUN npm ci
# Copy the rest of the source and build
COPY . .
RUN npm run build
# --- Final Stage ---
FROM node:22-slim
RUN apt-get update && apt-get install -y --no-install-recommends \
git \
ca-certificates \
&& rm -rf /var/lib/apt/lists/*
WORKDIR /repomix
# Copy only necessary artifacts from builder stage
COPY --from=builder /repomix/package.json /repomix/package-lock.json ./
COPY --from=builder /repomix/lib ./lib
COPY --from=builder /repomix/bin ./bin
COPY --from=builder /repomix/README.md ./
COPY --from=builder /repomix/LICENSE ./
# Install only production dependencies and link
RUN npm ci --omit=dev \
&& npm link \
&& npm cache clean --force
WORKDIR /app
# ... rest of your Dockerfile
RUN repomix --version
RUN repomix --help
ENTRYPOINT ["repomix"]While this is a larger change, it's a significant improvement for the Dockerfile's efficiency and maintainability, aligning with Docker best practices.
2 participants
Summary
This change replaces
npm prune --omit=devwithnpm ci --omit=devin both the Dockerfile and CI workflow. This provides a more consistent and reliable approach to installing only production dependencies.Changes
npm prune --omit=devwithnpm ci --omit=devin the build layer, and reordered it to run beforenpm linkfor better layer efficiencynpm ci --omit=devinstead ofnpm prune --omit=devRationale
npm ci --omit=devis more explicit and reliable thannpm prune --omit=devfor ensuring only production dependencies are present. It performs a clean install based on the lockfile, which is more predictable than pruning after installation.Checklist
npm run testnpm run linthttps://claude.ai/code/session_01WorTDgSv8c9jvVpa1YsSeo