fix(install): move ajv to dependencies and add .yarnrc.yml for node-modules linker#956
Conversation
…nker ajv was in devDependencies but required at runtime by scripts/lib/install/config.js, causing 'Cannot find module ajv' when running ./install.sh. Also adds .yarnrc.yml with nodeLinker: node-modules so plain `node` can resolve packages without PnP. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Analysis Failed
Troubleshooting
Retry: |
📝 WalkthroughWalkthroughA new Yarn configuration file is added with Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 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 |
There was a problem hiding this comment.
No issues found across 3 files
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Greptile SummaryThis PR fixes a genuine runtime breakage: Confidence Score: 4/5Safe to merge after reviewing the dual lock-file concern; the core bug fix is correct and well-scoped. The primary fix (ajv → dependencies) is clearly correct and directly addresses the reported runtime failure. The .yarnrc.yml addition is a sensible companion change for Yarn Berry environments. Two non-blocking P2 concerns exist: the coexistence of package-lock.json and yarn.lock (contributor confusion risk) and .yarnrc.yml being absent from the files array (affects npm-published users who rely on Yarn). Neither is a showstopper, but the lock-file conflict is worth a follow-up cleanup. .yarnrc.yml (missing from files array) and the pre-existing package-lock.json (should be removed to avoid conflict with the new yarn.lock) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["./install.sh"] --> B["node scripts/install-apply.js"]
B --> C["scripts/lib/install/config.js"]
C --> D{"require('ajv')"}
D -- "Before PR\n(devDependencies only)" --> E["❌ Cannot find module 'ajv'\n(prod/global install)"]
D -- "After PR\n(dependencies)" --> F["✅ Ajv loads successfully"]
F --> G["Validate install config JSON Schema"]
G --> H["Install proceeds normally"]
subgraph "Yarn Berry Setup"
I[".yarnrc.yml\nnodeLinker: node-modules"] --> J["node_modules/ directory\n(not PnP .pnp.cjs)"]
J --> F
end
Reviews (1): Last reviewed commit: "fix: move ajv to dependencies and add .y..." | Re-trigger Greptile |
| "node": ">=18" | ||
| } | ||
| }, | ||
| "packageManager": "yarn@4.9.2+sha512.1fc009bc09d13cfd0e19efa44cbfc2b9cf6ca61482725eb35bbc5e257e093ebf4130db6dfe15d604ff4b79efd8e1e8e99b25fa7d0a6197c9f9826358d4d65c3c" |
There was a problem hiding this comment.
Conflicting lock files coexist
Adding yarn.lock and a "packageManager": "yarn@4.9.2" field while the pre-existing package-lock.json (npm lockfileVersion 3) remains in the repo creates a dual-lock-file situation. Developers who run npm install out of habit will get a dependency graph resolved by npm (using package-lock.json) rather than yarn, which may produce a different node_modules tree and hide future regressions.
Consider either:
- Removing
package-lock.jsonin this PR to make the yarn-only intent explicit, or - Adding a note in the PR / CONTRIBUTING docs that this repo has migrated to Yarn Berry and that contributors should use
yarninstead ofnpm install.
| @@ -0,0 +1 @@ | |||
| nodeLinker: node-modules | |||
There was a problem hiding this comment.
.yarnrc.yml excluded from files in package.json
The files array in package.json controls what gets bundled into the npm tarball. .yarnrc.yml is not included in that list. This means users who install the package via npm install -g ecc-universal or npx ecc-universal will not receive the file, so the nodeLinker: node-modules setting won't apply to them and they could still hit Yarn PnP resolution failures when running the scripts.
The primary ajv → dependencies fix covers the common case. However, if Yarn PnP compatibility is a supported scenario for npm-installed users, .yarnrc.yml needs to be added to the files array:
"files": [
".yarnrc.yml",
...
]There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 126: The package.json packageManager entry currently references a
non-existent Yarn version; update the "packageManager" field to a valid Yarn
release (e.g., "yarn@2.4.3") and replace the accompanying integrity/hash with
the correct integrity string for that release; locate the packageManager key in
package.json and regenerate the correct integrity (for example by using Corepack
to prepare/activate the desired yarn version or consulting the official Yarn
release integrity) and paste the updated "yarn@<version>+sha512.<integrity>"
value.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 09728c85-c267-42b6-a455-796e971d6b96
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
.yarnrc.ymlpackage.json
| "node": ">=18" | ||
| } | ||
| }, | ||
| "packageManager": "yarn@4.9.2+sha512.1fc009bc09d13cfd0e19efa44cbfc2b9cf6ca61482725eb35bbc5e257e093ebf4130db6dfe15d604ff4b79efd8e1e8e99b25fa7d0a6197c9f9826358d4d65c3c" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify yarn@4.9.2 exists on npm registry and check the integrity hash
# Check if yarn@4.9.2 exists
echo "=== Checking yarn@4.9.2 on npm ==="
curl -s "https://registry.npmjs.org/yarn/4.9.2" | jq -r '.version // "NOT FOUND"'
# List recent yarn versions to confirm 4.9.2 is plausible
echo ""
echo "=== Recent yarn versions ==="
curl -s "https://registry.npmjs.org/yarn" | jq -r '.versions | keys | .[-10:]'Repository: affaan-m/everything-claude-code
Length of output: 349
Update packageManager to use a valid Yarn version.
Yarn 4.9.2 does not exist on the npm registry. The latest available version is 2.4.3. Update the packageManager field to reference an existing Yarn version and its correct integrity hash to prevent installation failures.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 126, The package.json packageManager entry currently
references a non-existent Yarn version; update the "packageManager" field to a
valid Yarn release (e.g., "yarn@2.4.3") and replace the accompanying
integrity/hash with the correct integrity string for that release; locate the
packageManager key in package.json and regenerate the correct integrity (for
example by using Corepack to prepare/activate the desired yarn version or
consulting the official Yarn release integrity) and paste the updated
"yarn@<version>+sha512.<integrity>" value.
Summary
ajvwas listed indevDependenciesbut is required at runtime byscripts/lib/install/config.js, causingCannot find module 'ajv'when running./install.shajvtodependenciesso it is always available.yarnrc.ymlwithnodeLinker: node-modulesso plainnodecan resolve packages without Yarn PnPType
Testing
Reproduced the error with
./install.sh typescript, applied the fix, confirmedajvloads successfully and the install command completes.Checklist
🤖 Generated with Claude Code
Summary by cubic
Fixes the install script failing with "Cannot find module 'ajv'" by moving
ajvto production deps and using Yarn’snode-moduleslinker so plain Node can resolve packages../install.shnow completes reliably across environments.ajvfromdevDependenciestodependenciesfor runtime use inscripts/lib/install/config.js..yarnrc.ymlwithnodeLinker: node-modulesto avoid PnP resolution issues.Written for commit fe6a6fc. Summary will update on new commits.
Summary by CodeRabbit