-
Notifications
You must be signed in to change notification settings - Fork 934
refactor: port rules to typescript #785
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
c8aeff6
to
f11e4d3
Compare
Hmm, I'm not sure why Circle CI is erroring... Any idea what is causing this? @marionebl
|
f11e4d3
to
9f0564e
Compare
Something related to |
If |
b1674c5
to
007aed2
Compare
I updated this PR to match the latest changes in In my point of view, we have 2 options here:
Personally, I like 2 more. We don't need actual type checking in the tests, it's already checked when building or linting. I also think using multiple workers is required when porting everything to TS and Jest. What do you think @marionebl / @escapedcat? |
2d07b49
to
13e4f56
Compare
13e4f56
to
26ce258
Compare
Description
Part of #659
Motivation and Context
Just finished a couple of things here to get the full
@commitlint/rules
package to typescript. I'm happy about most of it, but for some, I still have some doubts. I know, I'm sorry, it's a huge PR. But this was the only way. 😅Strictly typed rule components
To get a uniform set of rules, we need some types. I looked to the current use cases and tried to come up with a simple draft for the types. Basically, there are
Rule
,RuleCondition
, andRuleOutcome
. There are some docblocks in the@commitlint/rules/src/types.ts
file, so please take a look. Also added some usage examples below. 😁Rules are not all using the
RuleCondition
This is something I noticed when combating TypeScript's "Type can be null" errors. I set all rules that do not use the
when
parameter explicitly toundefined
. (you can find them by searching forwhen = undefined
)To be honest, I think all rules should still implement the negation, even if it doesn't make sense. Solely to get a uniform way of working with these negations.
Export
TargetCaseType
from@commitlint/ensure
.Some of the rules are using
ensure.case
, to get the value argument correctly typed we need to reuse this type. Exporting this from the package where it "lives" was my best option. We can also define the types in@commitlint/rules
, but that didn't feel nice.Usage examples
I went for a generic-typed
Rule
interface that allows us to type all 3 parameters strictly. It defaults tonever
, so if you don't define a generic type, you should not use the value. With this, we can define a casing rule asRule<ensure.TargetCaseType>
, where the 3rd parameter will be strictly typed toensure.TargetCaseType
! 🎊🚀Example rules
Full types
How Has This Been Tested?Types of changes
Checklist: