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

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

Merged
merged 1 commit into from
Jan 17, 2019

Conversation

johnmalkovich100
Copy link

@johnmalkovich100 johnmalkovich100 commented Dec 11, 2018

We are actively using exceljs 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 that node-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 the unzipper lib is a well supported fork of the node-unzip lib and it's pretty robust.

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

Thanks a lot for the cool exceljs lib! 🤝

@alubbe
Copy link
Member

alubbe commented Dec 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

@alubbe
Copy link
Member

alubbe commented Dec 18, 2018

@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

@defshift defshift merged commit b5c78cd into exceljs:master Jan 17, 2019
@CaioFugii
Copy link

unzipper package has no support for *.xls files.

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 this pull request may close these issues.

5 participants