Upgrade deps and modernize eslint usage#187
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new ESLint config, updates linting/dev dependencies and axios, removes several inline ESLint disables, and applies small control-flow and test cleanup changes across source and test files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
package.json (2)
53-62:⚠️ Potential issue | 🟡 MinorRemove the stale
eslintConfigblock — it's dead config.The eslintrc config system has been completely removed in ESLint v10.0.0. That means
"eslintConfig"inpackage.jsonis silently ignored by ESLint v10. Additionally,eslint-config-standardwas removed fromdevDependenciesin this PR, so the"extends": "standard"inside this block references an uninstalled package. Leaving it here will mislead anyone reading the configuration.🗑️ Proposed fix — remove the dead section
- "eslintConfig": { - "extends": "standard", - "env": { - "mocha": true, - "node": true - }, - "rules": { - "no-return-await": "off" - } - },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 53 - 62, Remove the stale eslintConfig block from package.json: delete the entire "eslintConfig" object (including "extends": "standard", the "env" and "rules" entries) so the package.json no longer contains dead ESLint configuration that is ignored by ESLint v10 and references an uninstalled eslint-config-standard.
63-65:⚠️ Potential issue | 🟠 Major
engines.nodeis incompatible with ESLint v10's Node.js requirement.ESLint v10 requires Node.js
(^20.19.0, ^22.13.0, or >=24)built with SSL support. The currentenginesfield (>=20.0.0) permits Node 20.0.0–20.18.x, on which ESLint v10 will refuse to install, breakingnpm testandnpm run lintfor those developers. Theenginesfield should be tightened to match.🔧 Proposed fix
- "node": ">=20.0.0" + "node": "^20.19.0 || ^22.13.0 || >=24"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 63 - 65, Update the package.json "engines" node semver to match ESLint v10's supported Node versions: replace the current "engines": { "node": ">=20.0.0" } with a range that excludes Node 20.0.0–20.18.x, e.g. set the "node" value to ">=20.19.0 <21 || >=22.13.0 <23 || >=24.0.0" so ESLint v10 can install reliably; modify the "engines" -> "node" string accordingly.
🧹 Nitpick comments (1)
eslint.config.mjs (1)
14-16:"no-return-await": "off"is redundant — the rule is deprecated.The
no-return-awaitrule was deprecated in ESLint v8.46.0 and there is no replacement rule. It is also not enabled byjs.configs.recommended, so this"off"entry has no effect. It can be safely removed.♻️ Suggested cleanup
- rules: { - "no-return-await": "off", - },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eslint.config.mjs` around lines 14 - 16, Remove the redundant "no-return-await" property from the rules object in eslint.config.mjs (the rules block that currently contains "no-return-await": "off"); simply delete that key/value entry and adjust commas/formatting so the rules object remains valid JSON/JS syntax.
🤖 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 32: The package.json dependency for axios is pinned to an exact version
("axios": "1.13.5"); change it to a caret semver range (e.g. "axios": "^1.13.5")
so consumers can benefit from compatible minor/patch updates; update the
lockfile by running your package manager (npm install or yarn install)
afterwards to persist the change.
---
Outside diff comments:
In `@package.json`:
- Around line 53-62: Remove the stale eslintConfig block from package.json:
delete the entire "eslintConfig" object (including "extends": "standard", the
"env" and "rules" entries) so the package.json no longer contains dead ESLint
configuration that is ignored by ESLint v10 and references an uninstalled
eslint-config-standard.
- Around line 63-65: Update the package.json "engines" node semver to match
ESLint v10's supported Node versions: replace the current "engines": { "node":
">=20.0.0" } with a range that excludes Node 20.0.0–20.18.x, e.g. set the "node"
value to ">=20.19.0 <21 || >=22.13.0 <23 || >=24.0.0" so ESLint v10 can install
reliably; modify the "engines" -> "node" string accordingly.
---
Nitpick comments:
In `@eslint.config.mjs`:
- Around line 14-16: Remove the redundant "no-return-await" property from the
rules object in eslint.config.mjs (the rules block that currently contains
"no-return-await": "off"); simply delete that key/value entry and adjust
commas/formatting so the rules object remains valid JSON/JS syntax.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
63-65:⚠️ Potential issue | 🟡 Minor
engines.nodeis below ESLint 10's minimum Node.js requirement.ESLint 10.0.1 requires
Node.js (^20.19.0, ^22.13.0, or >=24), but theenginesfield currently allows>=20.0.0. Any contributor or CI runner on Node.js 20.0.0–20.18.x will satisfy this package's engine constraint but fail when npm attempts to install ESLint 10. Tightening theenginesfloor to>=20.19.0prevents confusing install-time errors.🔧 Suggested fix
"engines": { - "node": ">=20.0.0" + "node": ">=20.19.0" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 63 - 65, The engines.node constraint is too permissive for ESLint 10; update the package.json "engines" -> "node" value from ">=20.0.0" to ">=20.19.0" so the declared Node.js minimum matches ESLint 10's supported range (this change is in the package.json engines.node entry); ensure any CI/tooling that reads this field will align with the new minimum.
🧹 Nitpick comments (2)
package.json (2)
40-40:eslintexact pin — consider using tilde (~) instead.ESLint minor updates may report more linting errors than the previous release; the project itself therefore recommends using
~inpackage.json(e.g.,"eslint": "~3.1.0") to guarantee build results. All other devDependencies in this file use^; the bare exact pin is inconsistent and prevents automatic uptake of patch-level bug fixes.♻️ Suggested change
- "eslint": "10.0.1", + "eslint": "~10.0.1",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 40, Change the exact dependency pin "eslint": "10.0.1" to a tilde range so patch fixes are picked up (e.g., "eslint": "~10.0.1"); update the "eslint" entry in package.json accordingly to match the project's policy of using non-exact ranges like other devDependencies.
53-62: Remove the orphanedeslintConfigstanza.With ESLint 10 and a flat config (
eslint.config.mjsintroduced in this PR), the oldeslintConfig-in-package.json config system is no longer supported. The stanza also referenceseslint-config-standard(via"extends": "standard") which has been removed fromdevDependenciesin this same PR — leaving it is misleading and potentially confusing to contributors.♻️ Suggested removal
- "eslintConfig": { - "extends": "standard", - "env": { - "mocha": true, - "node": true - }, - "rules": { - "no-return-await": "off" - } - },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 53 - 62, Remove the orphaned eslintConfig stanza from package.json (the entire block starting with "eslintConfig": { ... }) — it references "extends": "standard" and legacy per-package ESLint config which is no longer used with the new eslint.config.mjs flat config; delete that block so package.json no longer contains eslintConfig, "extends": "standard", or the associated "rules" entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@package.json`:
- Around line 63-65: The engines.node constraint is too permissive for ESLint
10; update the package.json "engines" -> "node" value from ">=20.0.0" to
">=20.19.0" so the declared Node.js minimum matches ESLint 10's supported range
(this change is in the package.json engines.node entry); ensure any CI/tooling
that reads this field will align with the new minimum.
---
Nitpick comments:
In `@package.json`:
- Line 40: Change the exact dependency pin "eslint": "10.0.1" to a tilde range
so patch fixes are picked up (e.g., "eslint": "~10.0.1"); update the "eslint"
entry in package.json accordingly to match the project's policy of using
non-exact ranges like other devDependencies.
- Around line 53-62: Remove the orphaned eslintConfig stanza from package.json
(the entire block starting with "eslintConfig": { ... }) — it references
"extends": "standard" and legacy per-package ESLint config which is no longer
used with the new eslint.config.mjs flat config; delete that block so
package.json no longer contains eslintConfig, "extends": "standard", or the
associated "rules" entries.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (1)
package.json
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
package.json (1)
32-32: Previously flagged; now correctly resolved.
axiosis now properly using a caret range^1.13.5.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 32, The package.json dependency for axios was previously flagged but is now correctly set to a caret range; confirm that the "axios" entry in package.json uses "^1.13.5" and no further change is needed—if you see any other package.json entries or lockfile inconsistencies, run npm install / yarn install to refresh the lockfile and ensure the caret range is reflected consistently.
🧹 Nitpick comments (1)
package.json (1)
36-45:eslintdevDependency looks good overall — minor version-pin inconsistency on Line 40.
@eslint/js(Line 36),globals(Line 41), andmocha(Line 42) all use a caret (^) range, whileeslintitself on Line 40 is pinned to an exact version (10.0.1) with no range specifier. ESLint's own versioning policy notes that minor updates may report more linting errors, and recommends using a tilde (~) range specifier inpackage.jsonto guard against that while still allowing patch updates. Consider adding at least a tilde for consistency:♻️ Proposed fix
- "eslint": "10.0.1", + "eslint": "~10.0.1",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 36 - 45, The eslint devDependency is pinned to an exact version ("eslint": "10.0.1") while sibling devDeps use ranges; update the "eslint" entry in package.json to use a tilde range (e.g., change "10.0.1" to "~10.0.1") so patch releases are allowed and behavior is consistent with other deps; ensure you run npm install / yarn install afterwards to update lockfile.
🤖 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 54: The engines.node field is overly constrained by the devDependency
(ESLint) requirement; update the "engines.node" entry in package.json to reflect
the library's actual runtime support (e.g., ">=15.0.0" to match availability of
require('timers/promises') used in lib/uploads.js, or ">=20.0.0" if you intend
to require Node 20+), removing the ESLint-driven exclusions (21.x, 23.x). Also
document the stricter developer/CI toolchain version in .nvmrc or Volta instead
of package.json so ESLint remains a dev-only constraint.
---
Duplicate comments:
In `@package.json`:
- Line 32: The package.json dependency for axios was previously flagged but is
now correctly set to a caret range; confirm that the "axios" entry in
package.json uses "^1.13.5" and no further change is needed—if you see any other
package.json entries or lockfile inconsistencies, run npm install / yarn install
to refresh the lockfile and ensure the caret range is reflected consistently.
---
Nitpick comments:
In `@package.json`:
- Around line 36-45: The eslint devDependency is pinned to an exact version
("eslint": "10.0.1") while sibling devDeps use ranges; update the "eslint" entry
in package.json to use a tilde range (e.g., change "10.0.1" to "~10.0.1") so
patch releases are allowed and behavior is consistent with other deps; ensure
you run npm install / yarn install afterwards to update lockfile.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
eslint.config.mjspackage.json
🚧 Files skipped from review as they are similar to previous changes (1)
- eslint.config.mjs
wesleyschlenker
force-pushed
the
deps-and-eslint
branch
from
7d85921 to
f7a3a9d
Compare
wesleyschlenker
left a comment
wesleyschlenker
left a comment
There was a problem hiding this comment.
Looks ready to me
2 participants
Summary by CodeRabbit
Chores
Refactor
Tests