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
Subscriber refactor for version 8 #6817
Conversation
* `bind`. In particular, a library called Monio requires this. | ||
*/ | ||
const _bind = Function.prototype.bind; | ||
class ConsumerObserver<T> implements Observer<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.
This is from #6815
src/internal/Subscriber.ts
Outdated
try { | ||
this.partialObserver.next(value); | ||
} catch (error) { | ||
reportUnhandledError(error); |
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.
Note the difference here because we don't have to worry about deprecated error behavior.
return value && isFunction(value.next) && isFunction(value.error) && isFunction(value.complete); | ||
} | ||
|
||
export function isSubscriber<T>(value: any): value is Subscriber<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.
These were just moved.
project, | ||
|
||
// HACK: Cast because TypeScript seems to get confused here. | ||
project as (value: T, index: number) => ObservableInput<ObservedValueOf<O>>, |
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.
This was super weird, but seems like an inference quirk with TypeScript.
@@ -40,6 +41,6 @@ import { noop } from '../util/noop'; | |||
*/ | |||
export function ignoreElements(): OperatorFunction<unknown, never> { | |||
return operate((source, subscriber) => { | |||
source.subscribe(createOperatorSubscriber(subscriber, noop)); | |||
source.subscribe(createOperatorSubscriber(subscriber as Subscriber<unknown>, noop)); |
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.
Subscriber<never>
doesn't match Subscriber<unknown>
(or even Subscriber<any>
, because never
, haha)
} | ||
} else { | ||
this.destination = EMPTY_OBSERVER; | ||
this.destination = isSubscriber(destination) ? destination : createSafeObserver(destination); |
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.
important magic here!
If we have a Subscriber
, then just use that, otherwise we don't know what it is, and we need to make a safe observer.
If you step through the commits, you find out that this is all SafeSubscriber
was doing, which is why we removed it.
|
||
function bind<Fn extends (...args: any[]) => any>(fn: Fn, thisArg: any): Fn { | ||
return _bind.call(fn, thisArg); | ||
} |
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 bind stuff can be removed! Yay.
+ Removes `Subscriber.create`. + Removes `SafeSubscriber`, which is no longer necessary. BREAKING CHANGE: Subscriber.create has been removed. Please use `new Subscriber` instead.
- Move to using `new Subscriber` throughout the library - undeprecate `new Subscriber`. - Correct typings for `destination` on `Subscriber`. - Removes `createSafeSubscriber` internal function.
- updates API guardian files - updates side-effects files - fixes a couple of minor typings issues - fixes circular dependency for isSubscriber
2fe4099
to
0a22d06
Compare
This is completely based off of #6815, but taken with a commit-by-commit refactor of related pieces.
Highlights:
Subscriber
constructor is no longer deprecated. It can be used to create a "safe subscriber" likenew Subscriber(nextHandler)
ornew Subscriber({ next, error })
, etc.Subscriber.create
API has been removed.SafeSubscriber
removed. Everything is now simplified.NOTE: Please review one commit at a time. I tried to keep the individual commits small and focused.