NotificationsYou must be signed in to change notification settings
Fork1.9k
Star15k

Replaced node-unzip-2 to unzipper package which is more robust#708

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

defshift merged 1 commit intoexceljs:masterfromjohnmalkovich100:replace-unzip-lib

Jan 17, 2019

Merged

Replaced node-unzip-2 to unzipper package which is more robust#708

defshift merged 1 commit intoexceljs:masterfromjohnmalkovich100:replace-unzip-lib

Jan 17, 2019

Conversation

Copy link

johnmalkovich100 commentedDec 11, 2018•
edited
Loading

We are actively usingexceljs in our company and noticed that sometimes we have a situation when CPU usage and memory consumption are very high if the source stream is broken or dead.

After some investigation I noticed thatnode-unzip-2 lib is the reason of this behaviour. Unfortunately it's not well supported and we didn't find the root cause of this problem.

We've found theunzipper lib is a well supported fork of thenode-unzip lib and it's pretty robust.

In this PR,node-unzip-2 is replaced to theunzipper lib.

Thanks a lot for the cool exceljs lib! 🤝

Copy link

Member

alubbe commentedDec 11, 2018

Would be awesome to see this land. Before this change, we've seen a lot of errors and OOM crashes in production when using the streaming WorkbookReader.

@guyonroche Please let us know if you need any additional info or code to get this merged

Copy link

Member

@guyonroche any chance that you can take a look at this over the holidays? We have a fork of your repo using node-unzipper and it's working great - but we'd love to return to your npm published package