fix: override yargs to v18 for Node 25 compatibility#391
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaced Node '24' with 'latest' in the GitHub Actions test matrix and added a top-level 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #391 +/- ##
=======================================
Coverage 46.97% 46.97%
=======================================
Files 3 3
Lines 149 149
=======================================
Hits 70 70
Misses 79 79 🚀 New features to boost your workflow:
|
c8 depends on yargs ^17 which breaks on Node 25 due to CJS/ESM interop changes. Override yargs to ^18 (ESM-first) until c8 updates its dependency (bcoe/c8#578). Also restores node latest to the test matrix alongside 20 and 22. Signed-off-by: Tomer Figenblat <tomer@figenblat.com>
ed8c560 to
cf0297a
Compare
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`:
- Around line 58-62: The package.json "overrides" entry currently forces "c8" to
use "yargs": "^18.0.0", but yargs@18 requires Node >=20.19.0 which breaks the
project’s declared Node floor (>=18.0.0); update package.json to either (A)
raise the package's "engines.node" minimum to ">=20.19.0" to match yargs@18, (B)
relax the override to target a Node-18-compatible version such as "yargs": "^17"
(or remove the override entirely) so Node 18 users can install, or (C)
remove/adjust the "overrides" block for "c8" to avoid pinning yargs@18; modify
the "overrides" -> "c8" -> "yargs" entry accordingly and ensure the lockfile is
regenerated.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
.github/workflows/test_package.ymlpackage.json
yargs 18 requires Node >= 20.19.0. Node 18 is EOL since April 2025. Signed-off-by: Tomer Figenblat <tomer@figenblat.com>
Summary
overridesto pinyargsto^18.0.0withinc8, fixing the CJS/ESM interop crash on Node 25node: latestto the test matrix (alongside 20 and 22)Context
c8depends onyargs@^17which usesrequire()in an extensionless file. Node 25 treats this as ESM in"type": "module"projects, causingReferenceError: require is not defined.yargs18 is ESM-first and resolves this.Upstream fix in progress: bcoe/c8#578. The override can be removed once
c8releases withyargs18.Summary by CodeRabbit