fix: improve typing of flatten to infer return iterator type #22
Conversation
Also add a test for flatten, based on flatten.test.ts
Codecov Report
@@ Coverage Diff @@
## master #22 +/- ##
==========================================
+ Coverage 92.02% 93.25% +1.22%
==========================================
Files 8 8
Lines 163 163
Branches 29 29
==========================================
+ Hits 150 152 +2
+ Misses 13 11 -2
Continue to review full report at Codecov.
|
| assert.equal(iterator.next().done, true) | ||
| }) | ||
| }) | ||
| describe('flatten', () => { |
There was a problem hiding this comment.
This test seems to be the exact same as in flatten.test.ts, so I don't understand the purpose? All the other tests in this file do not have tests in external files because they are implemented on the instance itself instead of external classes
There was a problem hiding this comment.
The main point for myself was to make sure that the type inference matches appropriately. Otherwise, it's just a smoke test that improves coverage. Perhaps it would be best to define the tests in a way to use them in both calling contexts.
There was a problem hiding this comment.
Got it. Let's leave this out for now to not duplicate code. Type inference should be tested with https://github.com/Microsoft/dtslint but I don't have that set up yet.
In the future I would see this library use functions like RxJS instead of prototype methods so that would eliminate the coverage concerns.
|
@felixfbecker is this PR waiting for something else? |
|
@cojack please read the discussion above. |
|
@cojack thanks for the reminder! i forgot i had this one open. I'll get to it soon, and I can also set up dtslint or https://www.npmjs.com/package/tsd in a new PR. |
3 participants
Also add a test for flatten, based on flatten.test.ts