Adopt @exadev/eslint-config - #68
Conversation
Adds @exadev/eslint-config as an exact-pinned dev dependency, and bumps the pinned package manager to npm 12.0.2. Also approves @anthropic-ai/claude-code's postinstall script, which npm 12's lifecycle-script gate now blocks by default -- the dependabot investigate job invokes this CLI directly via npx, so the script genuinely needs to run.
Adopts @exadev/eslint-config's type-checked ruleset (replacing the
previous tseslint.configs.recommended, which carried no type
information at all) and fixes the resulting fallout in the two
semantic-release plugin scripts it referenced by path,
format-changelog.mjs and move-major-tag.mjs: both had no type
annotations, and eslint's type-checked rules crashed outright on a
plain .mjs file with no parserOptions/project configured for it.
Converts both to real TypeScript (.mts), typed against
semantic-release's own exported PrepareContext/SuccessContext, and
adds scripts/**/*.mts to tsconfig.json's include so they get real
type-checking. Both types' logger field resolves unsafely, since the
signale package they depend on ships no type declarations of its own
and skipLibCheck hides the resulting gap from tsc -- overridden with
a narrow, honest local type for just the log method each script
actually calls, rather than accepting the effectively-any upstream
type. Also fixes a genuine noUncheckedIndexedAccess gap in
move-major-tag.mts (nextRelease.version.split(".")[0] is
string | undefined) with an explicit guard, and drops the unneeded
async from a function that never awaits.
Updates every reference to the old .mjs paths (release.config.ts,
the CI workflow, README.md).
lint-staged.config.ts's "*.{ts,md}" pattern never matched a .mts
file, so the two semantic-release plugin scripts just converted from
.mjs would have silently bypassed pre-commit eslint/prettier on every
future edit. Widens the pattern to "*.{ts,mts,md}".
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean, well-verified adoption of @exadev/eslint-config. Cross-checked every non-trivial claim against upstream sources rather than taking the PR description at face value:
eslint.config.ts's new structure (parser-options block before...exadev) matches the package's own documented consumer pattern exactly, and its internal file-matching glob (**/*.mtsincluded) confirms the renamed plugin scripts get type-checked linting.package.json's newallowScriptsentry is a real npm 12 feature, and the one entry present (@anthropic-ai/claude-code@2.1.259) is exactly the only package inpackage-lock.jsonwithhasInstallScript: true— nothing missing, nothing extraneous.TypedPrepareContext/TypedSuccessContextcorrectly narrow semantic-release's realPrepareContext/SuccessContextexports, and the rationale (signaleships no type declarations) checks out.- The
noUncheckedIndexedAccessguard inmove-major-tag.mts(majorVersionSegment === undefined) and thecwd ?? process.cwd()fix informat-changelog.mtsare both correctly typed against the real upstream shapes. lint-staged.config.ts's.mtsglob addition is a genuine bug fix — without it the renamed plugin files would silently skip pre-commit lint/format. No other config in the repo has the same blind spot.- No stale
.mjsreferences remain anywhere.
One Nit inline: no CI coverage exercises the newly-renamed .mts semantic-release plugins before an actual push-to-main release attempt — worth a local dry-run before merging, not blocking.
Context: PR #67 (Dependabot bump of @types/node/eslint/lint-staged/typescript) touches the same package.json/package-lock.json — whichever merges first will need the other rebased. No related open issues found to reference. No prior reviews/comments existed on this PR.
Disclosure: sandboxed execution in this run couldn't reach the network for npm install/npm run lint/typecheck/format:check, so verification above relied on reading upstream sources (semantic-release's own .d.ts, @exadev/eslint-config's README and source, npm's release notes) via gh api/WebFetch rather than running the toolchain directly.
There was a problem hiding this comment.
This reviews commit 32e169e (the current HEAD), which already carries an identical APPROVED review from this same bot at this exact commit — nothing has changed since, so this pass is a re-verification rather than a fresh look. Not treating the prior review as stale/superseded: dismissal applies to a review left on an earlier commit a new push has addressed, and there is no newer commit here.
Independently checked: eslint.config.ts, tsconfig.json, the package.json/package-lock.json diff, the .mjs→.mts conversions (format-changelog.mts, move-major-tag.mts), lint-staged.config.ts, .npmrc, and the CI/README reference updates. No new correctness, security, or compatibility issues beyond what the prior review already raised.
Re-verified the one open finding still holds — 🟡 Nit [stack-default] on release.config.ts:76: confirmed via .github/workflows/ci.yml that the release job (the only place semantic-release actually executes, and therefore the only place Node's native .mts type-stripping for these plugin files is exercised) runs solely on: push to main (if: ... && github.event_name == 'push'), never on a pull request. That thread stays open, since nothing in this commit addresses it — worth a local npx semantic-release --dry-run (or otherwise exercising this branch's own plugin files) before merge, as the prior review suggested.
Also checked: semantic-release 25.0.9's own published type declarations show PrepareContext/SuccessContext.cwd as string | undefined on both types, so format-changelog.mts's cwd ?? process.cwd() fallback is correct and necessary there (used in a plain join(), which doesn't otherwise handle undefined). move-major-tag.mts passing cwd straight into execFileSync's options needs no equivalent fallback — Node's child_process cwd option already treats undefined as "use the parent process's own cwd" — so this isn't an inconsistency between the two files.
Title and description are already complete and grounded in the diff; left unchanged.
No blocking issues. Approving.
|
🗜️ Headroom context compression
|
|
🎉 This PR is included in version 1.12.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Summary
@exadev/eslint-config@2.12.3as an exact-pinned dev dependency, replacingtseslint.configs.recommended(which carried no type information) with the shared type-checked config array.@anthropic-ai/claude-code's postinstall script, which npm 12's lifecycle-script gate now blocks by default — the dependabotinvestigatejob invokes this CLI directly vianpx, so the script genuinely needs to run.release.config.ts(format-changelog.mjs,move-major-tag.mjs) to real TypeScript (.mts), typed againstsemantic-release's own exportedPrepareContext/SuccessContext. Both scripts'loggerfield resolves unsafely because thesignalepackage they depend on ships no type declarations at all andskipLibCheckhides the gap fromtsc— overridden with a narrow, honest local type for just thelogmethod each script actually calls, rather than accepting the effectively-anyupstream type. Also fixes a genuinenoUncheckedIndexedAccessgap (nextRelease.version.split(".")[0]isstring | undefined) with an explicit guard..mtsconversion would otherwise have introduced silently:lint-staged.config.ts's"*.{ts,md}"glob never matched a.mtsfile, so these two scripts would have bypassed pre-commit lint/format on every future edit. Widened to"*.{ts,mts,md}"..mjspaths (release.config.ts, the CI workflow,README.md).Test plan
npm run lintcleannpm run typecheckcleannpm run format:checkcleanactionlintclean (noaction.yml/workflow behavioural changes)