AvianFlu / ntwitter

Asynchronous Twitter REST/stream/search client API for node.js
https://github.com/AvianFlu/ntwitter
Other
785 stars 163 forks source link

support Stream interface #3

Open dominictarr opened 12 years ago

dominictarr commented 12 years ago

hey this is really cool, but it would be even cooler if it supported the Stream interface.

like, you could change this:

twit.stream('user', {track:'nodejs'}, function(stream) {
    stream.on('data', function (data) {
        console.log(console.dir(data));
    });
    // Disconnect stream after five seconds
    setTimeout(stream.destroy, 5000);
});

to this:

twit.stream('user', {track:'nodejs'}).pipe(writable)

I'm also working on a toolkit for working with streams, https://github.com/dominictarr/event-stream that might have some useful examples.

AvianFlu commented 12 years ago

I'm completely in favor of this. When I get some time next, I'll look at the custom stream parser this uses, and see if I can make it inherit from stream.Stream like it should.

dominictarr commented 12 years ago

you could also just wrap the api in a instance of stream,

so that twit.stream(opts) returns a new Stream, that the data event gets emitted off,
rather than an event emitter being passed to the callback. (actually, it looks like it might callback a stream instance)

then, maybe, streamify the underling implmentation later.

fent commented 12 years ago

This looks like it's really simple to do. Just replace EventEmitter with Stream here https://github.com/AvianFlu/ntwitter/blob/master/lib/parser.js#L13

and return the stream object instead of the ntwitter instance here https://github.com/AvianFlu/ntwitter/blob/master/lib/twitter.js#L284

And then add pause() and resume() methods to it that pause/resume the response object.

dominictarr commented 12 years ago

and destroy()

isn't the response line seperated json?

if so just pipe it through https://github.com/dominictarr/event-stream/blob/master/index.js#L396-417 and https://github.com/dominictarr/event-stream/blob/master/index.js#L478-489

hmm, just realized parse is undocumented...

On Wed, Jul 18, 2012 at 1:16 PM, Roly Fentanes reply@reply.github.com wrote:

This looks like it's really simple to do. Just replace EventEmitter with Stream here https://github.com/AvianFlu/ntwitter/blob/master/lib/parser.js#L13

and return the stream object instead of the ntwitter instance here https://github.com/AvianFlu/ntwitter/blob/master/lib/twitter.js#L284

And then add pause() and resume() methods to it that pause/resume the response object.


Reply to this email directly or view it on GitHub: https://github.com/AvianFlu/ntwitter/issues/3#issuecomment-7064413

fent commented 12 years ago

I created a module for parsing coutinuous json strings

https://github.com/fent/node-jstream

Might be faster for this since data can be streamed into the parser. It uses clarinet

dominictarr commented 12 years ago

you should also use this: https://github.com/fent/node-streamify

then instead of going:

twitter.stream(query, function (err, stream) {
  stream.pipe(...)
})

you could do this:

twitter.stream(query).pipe(whereEver)

which is way less typing!

AvianFlu commented 12 years ago

I think that's a great idea. If somebody doesn't beat me to it with a PR, I'll have a look this weekend.