Skip to content
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

obs.subscribe(next, error, complete) should bind their callbacks to undefined when present #194

Open
dead-claudia opened this issue Aug 4, 2018 · 5 comments

Comments

@dead-claudia
Copy link

dead-claudia commented Aug 4, 2018

It seems odd and inconsistent for them to have a this that's:

  1. provided by the language only
  2. not undefined like pretty much everything else

Pretty much everything else in the language binds things to undefined where necessary when a function is passed as a clear non-method, so why shouldn't this?

@dead-claudia
Copy link
Author

dead-claudia commented Aug 4, 2018

Oh, and BTW, this is an area where implementations differ:

  • RxJS' this is a private observer that shouldn't have been exposed. Edit: link
  • zen-observable and fate-observable both follow the spec correctly by ensuring this is that newly created object.

@benjamingr
Copy link

Well, honestly I always aw observables as:

class Observable {
  constructor(subscribe) { this.subscribe = subscribe; }
}

Which is a "minimal observable implementation" (just a function reference I can call to). In that scenario when calling .subscribe on the observable - I would expect the this value to be the observable I am subscribing to.

That said, I don't recall ever using it in any way ever.

@dead-claudia
Copy link
Author

@benjamingr Neither do I, and personally, I wouldn't miss it if that 3-argument prototype disappeared from the spec altogether. It's not like very many have it - most implementations that don't try to implement the spec don't even bother to implement the 3-argument variant, just the one.

@dead-claudia
Copy link
Author

I think the only times I've used it were in toy examples. The problem with it is that it's not obvious from first sight which one's next, which one's error, and which one's complete. It's something you could give a pass for with promises since there's only two (it's hard to miss a divider between two functions), but I still find myself occasionally having to do a double-take to remember it.

@appsforartists
Copy link
Contributor

appsforartists commented Aug 7, 2018

Considering the observer variant is effectively named args, I don't see a need at all for the 3 arg variant:

$.subscribe({
  next(value) {...},
  complete() {...},
  error(error) {...},
})

is a more readable/easier to understand alternative that's already supported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants