fix(core): Change prepack back to prepare for git dependency builds#1282
fix(core): Change prepack back to prepare for git dependency builds#1282
Conversation
…emoval prepack does not reliably run when installing repomix as a git dependency, causing the server Docker build to fail (lib/ directory missing). Change to prepare which reliably runs for git dependency installation. Also change npm ci --omit=dev to npm prune --omit=dev in Dockerfile and CI, since npm prune does not trigger lifecycle scripts (avoiding prepare failure when devDependencies are no longer available). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
⚡ Performance Benchmark
Details
|
|
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 pull request replaces redundant dependency reinstallation steps with in-place pruning in CI and Docker workflows, and shifts the npm build lifecycle hook from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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 |
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! このプルリクエストは、git 依存関係としてインストールされた場合にパッケージのビルドが失敗する問題を解決することを目的としています。以前の変更で 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. Ignored Files
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. Footnotes
|
Code ReviewOverall: Looks good - This is a clean, well-motivated revert with a thorough explanation of the underlying npm/pacote bug. Changes ReviewAll three changes are correct and consistent:
Observations
Potential ConcernsDetails
No bugs, security issues, or test coverage concerns. The change is minimal and reverts to a previously working configuration with a well-understood fix for the 🤖 Generated with Claude Code |
Deploying repomix with
|
| Latest commit: |
37fa9f7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6a118e54.repomix.pages.dev |
| Branch Preview URL: | https://fix-prepack-to-prepare.repomix.pages.dev |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Dockerfile (1)
14-18: Change is correct; consider removing redundant build step.Using
npm prune --omit=devinstead ofnpm ci --omit=devis the right approach—it removes devDependencies without re-triggering lifecycle scripts that would fail without build tools.However, with
prepare: npm run buildin package.json,npm cion line 14 already triggers the build automatically. The explicitnpm run buildon line 15 is now redundant and adds to Docker build time.♻️ Optional optimization to remove redundant build
RUN npm ci \ - && npm run build \ && npm link \ && npm prune --omit=dev \ && npm cache clean --force🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 14 - 18, Remove the redundant explicit build step: since package.json runs "prepare": "npm run build", the initial npm ci will already run the build, so delete the standalone npm run build invocation in the Dockerfile (the line calling npm run build) and keep npm ci, npm link, npm prune --omit=dev, and npm cache clean --force as-is to avoid re-triggering lifecycle scripts and reduce image build time.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Dockerfile`:
- Around line 14-18: Remove the redundant explicit build step: since
package.json runs "prepare": "npm run build", the initial npm ci will already
run the build, so delete the standalone npm run build invocation in the
Dockerfile (the line calling npm run build) and keep npm ci, npm link, npm prune
--omit=dev, and npm cache clean --force as-is to avoid re-triggering lifecycle
scripts and reduce image build time.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 55c0940b-6bfc-4576-aa6c-7924247ef9e6
📒 Files selected for processing (3)
.github/workflows/ci.ymlDockerfilepackage.json
The prepare lifecycle script already runs npm run build automatically during npm ci, making the explicit build step redundant. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1282 +/- ##
=======================================
Coverage 87.18% 87.18%
=======================================
Files 115 115
Lines 4324 4324
Branches 1002 1002
=======================================
Hits 3770 3770
Misses 554 554 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Updated Review (post second commit)My previous review still stands. The second commit ( Bot Comment Evaluation
Final AssessmentApprove — All changes are correct. The PR cleanly reverts the problematic 🤖 Generated with Claude Code |
Summary
prepackback topreparelifecycle scriptnpm ci --omit=devtonpm prune --omit=devin Dockerfile and CIBackground
PR #1215 changed
preparetoprepack, but npm'sprepackdoes not actually run when installing git dependencies (npm bug: npm/pacote#257).As a result, when
website/serverinstalls"repomix": "github:yamadashy/repomix#main", thelib/directory is never built, causing TypeScript compilation to fail.The reality of npm prepack
prepare, notprepack, indir.jsWhy npm prune is needed
prepareruns duringnpm ci --omit=dev, but devDependencies (rimraf, tsc) are not available, causingnpm run buildto fail.npm prune --omit=devdoes not trigger lifecycle scripts, avoiding this issue.Reverts
Checklist
npm run testnpm run lint