-
-
Notifications
You must be signed in to change notification settings - Fork 582
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
refactor: RxJS chains #1137
refactor: RxJS chains #1137
Conversation
@martinsik it looks great! You're really better at rxjs than I am 😁 |
Guys, as I know the function |
@iamnaumovnikita You mean this line? https://github.com/ngx-translate/core/pull/1137/files#diff-5a524f36ea5ac706856e90f772633f7f76a3a11e3a55e39e576045fc7c20f967R30 I think you're right, this is incorrect. I'll check it. |
I hve created PR #1411 can you review it, please? @martinsik @ocombe |
I went through the code base and refactored some RxJS usage where I thought it could be improved or where it was using old and deprecated notations. I'm adding comments where it's not obvious what I did there.
In general there are two larger changes:
subscribe(v => void, func)
calls withsubscribe({ next: v => void, error: e => void})
Refine subscribe and tap signatures ReactiveX/rxjs#4159.typeof res.subscribe === 'function'
withisObservable(res.subscribe)
.All existing tests were passing I just added one more additional test for some sync behavior.