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

Proposal: Add a "do" method, like "forEach" #50

Closed
zenparsing opened this issue Jul 30, 2015 · 18 comments
Closed

Proposal: Add a "do" method, like "forEach" #50

zenparsing opened this issue Jul 30, 2015 · 18 comments

Comments

@zenparsing
Copy link
Member

I'm happy with the decision to use an observer object instead of callbacks for "subscribe". However, this can make things a little cumbersome to type:

fromEvent(element, "click").subscribe({
    next(event) { console.log(event.x + ":" + event.y); }
});

RxJS overloads "subscribe" such that it allows a list of callbacks, in addition to an observer. I think this overload is a little questionable for the ES spec though. In ES, all "callables" are objects, so it's entirely possible for a callable object to also implement "next", "error", and "complete". I would like to avoid any heuristic overloading.

Previously, we had a "forEach" method which accepted a single callback and returned a Promise for the completion of the stream. I'd like to bring that back, with some minor adjustments.

Proposal: add a "do" method which accepts a callback, subscribes to the observable and returns a promise for the completion value of the stream.

element.on("click").do(event => {
    console.log(event.x + ":" + event.y);
});

Observable.from(something).do(x => {
    // Process each item in the stream
}).then(_=> {
    // Do something after the stream is complete
}).catch(err => {
    // Catch errors
});

The "do" method would subscribe synchronously, and would provide no built in cancelation mechanism. Instead, a combinator could be used for early completion. For example:

let cancel, cancelToken = new Promise(resolve => cancel = resolve);

element.on("click").takeUntil(cancelToken).do(event => { 
    // Process event
});

cancel();
@zenparsing
Copy link
Member Author

@zenparsing
Copy link
Member Author

BTW @domenic @jhusain @annevk the above is basically my proposal for using observable as a next-gen addEventListener api.

To repeat:

element.on("click", options).do(event => {
    // whateva
});

The "on" method returns an Observable, which can be filtered, mapped, etc. and then listened to with "do".

@benlesh
Copy link

benlesh commented Jul 30, 2015

I would prefer forEach or observe rather than do because the latter is already in use and well understood in RxJS.

@benlesh
Copy link

benlesh commented Jul 30, 2015

Also, FWIW, I think it would be better to have await understand Observable completion than have anything return a promise. If that were the case, do could work just like it does in RxJS today.

@jhusain
Copy link
Collaborator

jhusain commented Jul 30, 2015

Are you concerned about similarity to event emitter?

JH

On Jul 29, 2015, at 6:48 PM, zenparsing notifications@github.com wrote:

BTW @domenic @jhusain @annevk the above is basically my proposal for using observable as a next-gen addEventListener api.

To repeat:

element.on("click", options).do(event => {
// whateva
});
The "on" method returns an Observable, which can be filtered, mapped, etc. and then listened to with "do".


Reply to this email directly or view it on GitHub.

@zenparsing
Copy link
Member Author

@Blesh I'm strongly opposed to implicit conversion from Observable to promise. : )

The challenge that I'm trying to address is the desire to provide an API which is ergonomically competitive with EventEmitter for users that don't really care about Rx, and don't particularly want to know any more about Observable than they have to.

With EventEmitter, you have:

object.on("type", event => {
    // Do stuff
});

Which is about as ergonomic as you can get. forEach and observe add just enough characters to make observables less appealing for those users.

Given that observable's are lazy, I really want a word that implies strong "action" and will differentiate it from the combinators. To me, "forEach" and "observe" sound passive.

Also, in JS the "do" keyword is associated with immediate execution as opposed to registering side-effects.

@zenparsing
Copy link
Member Author

@jhusain That's a good point. Node's EventEmitter will throw if the second argument is not a function, so it's totally possible to create a backward-compatible overload.

https://github.com/joyent/node/blob/master/lib/events.js#L143

EventEmitter.prototype.addListener returns this, so the overload would have to have a different return type. That's a bit messy, but I think observables are worth it.

@benlesh
Copy link

benlesh commented Jul 30, 2015

To be fair, I'm strongly opposed to promises in general. I don't think of it as implicit conversion, I think of it as await being more useful. It should just subscribe to the observable and await completion, returning the last value.

@benlesh
Copy link

benlesh commented Jul 30, 2015

Regardless, I feel like the completion value on observable should probably go away. I was on board with it, but I can't seem to find the utility of it.

@jhusain
Copy link
Collaborator

jhusain commented Jul 30, 2015

Isn't the utility of the completion value that we can continue to drive generators with Observers?

@benlesh
Copy link

benlesh commented Jul 30, 2015

But sending a value into a generator via return doesn't do anything but call the finally block if one exists, right?

@jhusain
Copy link
Collaborator

jhusain commented Jul 30, 2015

A generator might want to be communicated a final computation value. This is for the co-routine case in which two algorithms are exchanging values.

JH

On Jul 29, 2015, at 9:34 PM, Ben Lesh notifications@github.com wrote:

But sending a value into a generator via return doesn't do anything but call the finally block if one exists, right?


Reply to this email directly or view it on GitHub.

@benlesh
Copy link

benlesh commented Jul 30, 2015

But if you have a generator:

function* myGeneratorObserver() {
  try {
    while(true) {
       var value = yield;
       console.log('next: ' + value);
    }
  } catch (err) {
    console.log('error: ' + err);
  } finally {
    console.log('done');
  }
}

and you call return on it:

var generatorObserver = myGeneratorObserver();
// primed
generatorObserver.next();

// and return
generatorObserver.return('where will this go?');

The return value isn't used for anything. What good is pushing something at it?

@annevk
Copy link
Member

annevk commented Jul 30, 2015

@zenparsing do you get the same timing as you do today? E.g. the canceling mechanism doesn't seem to integrate well with events firing synchronously.

@zenparsing
Copy link
Member Author

@Blesh I agree that a value supplied to "complete" doesn't seem particularly useful. On the other hand, I can't think of a good reason to not forward the argument from the normalized observer's "complete" method to the observer's "complete".

@annevk With this spec, observables send their "next" values synchronously, so per-event things like preventDefault and stopPropagation would continue to work just fine.

As an exercise, I rewrote addEventListener and removeEventListener on top of an Observable-producing on method here:

https://github.com/zenparsing/es-observable/blob/master/dom-event-dispatch.md

@annevk
Copy link
Member

annevk commented Jul 30, 2015

@zenparsing cool, seems compelling. Glad we haven't specified/implemented https://gist.github.com/annevk/5238964 yet.

@benlesh
Copy link

benlesh commented Jul 30, 2015

@zenparsing I guess what I'm saying is I can't think of a good reason to pass a value to complete at all...

For a while I thought it was a good idea, not now I'm not so sure.

  1. it doesn't compose through operators like flatMap or merge, not without some sort of weird additional aggregation function
  2. it creates confusion between the value of Observables of one and Observables of none that return a value.

The second issue is a bigger deal, IMO. An Observable of 1 composes through any other observable operation. An Observable of none with a completion value is just an oddity. There would have to be totally different operations added for those, and maybe some sort of weird context switching operators to switch a nexted value or values over to be a completion value? reduceAndComplete? asCompletion? And what about the other direction? completion value -> observable of 1? toScalarObservable?

The more I think about it, the more I'm not sure it makes sense.

@zenparsing
Copy link
Member Author

Switched back to "forEach"

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

4 participants