build(deps): bump @eslint/js to ^10.0.1 and eslint to ^10.3.0#12
build(deps): bump @eslint/js to ^10.0.1 and eslint to ^10.3.0#12
Conversation
Supersedes #6 (closed before yarn.lock was updated). Addresses two Devin Review findings on PR #6: 1. yarn.lock desync — PR #6 bumped eslint to ^10.3.0 in package.json and package-lock.json but left yarn.lock pointing at eslint@npm:^9.39.2. Since the project's packageManager is Yarn Berry 4.9.2, yarn.lock is the primary lockfile and 'yarn install --immutable' would fail. Re-ran 'yarn install' so yarn.lock now resolves eslint@10.3.0 and updated transitive deps (@eslint/config-array@0.23.5, @eslint/core, etc.). 2. ESLint 10 dropped Node 18 — bumped engines.node from '>=18' to '>=20.19.0' (matching what eslint 10.x and @eslint/config-array 0.23.5 actually require) and removed Node 18.x from the CI test matrix in ci.yml + reusable-test.yml. The Setup-pnpm-via-Corepack fallback that only existed for Node 18 is no longer needed. Also fixes 18 ESLint 10 errors that surface from the new preserve-caught-error and no-useless-assignment recommended rules: - Attach { cause } to errors re-thrown in 13 catch blocks. - Drop dead initial assignments in 4 let declarations that are unconditionally overwritten before any read.
Original prompt from FlexNetOS
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
11 similar comments
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| let comparablePath; | ||
| try { | ||
| comparablePath = fs.realpathSync.native ? fs.realpathSync.native(nativePath) : fs.realpathSync(nativePath); | ||
| } catch { |
There was a problem hiding this comment.
📝 Info: Variable un-initialization in hooks.test.js is safe but removes a subtle fallback
The change from let comparablePath = nativePath to let comparablePath; at tests/hooks/hooks.test.js:66 removes an implicit fallback. Previously, if the try block threw before the assignment (e.g., if fs.realpathSync.native itself threw during property access rather than during the function call), comparablePath would retain nativePath. Now the catch block handles it with path.resolve(nativePath), which is functionally equivalent for this use case. Both nativePath and path.resolve(nativePath) resolve to the same value when nativePath is already absolute, which it will be after fromBashPath(). Safe, but the semantic intent shifted slightly from 'use raw path as fallback' to 'use resolved path as fallback'.
(Refers to lines 66-71)
Was this helpful? React with 👍 or 👎 to provide feedback.
| "@eslint/js": "^10.0.1", | ||
| "@opencode-ai/plugin": "^1.0.0", | ||
| "@types/node": "^20.19.24", | ||
| "c8": "^11.0.0", | ||
| "eslint": "^9.39.2", | ||
| "eslint": "^10.3.0", |
There was a problem hiding this comment.
🚩 ESLint major version bump (v9 → v10) with no config changes
Both eslint and @eslint/js are bumped from v9 to v10. The repo uses the flat config format (eslint.config.js with module.exports = [...]), which was introduced in ESLint v8 and is the default in v9+. The config uses js.configs.recommended, standard languageOptions, and basic rules — all of which should be stable across the v9→v10 transition. However, ESLint major versions can introduce breaking changes in rule defaults or config schema. This should be validated by CI (the lint job runs npx eslint scripts/**/*.js tests/**/*.js), so any incompatibility would surface there.
Was this helpful? React with 👍 or 👎 to provide feedback.
| }, | ||
| "engines": { | ||
| "node": ">=18" | ||
| "node": ">=20.19.0" |
There was a problem hiding this comment.
🚩 Engine constraint uses unusually specific minor version >=20.19.0
The engine field changed from >=18 to >=20.19.0. While dropping Node 18 is consistent with the CI matrix change, the specific minor version 20.19.0 (rather than simply >=20) is notable. This may be intentional to require a specific Node 20 patch that includes a needed API (e.g., path.matchesGlob used in tests/run-all.js:18 was added in Node 22 and has a fallback, but other APIs may be version-sensitive). The @types/node version ^20.19.24 suggests 20.19.x is a real Node.js release line. Worth confirming this minimum is intentional and documented.
Was this helpful? React with 👍 or 👎 to provide feedback.
| return fs.readFileSync(filePath, 'utf8'); | ||
| } catch (error) { | ||
| throw new Error(`Failed to read ${path.basename(filePath)}: ${error.message}`); | ||
| throw new Error(`Failed to read ${path.basename(filePath)}: ${error.message}`, { cause: error }); |
There was a problem hiding this comment.
📝 Info: Consistent application of { cause: error } across all error re-throw sites
The PR adds { cause: error } to 14 different new Error() re-throw sites across 11 files. This is a systematic improvement for debuggability — the cause property (standardized in ES2022/Node 16.9+) preserves the original error in the chain. The pattern is applied consistently: every catch (error) { throw new Error(...) } site now includes the cause. This is safe given the engine bump to >=20.19.0.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
What Changed
Supersedes #6 (closed before
yarn.lockwas updated). Picks up the same dependency bump and lands the two follow-ups Devin Review flagged on that PR.@eslint/jsfrom^9.39.2to^10.0.1.eslintfrom^9.39.2to^10.3.0.yarn install(Yarn Berry 4.9.2) soyarn.lockactually resolveseslint@npm:10.3.0plus the updated transitive dependencies (@eslint/config-array@0.23.5,@eslint/core, etc.). PR Bump @eslint/js from 9.39.2 to 10.0.1 #6 only updatedpackage-lock.json.engines.nodefrom>=18to>=20.19.0. ESLint 10 /@eslint/config-array@0.23.5requirenode ^20.19.0 || ^22.13.0 || >=24, so claiming>=18was contradictory and would breakyarn install/npm cifor any contributor on Node 18.'18.x'from the test matrix in.github/workflows/ci.ymland remove the matchingnode-version == '18.x'guard in.github/workflows/reusable-test.yml. The "Setup pnpm via Corepack" fallback that only existed to install pnpm@9 on Node 18 goes away with it.preserve-caught-errorandno-useless-assignmentrecommended rules:{ cause: error }to errors re-thrown in 13 catch blocks acrossscripts/ci/,scripts/lib/, andscripts/lib/install*/.letdeclarations (scripts/hooks/evaluate-session.js,scripts/hooks/observe-runner.js,tests/hooks/hooks.test.js,tests/run-all.js) that are unconditionally overwritten in bothtryandcatchbranches before any read.Why This Change
PR #6 (Devin Review) bumped
eslintto^10.3.0inpackage.jsonandpackage-lock.json, but the second commit on that PR did not regenerateyarn.lock. Since this repo'spackageManagerisyarn@4.9.2,yarn.lockis the canonical lockfile — leaving it pointing ateslint@npm:^9.39.2would causeyarn install --immutable(and any Yarn-based CI lane) to fail. Devin Review also pointed out that ESLint 10 silently broke the project's statednode >=18support contract, since ESLint 10's own engines field requires Node 20.19+. Bumping the floor to 20.19 + dropping 18 from CI keeps the engines field, the lockfiles, and CI all consistent.Testing Done
node tests/run-all.js) — same 8 pre-existing failures asmain(unrelated:lib/session-manager,opencode-plugin-hooks,build-opencode,npm-publish-surface,trae-install); my changes do not introduce or fix any of these.npm run lintpasses (eslint .is clean,markdownlintis clean).yarn installreports+ @eslint/js@npm:10.0.1, eslint@npm:10.3.0and removes the stale@eslint/config-array@npm:0.21.1and 22 other entries.package-lock.jsonregenerated;node_modules/eslintnow resolves to10.3.0andnode_modules/@eslint/jsto10.0.1.Type of Change
fix:Bug fixfeat:New featurerefactor:Code refactoringdocs:Documentationtest:Testschore:Maintenance/toolingci:CI/CD changesSecurity & Quality Checklist
Documentation
Link to Devin session: https://app.devin.ai/sessions/af73321ca16a4da6bbbbb27fc0d6bf24
Requested by: @FlexNetOS