tc39 / proposal-observable

Observables for ECMAScript
https://tc39.github.io/proposal-observable/
3.06k stars 90 forks source link

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

Open dead-claudia opened 6 years ago

dead-claudia commented 6 years ago

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 commented 6 years ago

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

benjamingr commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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.