Skip to content

Commit

Permalink
commitlint: do not use empty class for None
Browse files Browse the repository at this point in the history
Empty classes result in strange issues that turn the
TS compiler into not very helpful. E.g.:
https://stackoverflow.com/questions/78953267/why-is-typescript-not-throwing-a-compile-time-error-when-constructor-is-not-used

NOTE: we still mark the new methods as deprecated to
give a hint that `instanceof` usage is better. These
tooltips will be properly shown by IDEs and linters
according to:
https://stackoverflow.com/a/62991642/544947
  • Loading branch information
knocte committed Sep 6, 2024
1 parent 1c9b0c4 commit e5a23bd
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 2 deletions.
30 changes: 28 additions & 2 deletions commitlint/fpHelpers.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,35 @@
export class None {}
interface IOption {
/**
* @deprecated it is better to use `if (foo instanceof None)` so that you can access the .value in the `else` case
**/
IsNone(): boolean;
/**
* @deprecated it is better to use `if (!(foo instanceof None))` so that you can access the .value inside the `if` block
**/
IsSome(): boolean;
}

export class None {
public IsNone(): boolean {
return true;
}
public IsSome(): boolean {
return false;
}
}
export class Some<T> {
value: T;

constructor(val: T) {
this.value = val;
}

public IsNone(): boolean {
return false;
}
public IsSome(): boolean {
return true;
}
}
export type Option<T> = None | Some<T>;

export type Option<T> = (None | Some<T>) & IOption;
9 changes: 9 additions & 0 deletions commitlint/tests/testHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@ test("testing Options", () => {
expect(typeGuard(bar)).toBe("4");
});

test("testing Is methods", () => {
let foo: Option<number> = new None();
let bar: Option<number> = new Some(2);
expect(foo.IsNone()).toBe(true);
expect(bar.IsNone()).toBe(false);
expect(foo.IsSome()).toBe(false);
expect(bar.IsSome()).toBe(true);
});

export function runCommitLintOnMsg(inputMsg: string) {
// FIXME: should we .lowerCase().startsWith("win") in case it starts
// returning Win64 in the future? thing is, our CI doesn't like this
Expand Down

0 comments on commit e5a23bd

Please sign in to comment.