fix(expect-type): check this param in functions
#252
Conversation
| expectTypeOf<AnyThisParam>().thisParam.toBeAny(); | ||
| }) | ||
|
|
||
| test('Of course, `.toEqualTypeOf` also distinguishes the `this` argument between functions', () => { |
There was a problem hiding this comment.
| test('Of course, `.toEqualTypeOf` also distinguishes the `this` argument between functions', () => { | |
| test('`.toEqualTypeOf` also distinguishes the `this` argument between functions', () => { |
I also wonder whether this test is stretching a bit to be part of the docs. If I were someone who used this, I would probably just assume it worked (which I suppose is why you added ‘of course’)!
There was a problem hiding this comment.
Sorry to take long. Is it possible to make a test that doesn't get included in the docs?
| }) | ||
|
|
||
| test('Of course, `.toEqualTypeOf` also distinguishes the `this` argument between functions', () => { | ||
| type NoThisParam = (a: number) => void |
There was a problem hiding this comment.
I’m glad you’ve done them, but I’m not sure so many tests are needed. Should be sufficient to show that the this param matters by comparing two. After that point isn’t it just re-testing DeepBrand?
There was a problem hiding this comment.
You may be right, but still I'd prefer to have them all to be honest. I may be re-testing DeepBrand in a way, but I prefer the test to be fully self-contained. If this test passes, I am sure my feature works as expected. If I rely on DeepBrand implicitly, something might change in the DeepBrand implementation and the test could still pass. I want the test to be a complete specification of what the feature must do...
| type UnknownThisParam = (this: unknown, a: number) => void | ||
| type AnyThisParam = (this: any, a: number) => void | ||
|
|
||
| // `NoThisParam` and `UnknownThisParam` are the only ones that should be considered equivalent. |
Codecov Report
@@ Coverage Diff @@
## main mmkal/ts#252 +/- ##
=======================================
Coverage 99.88% 99.88%
=======================================
Files 46 46
Lines 907 907
Branches 165 165
=======================================
Hits 906 906
Misses 1 1
Continue to review full report at Codecov.
|
|
Hi @mmkal, sorry to take long, my last commit fixed the |
|
Closing in favour of mmkal/expect-type#15 |
Fixes #7 Porting over mmkal/ts#252 - thank you @papb for creating that one.
Closes mmkal/expect-type#7
I also added
.thisParam. So this PR is a mix of fix and feat. Should it have been two separate PRs?