-
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
fix(reduce): change order of reduce overload type definitions #2523
Conversation
Returning type T takes precedence over returning type T[].
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.
probably we may need test cases for this to check if there's no regressions / validate this changes fixes issue. what do you think? @david-driscoll
Yeah we should have two tests. The ones from #2519 should be sufficient. Make sure you assign them to a specific type so that the types match both sides. const o1: Observable<number[]> = Observable.of([0, 1, 2, 3], [4, 5, 6, 7])
.reduce((acc, value) => acc.concat(value), [])
.map(numbers => numbers.map(n => n++))
const o2: Observable<number[]> = Observable.of(0, 1, 2, 3, 4, 5, 6, 7)
.reduce((acc, value) => { acc.push(value); return acc; }, [])
.map(numbers => numbers.map(n => n++)) |
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.
Once we get a the typings tests in this is gtg
Sorry, I rarely do pull requests on github. I'll check to see how testing works in other pull requests and get this updated. |
Generated by 🚫 dangerJS |
Add tests for reducing T[] to T[] (e.g. concatenating arrays into one array) and for reducing T to T[] (e.g. pushing items into an array).
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.
LGTM @kwonoj ?
What'd be release for this? /cc @david-driscoll |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description: Update overload type definitions of operator reduce so returning type T takes precedence over returning type T[].
Related issue (if exists): #2519