addListener() to not require all three next, error, and complete functions

[mspoulsen]

How do you guys feel about removing this requirement?

I personally think it makes code (especially unit tests) clunky. I like the way Rx allow you to pass in 1-3 functions instead of the complete listener object. I understand the reasons you designed it this way but in unit tests we use addListener all the time.

stream.addListener(fnNext, fnError?, fnCompleted?) would be really nice :)

I can of course make a wrapper for my unit tests and I have already but same argument applies when we create small sample code in webpackbin etc. Adding the listener is pretty cumbersome, so would be nice to be able to add just the next function.

[staltz]

I know it's cumbersome, and it's primarily an experiment in API design, I wanted to make it cumbersome to do things that are not supposed to be done often. Like it's cumbersome to write shamefullySendNext because we're not supposed to do it often. Adding listeners in normal Cycle.js code is not supposed happen, because they happen in drivers. And when we do, it's important to not ignore error handlers.

That said, you use it often in unit tests. So what I'm interested in is to see how your unit tests look like. Can you show a small example?

We could end up creating a new API just for tests, like e.g. stream.assert('--a---b--c-|'), and then you wouldn't need to do addListener even in unit tests. The goal is to keep it cumbersome to write anti-patterns, and stream.addListener in Cycle.js code (excluding tests) is one. Consider that making it easy to write stream.addListener(fnNext) would make it easier to do the wrong thing in Cycle.js.

[mspoulsen]

I understand the reasons, but having an inflexible API has downsides as well. Problem is also with e.g. webpackbins (and they help us learn a lot). We could add an undocumented wrapper function like stream.subscribe(fnNext) but that probably just adds to the confusion. I would prefer overload on addListener.

Anyway, here is a unit test sample:

t('Controls action$', (t) => {

    const config    =   {
                            '.btn-backward' : { 'click': xs.of(1) },
                            '.btn-forward'  : { 'click': xs.of(1) },
                            '.btn-play'     : { 'click': xs.of(1) }
                        };

    const expected  = [ {type: "BACKWARD"}, {type: "FORWARD"}, {type: "PLAY"} ];

    const action$   = Controls({ DOM : utils.mockDOM(config) }).action$;
    const actual$   = utils.toArray(action$);

    actual$
        .addListener(utils.listen(actual => {
            t.deepEqual(actual, expected);
            t.end();
        }));

});

Here I wrap the listener in utils.listen and that works fine, but we still have the issue when spinning something up in webpackbin. I actually copy/paste from Webstorm(!).

PS: I you want to keep it the way it is, that is fine and I understand, just providing feedback 😄

[ivan-kleshnin]

Like it's cumbersome to write shamefullySendNext because we're not supposed to do it often

I also find XStream learning process encomplicated by annoying shamefullySendNext name and addListener API. And also by keeping half of the core functionality in extra. Not to mention .default ES5 hostile import style.

It was said you don't need them often in production.
But what about sandbox code?
What about presentation slides, etc, etc?

I was surprised to get much more boilerplate with XStream than with Rx.
.share() is gone, more junk come instead.

Just one example: sometimes it's not immediately obvious which flatMap*** version you need to use. The "quick and dirty" way is to try several versions and compare output.

In RxJS: change suffixes.
In XStream: import additional stuff, juggle between methods, functions and compose patterns.

You guys really need to consider functional API where all this expression problem stuff no longer exists and compose hacks are not required. MostJS exposes functional API along with object-oriented and XStream can too.

[staltz]

also find XStream learning process encomplicated by annoying shamefullySendNext name and addListener API.

If you're writing shamefullySendNext or addListener too often, you're using xstream for something it's not meant for.

Just one example: sometimes it's not immediately obvious which flatMap*** version you need to use. The "quick and dirty" way is to try several versions and compare output.

If you need any other strategy than flatten(), you're probably using xstream for something it's not meant for.

And also by keeping half of the core functionality in extra

What's in core is measured from statistics as the most common operators to get stuff done in Cycle.js apps. If you're using a lot of extra operators, you're probably using xstream for something it's not meant for.

compose is not a hack. It exists in RxJS as let, and in RxJava as compose. It's just composition with any custom operator.

[ivan-kleshnin]

If you're writing shamefullySendNext or addListener too often, you're using xstream for something it's not meant for.

I said my usecases. 10 examples to demonstrate how scan / fold work for my students contain about 100 shamefullySendNext invocations. Mostly copy-pasted, still painful...

If you need any other strategy than flatten(), you're probably using xstream for something it's not meant for.

concatMap aka .compose(flattenSequentially) is required pretty often for animations.

compose is not a hack. It exists in RxJS as let, and in RxJava as compose. It's just composition with any custom operator.

It is a hack around expression problem. It was a hack in all other implementations as well.

What's in core is measured from statistics as the most common operators to get stuff done in Cycle.js apps. If you're using a lot of extra operators, you're probably using xstream for something it's not meant for.

Aren't my usecases also a statistics?

[staltz]

Aren't my usecases also a statistics?

10 examples to demonstrate how scan / fold work for my students contain about 100 shamefullySendNext invocations.

No these are not xstream use cases. xstream is for Cycle.js or similar use case.

concatMap aka .compose(flattenSequentially) is required pretty often for animations.

This is potentially true, and we could eventually move concat and flattenSequentially into core, but we'd do that conservatively, waiting first for the community to express how often they use it.

It is a hack around expression problem. It was a hack in all other implementations as well.

I don't know what is your definition of hack, but IMO it's not a hack.

[ivan-kleshnin]

No these are not xstream use cases. xstream is for Cycle.js or similar use case.

So people are expected to

1. Know RxJS before using Xstream
2. Know Xstream from birth
3. ???

How do you propose to use it without learning it. And if learning is encomplicated it's not good.

This is potentially true, and we could eventually move concat and flattenSequentially into core, but we'd do that conservatively, waiting first for the community to express how often they use it.

Well, I've just expressed my opinion. One vote for particular things – not pretending for more.

[staltz]

How do you propose to use it without learning it. And if learning is encomplicated it's not good.

We don't need to use shamefullySendNext to teach xstream. I've been teaching RxJS without using Subject.onNext (just check all Egghead courses that I made, or the 'intro to reactive programming you've been missing'), and people are learning a lot. The same principles would apply to xstream, using xs.create(producer).

[ivan-kleshnin]

We don't need to use shamefullySendNext to teach xstream. I've been teaching RxJS without using Subject.onNext (just check all Egghead courses that I made, or the 'intro to reactive programming you've been missing'), and people are learning a lot. The same principles would apply to xstream, using xs.create(producer).

Ok, I will consider this.

[wclr]

I would make addListener polymorphic and allow to accept only one next listener as single param

stream$.addListener((msg: string) => console.log(msg))

That would remove user annoyance when have to use addListener for simple driver or in tests. And would not require ts/eslit exceptions for empty handlers.

[TylorS]

It's easy to create a small function to work in whatever way you prefer.

function listen (next, stream) {
  const listener = {
    next,
    error: Function.prototype,
    complete: Function.prototype
  }

  stream.addListener(listener)

  return () => stream.removeListener(listener)
}

const unsubscribe = listen(x => console.log(x), stream)

// later
unsubscribe()

[wclr]

It's easy to create a small function

Yes it is easy, but not every time you need this, it just creates unnecessary annoyance. Yes and it could also return "unlisten".

[staltz]

@mspoulsen this is now available in v6.6.0 :)

[mspoulsen]

Excellent! Thank you very much 👍