fix(lint-staged-config): stop mutating the exported extension defaults - #1118
Conversation
`defineConfig` seeded its defaults with the exported `eslintExtensions`,
`typescriptExtensions` and `stylesheetsExtensions` tuples by reference, then
pushed "md" onto the ESLint one when no markdownlint CLI was present. The push
mutated the exported constant, so a second `defineConfig()` call in the same
process saw "md" already there and appended it again. The defaults are now
copied per call.
The existing test did not catch this because it built its expectation from the
same mutated array, comparing the damage against itself. It now spells out the
appended "md" explicitly.
Each option was also read as `config.eslint as EslintConfig` at every use, which
covered up that the value is `EslintConfig | false | undefined`. With the option
unset the assertion produced a property read on `undefined` and a TypeError
rather than the intended "extensions option is required" error. Each option is
now narrowed once into a local.
`getNearestConfigPath` asserted `startsWith("/")` results into the
`` `/${string}` `` template literal type at four call sites. That check is now an
`isAbsolutePath` predicate, so the narrowing happens in the type system.
Two assertions remain, each documented: `Join` cannot be derived from
`path.join`, and the package root is an ancestor of `cwd` so it cannot be proven
to be the same `A`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014FP75FgnREe4L45kZtsa9a
|
Warning Review limit reached
Next review available in: 32 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
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 |
|
Thank you for following the naming conventions! 🙏 |
The @anolilab/eslint-config 29.0.0 release repointed lint-staged-config's
manifest at the new version but left pnpm-lock.yaml recording 28.1.3. Every job
on main therefore dies at `pnpm install --frozen-lockfile`:
[ERR_PNPM_OUTDATED_LOCKFILE] Cannot install with "frozen-lockfile" because
pnpm-lock.yaml is not up to date with <ROOT>/packages/lint-staged-config/package.json
- @anolilab/eslint-config (lockfile: 28.1.3, manifest: 29.0.0)
Regenerate it. Verified with the same frozen install CI runs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014FP75FgnREe4L45kZtsa9a
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
commit: |
1 participant
mainis currently red.@anolilab/eslint-config@29.0.0shipped from #1115, semantic-release repointedlint-staged-configat it, and that package lints itself with the published config — so it now hits the new rules with 28 errors. #1117 inherits the failure because its lint job covers every project.Verified against a clean
origin/maincheckout: the 28 errors are onmain, not introduced by #1117.Fixing the violations rather than weakening the rules. Two of them turned out to be real defects.
The exported defaults were being mutated
defineConfigseeded its defaults with the exported tuples by reference:and then, when no markdownlint CLI is installed, did
extensions.push("md").as constmakes those tuples readonly to the type checker but they are ordinary mutable arrays at runtime, and theas string[]assertion at the push site removed the one guard that would have caught it. So the push mutated the module-level exported constant: a seconddefineConfig()call in the same process saw"md"already present and appended it again.The defaults are now copied per call.
The existing test did not catch this because it built its expectation from
eslintExtensions.join(",")— the same array the implementation had already mutated. It was comparing the damage against itself and passing. It now spells out the appended"md".An unset option crashed instead of erroring
Every option was read as
config.eslint as EslintConfigat each use, but the declared type isEslintConfig | false | undefined. With the option unset, the assertion produced a property read onundefined— aTypeError— instead of the intendedThe extensions option is required...error. Each option is now narrowed once into a local.Template-literal paths
getNearestConfigPathassertedstartsWith("/")results into`/${string}`at four call sites, becausestartsWithdoes not narrow to a template literal type. That is now anisAbsolutePathtype predicate, so the narrowing happens in the type system instead of being asserted away.Remaining assertions
Two, each one line with the reason inline:
Join<T, "/">cannot be derived frompath.join'sstringreturn.cwd, so it cannot be proven to be the sameA. The generic overstates what the function knows; correcting it would change the public signature and belongs in its own change.One
no-restricted-typessuppression:Extract<Configuration, Record<string, unknown>>selects lint-staged's object task-map from its config union. That is a union filter, not a dictionary contract, so the rule does not apply.Verification
tsc --noEmitcleanpnpm run lint:eslint— 6/6 projects, 0 errors (was 28)pnpm run build:packages— 8/8pnpm run test:all— 4/4 projects, 20 tests🤖 Generated with Claude Code
https://claude.ai/code/session_014FP75FgnREe4L45kZtsa9a