fix(types): incorrect types for rule options#4633
fix(types): incorrect types for rule options#4633escapedcat merged 10 commits intoconventional-changelog:masterfrom
Conversation
Review Summary by QodoFix incorrect mutable types for rule option arrays
WalkthroughsDescription• Fix incorrect mutable array types in rule configurations • Change array types to readonly in scope-case and scope-enum rules • Allow const assertions without unnecessary type casting Diagramflowchart LR
A["Rule Config Types"] -->|"Update scope-case"| B["cases: readonly TargetCaseType[]"]
A -->|"Update scope-case"| C["delimiters: readonly string[]"]
A -->|"Update scope-enum"| D["scopes: readonly string[]"]
A -->|"Update scope-enum"| E["delimiters: readonly string[]"]
B --> F["Enable const assertions"]
C --> F
D --> F
E --> F
File Changes1. @commitlint/types/src/rules.ts
|
Code Review by Qodo
1. Readonly fix incomplete
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
|
@Zamiell can you add some tests? |
|
sure: Zamiell@ce22011 (this commit also satisfies the request from our robot overlords above) |
There was a problem hiding this comment.
Pull request overview
This PR adjusts @commitlint/types rule option types to accept readonly arrays (improving as const compatibility) and adds compile-time regression coverage to prevent the issue from reappearing.
Changes:
- Update rule option types to use
readonly ...[]where appropriate (including object-form configs forscope-case/scope-enum). - Add a compile-time type regression test (
rules.test-d.ts) coveringas constconfigurations. - Enable Vitest typechecking and wire the new
*.test-d.tsfile into the root typecheck TS config.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
vitest.config.ts |
Enables Vitest typechecking so compile-time type tests can run in CI. |
tsconfig.json |
Includes *.test-d.ts files and adds a project reference to @commitlint/types to support typechecking across packages. |
@commitlint/types/tsconfig.json |
Excludes *.test-d.ts from the package build output while still allowing root-level typechecking. |
@commitlint/types/src/rules.ts |
Changes several rule option array types to readonly to support as const configs. |
@commitlint/types/src/rules.test-d.ts |
Adds compile-time regression coverage for readonly-friendly rule configs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@Zamiell thanks! Please have a look at the feedback from copilot |
|
I think Copilot is hallucinating. This is a non breaking change because using |
|
Furthermore, even if this was not the case, nobody would mutate their configs AFTER passing it inside of commitlint, that doesn't make any sense. |
You would be surprised about how some devs shoot their own foot. But yeah I think to just address this, we flag this PR as a breaking-change and end of story? @escapedcat will anyway add another breaking change in next release as well, so it's the perfect time for these IMO |
|
To be clear, this is strictly not a breaking change though. Just check out my PR and try to use a mutable array to see for yourself. |
|
The only further advice I would give here, IMHO, is to reverse the order of the commits, so as to make them be proper unit tests instead of regression tests (in the sense that we should see the red CI in 1st commit but then green CI in the second). |
|
This TypeScript playground demonstrates that it is not a breaking change.
Can we squash the commits instead please? |
If you do that we can't see the red CI (the proof that there was a problem before the fix). |
|
Oh, I see. Let me see if I can reorder the commits. |
…lity Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Ok, I think it should be good to go now: I force pushed to the branch. |
|
Oh, I think it just skipped running CI on that commit because GitHub only runs CI on the most recent commit of a git push (as an optimization) and I pushed them both at the same time. Let me try reverting the last commit and then reverting the revert. |
This reverts commit fdcf183.
:D "I heard you like reverts" |
|
ok, we can see now that CI fails: https://github.com/conventional-changelog/commitlint/actions/runs/22575027298/job/65392114597?pr=4633 Do you want me to revert the revert now? |
This reverts commit 38d91ac.
|
ok, revert has been reverted, CI should go green now. |

Description
The types for several rules are marked as being mutable only, which is a common mistake when writing TypeScript libraries and is probably unintended. This pull request fixes it.
Motivation and Context
This code results in a type error:
To work around this, you have to do a pointless type assertion:
This pull request fixes the issue.
How Has This Been Tested?
n/a
Types of changes
Checklist: