-
Notifications
You must be signed in to change notification settings - Fork 3k
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
test(dtslint): add dtslint test for scan operator (#4093) #4347
Conversation
Pull Request Test Coverage Report for Build 7645
💛 - Coveralls |
spec-dtslint/operators/scan-spec.ts
Outdated
}); | ||
|
||
it('should infer correctly with Array of T value', () => { | ||
const a = of(1, 2, 3).pipe(scan((acc: number[], val: number, i) => [...acc, val])); // $ExpectType Observable<number[]> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without a seed of []
this should effect an error. It's a flaw in the typings. I'd add the []
. I'll look at fixing the scan
and reduce
typings later; it's on my TODO list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll be good if you could tag me when you do a PR for this. I'd like to learn how to resolve this kind of problem. Thanks Nick!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The typings for scan
and reduce
have issues. These will be addressed at a later date.
The simplest thing to do with these tests would be to replace them with the tests that were written for reduce
, but change the operators and change the type expectations to be arrays instead of the the reduced values - e.g. Observable<number[]>
instead of Observable<number>
, etc.
More comprehensive tests can be written when the issues with the typings are addressed.
spec-dtslint/operators/scan-spec.ts
Outdated
}); | ||
|
||
it('should infer correctly with type change accumulator', () => { | ||
const a = of(1, 2, 3).pipe(scan((acc: string, val: number, i) => `${acc} ' ' ${val}`)); // $ $expectType Observable<string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$ $expectType
is a typo.
c5c2336
to
8d5551a
Compare
}); | ||
|
||
it('should infer correctly for accumulator of type array', () => { | ||
const a = of(1, 2, 3).pipe(scan((x: number[], y: number, i: number) => x, [])); // $ExpectType Observable<number[]> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @cartant ,
I have changed the test to identically like reduce. But, I'm not sure about the type expectations you mentioned on the comment above - look like the typings are exactly the same as reduce.
Is this correct?
Description: Add dtslint test for Scan operator
Related issue (if exists): #4093