tc39 / proposal-observable

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

`ExecuteSubscriber` should either memoize `unsubscribe` or defer the check #176

Open dead-claudia opened 6 years ago

dead-claudia commented 6 years ago

It seems odd that ExecuteSubscriber both checks for unsubscribe early and accesses it later, unlike everywhere else in the spec. I feel it'd make more sense to:

  1. Memoize the method and use that.
  2. Defer the check until the subscription cleanup function is called.

The second IMHO would make more sense, since it takes less memory and is more common in the rest of the ES spec.

zenparsing commented 6 years ago

I think option 2 makes sense. In that case, I think we should no longer throw a TypeError if the subscriber return value is not one of null, undefined, a function , or an object with an "unsubscribe" method (although we should still throw if the value has an "unsubscribe" property that is not callable).

I've create a PR for zen-observable based on this suggestion in https://github.com/zenparsing/zen-observable/pull/38.