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

JSZip version 3 #405

Conversation

Landeers
Copy link

@Landeers Landeers commented Jun 6, 2018

Hi,
I had problem with jszip 2.x on Angular6 so I've made an update of jszip version, all functions that use this library are now asynchronous. I let you checkout the result if you want to merge it.
Best regards

@edi9999
Copy link
Member

edi9999 commented Jun 7, 2018

I think it is not a good idea to move to the async version of JSzip, since docxtemplater and zipping are CPU intensive tasks, using Promises will hurt performance and users can already run the generation in the background using workers or webworkers.

See #340 (comment)

@edi9999 edi9999 closed this Jun 7, 2018
@bertho-zero
Copy link

Why stay obsolete? I can not generate large docs and since JSzip v3 there have been a lot of fixes.

Why do we have to do synchronous? even if we can use workers, it should be a choice and not an obligation.

@edi9999
Copy link
Member

edi9999 commented Feb 8, 2019

JSZip v3 never got any commits since Nov 2017, so it is not that much more frequently updated.

JSZip v3 is asynchronous only.

If we update to JSZip v3, that will mean that much of the docxtemplater code will also need to be asynchronous (also in modules), and it would not be possible to run docxtemplater synchronously anymore.

If we need to make a choice between full sync or full async, I prefer full sync because it will be more performant as this is a CPU intensive task.

If there is an other zip library that works well synchronously and is maitained, I would be inclined to migrate to that.

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.

3 participants