chore: convert lint-staged config to TypeScript - #32
Conversation
Node's native TypeScript type-stripping only runs unflagged from 23.6 onward; CI's actions/setup-node steps and the release job now pin the current LTS (24) to satisfy that floor, and package.json declares the real minimum via "engines". This unblocks converting lint-staged's own config to a real .ts file in the next commit.
lint-staged dynamically imports its raw config file, so a .ts config only loads without a flag once Node's native type-stripping is unflagged (23.6+, now the repository's declared minimum). Matches the repository's other *.config.ts files and uses lint-staged's own defineConfig helper for a typed Configuration object instead of a bare default export.
npm ci has been taking most of 5 minutes to install this repository's dependencies under npm 11 (bundled with the Node 24 baseline), even with a fully warm actions/setup-node cache -- against a prior baseline of well under a minute on Node 22's bundled npm. The existing 5-minute timeout on commitlint/typecheck/lint/format left too little margin and was intermittently cancelling jobs mid-install; raised to 10 minutes.
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}
This reverts commit 2fd2a43.
This reverts commit 2d41825.
Node backported "type stripping enabled by default" to both v23.6.0 and the 22.x LTS line at v22.18.0, not only to v23.6.0+ as the prior comment claimed. actions/setup-node's node-version: "22" resolves to the latest 22.x release, already past 22.18.0, so lint-staged.config.ts never needed a Node baseline bump to load as a real .ts file. Declares the actual constraint via "engines" instead of an unnecessarily high floor.
Superseded by re-review on the latest commit.
npm ci installing this repository's dependencies has intermittently taken most of 5 minutes on GitHub's shared runners even with a warm actions/setup-node cache -- observed on both Node 22 and Node 24, so it isn't tied to a specific Node version. Raised commitlint/typecheck/ lint/format's timeout-minutes from 5 to 10 for real margin.
Superseded by re-review on the latest commit.
"see the 'engines' field below" read as if the field were further down this same file, when it's actually in package.json.
Superseded by re-review on the latest commit.
lint-staged@17.4.1 declares its own engines.node of >=22.22.1, higher than the >=22.18.0 this repository previously declared -- a contributor between those two versions would satisfy this repository's stated floor but still hit an EBADENGINE warning installing lint-staged itself. Bumped to match, and reworded the comment in lint-staged.config.ts so it no longer implies the "engines" floor is set purely by the type-stripping requirement.
|
✅ Claude finished — see below for the result. View job run |
Superseded by re-review on the latest commit.
There was a problem hiding this comment.
Re-review on the latest commit (491d8c10). The one open Should-fix from my prior review — package.json's engines.node floor being looser than what lint-staged@17.4.1 itself requires — is fixed: the floor is now >=22.22.1, matching lint-staged's own declared engines.node (>=22.22.1, confirmed directly in package-lock.json). That thread, and the two earlier ones from this PR's history, were already resolved by an earlier pass; I've cleared and dismissed my own stale CHANGES_REQUESTED review from 659db9a7, which predates this fix.
No new findings on the delta since that review (only lint-staged.config.ts's comment and package.json's floor changed, both correctly).
I updated the PR description: it still quoted the pre-fix engines.node value (>=22.18.0 ...) instead of the final >=22.22.1 ..., and didn't explain why the floor is 22.22.1 rather than the type-stripping requirement's own 22.18.0. Title was already accurate and left unchanged. No open issue or PR tracks or overlaps this change.
Nothing else outstanding. Approving.
|
🎉 This PR is included in version 1.1.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
lint-staged.config.jscarried a comment explaining why it stayed plain JS: lint-staged dynamically imports its raw config file, and a.tsconfig only loads without a flag once Node's native type-stripping runs unflagged.An earlier revision of this PR mistakenly attributed that floor to Node >=23.6 only and, on that basis, raised the Node baseline, bumped CI to Node 24, and widened CI timeouts to cover a slower
npm ciunder Node 24. That premise was wrong: Node backported the same default-enable to the 22.x LTS line at v22.18.0, and CI'sactions/setup-nodenode-version: "22"pin already resolves to a release past that. Those changes were reverted.The final diff is:
lint-staged.config.ts(renamed from.js), using lint-staged's owndefineConfighelper, matching the repository's other*.config.tsfiles.package.jsonengines.nodefield (>=22.22.1 <23.0.0 || >=23.6.0): the floor is set bylint-staged@17.4.1's own declaredengines.noderequirement, which is higher than the type-stripping requirement alone (>=22.18.0) would need, with no change to CI's Node version.commitlint/typecheck/lint/format's CItimeout-minutesraised from 5 to 10:npm cihas intermittently taken most of 5 minutes on GitHub's shared runners even with a warmactions/setup-nodecache, observed on both Node 22 and Node 24, so this is independent of the Node-version revert above.Verified locally:
lint-staged --debugloads and runs the.tsconfig successfully, and typecheck/lint/format:check/actionlint all pass.