-
Notifications
You must be signed in to change notification settings - Fork 220
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
Throws error in node v8.0.0 #236
Comments
me too |
More stacktrace? Weird thing is that it comes from here |
@soyuka I've updated the issue with my stacktrace. |
Thanks @jwhitmarsh I can't explain the error though. Archive output stream correctly inherits the Transform stream which should have a For the ease of debugging the source of the error is zip-archive-output-stream (node-compress-commons). The stream there is using @ctalkington any idea on this? |
Same here. |
I'm not sure if this helps but here goes. I got this error trying to build an Ionic project with all the latest versions and got some extra information.
|
One of the reasons why it doesn't work is this commit: nodejs/node@8e69f7e. In v8.0.0, |
Nice catch @watilde any idea on how this can be fixed? The best would be to have some kind of fallback for older nodejs versions :/. Weird that this didn't show up in nodejs changelog btw. |
For now installed Node 6.10.3 LTS to bypass the issue for the time being until a fix for v 8.0.0 is available. It worked! |
i'm leaning on before: var zip = archiver('zip').directory(pathname).finalize() after: var zip = child_process.spawn('zip', ['-r', '-', pathname]).stdout |
i'm following along with this and will work with node team as needed, seems crc32-stream may be added their testbed due to its popularity/dep usage to catch breakages earlier. i believe it was the refactoring of the zlib class that caused this (that was in the changelog) |
They moved zlib library to classes. Now in fallback process. I guess today/tomorrow will be fixed. |
We may need to wait for this patch(nodejs/node#13374) in Node.js core to fix this issue. Since the PR got some approved already, I think the patch will be landed in the next build(@jasnell is working on it nodejs/node#13322 (comment)). As @ctalkington said, to not make this happen again, we're going to keep our eyes on the module with citgm: nodejs/citgm#430. |
Same error with NodeJS v8 on TravisCI (I'm downgrading to NodeJS v7 until this is fixed)
|
Yep. The patch already landed in core and 8.1 is coming out tomorrow with the fix. |
Cool, I thought its will be longer |
Same here. |
For anyone tracking this, here's a link to the 8.1.0 Proposal pull request. |
Thanks! |
Same Issue here with node.js v8 |
@Bizarrus Have you only checked nodejs/node#13483 and tried to install node 8.1.0 ? |
Is resolved on node 8.1.2 |
Code:
Throws:
The text was updated successfully, but these errors were encountered: