feat(infra): update to eslint v10#643
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates ESLint from v9 to v10 as a maintenance update for the project's development dependencies. The upgrade includes version bumps to ESLint and related packages, cleanup of transitive dependencies in the lock file, removal of version constraints that were preventing the v10 upgrade, and an improvement to error handling in the Node.js stream utility by adding error cause chaining for better debugging.
Changes:
- Update ESLint from v9.39.4 to v10.1.0 and
@eslint/jsfrom v9.39.4 to v10.0.1 - Enhance error handling in stream.ts by adding error cause chaining to preserve original error context
- Remove dependabot ignore rules for ESLint v10 versions
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| package.json | Updated eslint and @eslint/js to v10 |
| package-lock.json | Updated all dependencies and removed transitive packages no longer needed in v10 |
| .github/dependabot.yml | Removed version constraints that were blocking ESLint v10 updates |
| packages/client-node/src/utils/stream.ts | Added error cause property when re-throwing RangeError to improve debugging |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -64,7 +64,7 @@ | |||
| "@vitest/coverage-istanbul": "^4.1.0", | |||
| "@vitest/coverage-v8": "^4.1.0", | |||
| "apache-arrow": "^21.0.0", | |||
| "eslint": "^9.39.4", | |||
| "eslint": "^10.1.0", | |||
There was a problem hiding this comment.
The PR description states that "Unit and integration tests covering the common scenarios were added" but no test files are shown in this diff. Please verify that tests were actually added as part of this PR, or clarify what was meant by the checklist item.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| throw new Error( | ||
| `The response length exceeds the maximum allowed size of V8 String: ${MAX_STRING_LENGTH} characters.`, | ||
| { cause: err }, | ||
| ) |
There was a problem hiding this comment.
The error chaining change using { cause: err } appears to be unrelated to the ESLint v10 update described in the PR. The PR description only mentions "Update eslint to v10" but this code change in stream.ts is a functional change that improves error diagnostics. Consider either: 1) documenting why this change is necessary for ESLint v10 compatibility, or 2) splitting this into a separate PR for better change tracking. This helps maintain clear commit history and makes it easier to understand the rationale for each change.
There was a problem hiding this comment.
update: remove this will emit a lint error
Summary
Update eslint to v10
Checklist