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

Problem with new streams API #32

Open
glenjamin opened this issue Mar 12, 2013 · 6 comments
Open

Problem with new streams API #32

glenjamin opened this issue Mar 12, 2013 · 6 comments

Comments

@glenjamin
Copy link

I can't quite figure out what's causing this, but I've got a reproducible testcase (on my OS X machine, anyway).

Sample file:

var lazy = require("lazy"),
fs = require("fs");

fs.writeFileSync('./data.txt', "1\n2\n3", "utf8")

var stream = fs.createReadStream('./data.txt');

new lazy(stream)
  .lines
  .forEach(function(line) {
    console.log(line.toString());
  });

On 0.8

> n use 0.8.* temp.js
1
2
3

On 0.10

> n use 0.10.* temp.js

I tried calling stream.resume() to force it into old stream mode, but this kicked it into an infinite loop (stack limit reached) outputting the first item repeatedly.

Discovered by @swestcott

@swestcott
Copy link

Confirmed on OS X 10.8 too.

@davemkirk
Copy link

Same issue on windows too.

@NathanaelA
Copy link

Ditto on Linux. Same (recursive) issue with stream.resume and other work around that I have been trying also.

@NathanaelA
Copy link

Fix should be in: #33 --- thanks for the test case; that narrowed down why the other module was breaking. The issue wasn't the new streams interface, it was that Node 10 also made some changes to the EventEmitter. After tracing down that the EE was acting really wonky I went and found a commit in the Node code base (nodejs/node-v0.x-archive#4971) that described what they changed and how the EE had to be created. However, then several things had to be changed to make it work.

@mhart
Copy link

mhart commented Jun 28, 2013

I'm still seeing this behaviour with the latest version (1.0.11) on node v0.10.11 - with at least one module anyway (https://github.com/rvagg/node-levelup). ie - there's no output.

It works fine with lazy in v0.8.x

If I comment out the // Check for v0.10 block in lazy.js, everything works fine.

This was referenced Jun 28, 2013
@mhart
Copy link

mhart commented Jun 28, 2013

Forget my last comment, wrapping the stream using new Readable().wrap(stream) seems to make it all 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

Successfully merging a pull request may close this issue.

5 participants