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

subscribe ignores second argument of event emitter #2431

Open
benjaminhon opened this issue Mar 1, 2017 · 12 comments
Open

subscribe ignores second argument of event emitter #2431

benjaminhon opened this issue Mar 1, 2017 · 12 comments

Comments

@benjaminhon
Copy link

@benjaminhon benjaminhon commented Mar 1, 2017

I am using redis, which exposes the event emitter as follows with 2 arguments, without rxjs, i can get both arguments, however with rxjs, the second argument is missing.

r.on 'message', (channel, message) ->
  console.log channel, message

# => Greet, Hello

Rx.Observable.fromEvent(r, 'message').subscribe (channel, message) ->
  console.log channel, message

# => Greet, undefined

is this supposed to be the case?

@mpodlasin

This comment has been minimized.

Copy link
Contributor

@mpodlasin mpodlasin commented Mar 1, 2017

That is how it works at the moment. You can get access to the second parameter by using project function:

Rx.Observable.fromEvent(r, 'message', (channel, message) => [channel, message])
.subscribe(arr => {
  console.log(arr[0]); // logs value of channel
  console.log(arr[1]); // logs value of message
});

That being said it seems like a strong case to make fromEvent behave like bindCallback which emits an array if there are many arguments in callback.

@mpodlasin

This comment has been minimized.

Copy link
Contributor

@mpodlasin mpodlasin commented Mar 1, 2017

Does redis have an off function for canceling subscriptions? How do you stop listening to events?

@benjaminhon

This comment has been minimized.

Copy link
Author

@benjaminhon benjaminhon commented Mar 2, 2017

@mpodlasin

This comment has been minimized.

Copy link
Contributor

@mpodlasin mpodlasin commented Mar 2, 2017

That's weird. If there is no off (I literally mean method that is named "off") function on redis, then fromEvent should throw an Error, since it is not supported...

@trxcllnt

This comment has been minimized.

Copy link
Member

@trxcllnt trxcllnt commented Mar 3, 2017

We should probably update fromEvent to work with EventEmitters, as I constantly have this problem too. From what I remember, EventEmitters have on and removeListener methods. Does that sound right?

@kwonoj

This comment has been minimized.

Copy link
Member

@kwonoj kwonoj commented Mar 3, 2017

, EventEmitters have on and removeListener methods.

doesn't node evemtEmitter works already via addListener (https://nodejs.org/api/events.html#events_emitter_addlistener_eventname_listener) /removeListener ?(

export type NodeStyleEventEmitter = {
addListener: (eventName: string, handler: Function) => void;
removeListener: (eventName: string, handler: Function) => void;
};
)

@benjaminhon

This comment has been minimized.

Copy link
Author

@benjaminhon benjaminhon commented Mar 9, 2017

@Podlas29 that does the job

@kwonoj

This comment has been minimized.

Copy link
Member

@kwonoj kwonoj commented Apr 16, 2017

I'm not sure where this issue has landed - is there specific we need to resolve in this issue?

@felixfbecker

This comment has been minimized.

Copy link
Contributor

@felixfbecker felixfbecker commented May 6, 2017

It would probably be a more ergonomic API to default to an array if multiple args are provided, but it would also be a breaking change. At the same time, the workaround is very concise:

Observable.fromEvent(r, 'message', Array.of)
  .subscribe(([channel, message]) => {
    // ...
  })
@felixfbecker

This comment has been minimized.

Copy link
Contributor

@felixfbecker felixfbecker commented May 6, 2017

or you could even create an object with lodash:

Observable.from(r, 'message', partial(zipObject, ['channel', 'message']))
  .subscribe(event => {
    console.log(event.channel)
    console.log(event.message)
  })
@benjaminhon

This comment has been minimized.

Copy link
Author

@benjaminhon benjaminhon commented Jun 5, 2017

zeromq for node alse use multipart replies which requires the above

@raycarter

This comment has been minimized.

Copy link

@raycarter raycarter commented Nov 5, 2017

A simple use case about this issue and more about #3048 and PR #3049:

const Observable = require('rxjs').Observable;
const Oboe = require('oboe');

// create oboe instance
const oboe = Oboe({
  url: 'http://test-streaming.shengsoft.net',
  method: 'GET'
});

// original usage of oboe node event
oboe.on('node', '!.*', function() {
  console.log('from original oboe node event callback');
  console.log(arguments); // here 3 arguments are passed in
  console.log('');
});

// create another oboe instance
const oboe1 = Oboe({
  url: 'http://test-streaming.shengsoft.net',
  method: 'GET'
});

// use fromEventPattern to convert oboe event in Observable
const oboeObs = Observable.fromEventPattern((handler) => {
  oboe1.on('node', '!.*', handler);
});

oboeObs.subscribe(function() {
  console.log('from Observable handler');
  console.log(arguments); // !!! only the first argument is passed in
  console.log('');
});

In this simple use case Observable combines Oboe to consume a json-streaming server. The server returns only a part of a whole json object every second. Observable is introduced to process the data returned from the json-streaming server. But it got only the first argument while the original Oboe event passed 3 arguments in event handler. I have created a simple repo for you to retry what I mean.

@BioPhoton BioPhoton mentioned this issue Oct 2, 2019
30 of 89 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.