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

Requires complete listener #1

Closed
elniffi opened this issue Apr 23, 2016 · 2 comments
Closed

Requires complete listener #1

elniffi opened this issue Apr 23, 2016 · 2 comments

Comments

@elniffi
Copy link

elniffi commented Apr 23, 2016

So just using this very basic example below to start messing around with xstream in Node.JS it looks like the code assumes there is a "complete" event handler. If I remove it it actually throws an exception when it triggers because it's trying to a run a function that does not exist.

'use strict';

var xs = require('xstream').default;

var stream = xs.periodic(50)
    .filter(i => i % 2 === 0)
    .map(i => i * 2)
    .endWhen(xs.periodic(5000).take(1));

stream.addListener({
    next: i => console.log(i)
});

/Users/elniffi/Documents/Code/xstream-testing/node_modules/xstream/lib/core.js:874
            a[0]._c();
                 ^

TypeError: a[0]._c is not a function
    at Stream._c (/Users/elniffi/Documents/Code/xstream-testing/node_modules/xstream/lib/core.js:874:18)
    at EndWhenOperator.end (/Users/elniffi/Documents/Code/xstream-testing/node_modules/xstream/lib/core.js:308:18)
    at OtherListener._n (/Users/elniffi/Documents/Code/xstream-testing/node_modules/xstream/lib/core.js:278:17)
    at Stream._n (/Users/elniffi/Documents/Code/xstream-testing/node_modules/xstream/lib/core.js:849:18)
    at TakeOperator._n (/Users/elniffi/Documents/Code/xstream-testing/node_modules/xstream/lib/core.js:789:17)
    at Stream._n (/Users/elniffi/Documents/Code/xstream-testing/node_modules/xstream/lib/core.js:849:18)
    at Timeout.intervalHandler [as _repeat] (/Users/elniffi/Documents/Code/xstream-testing/node_modules/xstream/lib/core.js:196:45)
    at Timeout.wrapper [as _onTimeout] (timers.js:408:11)
    at tryOnTimeout (timers.js:224:11)
    at Timer.listOnTimeout (timers.js:198:5)

Using Node.js 5.10.1 and Xstream 1.0.2

@staltz
Copy link
Owner

staltz commented Apr 23, 2016

Hi, thank you for taking your time to send us this issue. We appreciate it, however, this is by design. It's required, for two reasons:

  • There should not be many addListener uses in your code. xstream is not meant for that, it assumes you have a few uses (hence manageable amount) of that
  • Having addListener polymorphic (types addListener(next: Function) and addListener(listener)) would be a bit of a hit on performance, and we try to keep it fast.

If you don't need to listen to error nor complete, just pass a noop function () => {}.

I'll close this issue unless after discussion we agree that it would make sense to change. Thank you for your time and I hope it's ok with you.

@staltz staltz closed this as completed Apr 23, 2016
@elniffi
Copy link
Author

elniffi commented Apr 23, 2016

That's totally fine with me, thank you for the response and great work!

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

2 participants