Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Issue with parseString in the new API? #34

Closed
meetamit opened this Issue Nov 1, 2012 · 2 comments

Comments

Projects
None yet
2 participants

meetamit commented Nov 1, 2012

Hi there,

(I'm trying out this module for the first time)

Running this very simple example:

var feedparser = require('feedparser');
feedparser.parseString(string)
  .on('article', console.log);

The Issue: The event handler (console.log) never gets invoked.

As I understand it, that's because the string is already parsed by the time on() gets to register the event handler. To prove it, I modified FeedParser.parseString() in the feedparser source with a setTimeout like so:

FeedParser.parseString = function (string, options, callback) {
  var fp = feedparser(options, callback);
  setTimeout(function() {
    fp.stream
      .on('error', fp.handleError.bind(fp))
      .end(string, Buffer.isBuffer(string) ? null : 'utf8'); // Accomodate a Buffer in addition to a String
  },100)
  return fp;
};

and indeed, this made things work. Am I misunderstanding something?

PS: I'm running feedparser v0.10.6 with node.js v0.8.8.

@danmactough danmactough added a commit that referenced this issue Nov 1, 2012

@danmactough danmactough Fix issue #34 .parseString() emitting too soon. All `emit()` and `cal…
…lback()` are wrapped in `process.nextTick()`. Bump version.
7059cc7
Owner

danmactough commented Nov 1, 2012

Thanks for the report. I was already tinkering with this, but I wasn't able to make it fail to prove it was an issue. You gave me a reason to push this change!

@danmactough danmactough closed this Nov 1, 2012

meetamit commented Nov 2, 2012

Thanks! And... I learned something new from this; I didn't know about process.nextTick()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment