Implement breaking changes for JSZip v3. #253
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The filter method (reused by the file/folder method) created defensive copies of every single item for each call. The idea was to prevent people from updating the options or the data outside our API, but : * that pointless because zip.files contains everything * people already bypass that (see Stuk#20) * this have a performance hit with many entries * this behavior was never documented
This removes :
- options.base64 in generate()
- options.{base64,binary,dir,date} on ZipObject
- JSZip.utils
- JSZip#{crc32,utf8encode,utf8decode}
Lone survivor : JSZip.base64, because I don't know yet how to decode it
in the tests.
Calling `generate()` won't work anymore, the developer needs to specify an
output type.
"base64" was a nice default when the main way to provide a zip to a user
was to do:
window.location = "data:application/zip;base64," + zip.generate({type:"base64"});
We now have better alternatives (saveAs, blob url, etc) that aren't
limited by the number of characters you can put in an url.
Change the createFolders default value from false to true. People are used to file managers creating folders, the false value is (only ?) needed when we don't generate a regular zip file (xlsx file for example).
This commit does several things on the unit tests: - upgrade QUnit 1.11 -> 1.20 - use qunit-cli instead of qunit (this let us use a recent version of QUnit) - split the test/test.js file: we now have separated files in the test/asserts/ folder - add the test files in the jshint list - fix the indentation (3 spaces -> 4 spaces)
It will be easier to predict if the nodejs stream methods will fail.
Also update the error message to be less confusing server side ("not
supported by this platform" instead of "not supported by this browser").
While technically you can have a stream with an objectMode == true and stream ArrayBuffers or unicode strings, asking for a type here is overkill and error prone. Buffers are the best default value.
An object stream streaming blob/arraybuffer/uint8array/string is strange and I don't know if it would be useful. I prefer disabling this feature and wait for a real use case over enabling this feature, finding out that that wasn't a good idea and deprecate an awkward method.
If the ZipObject contains a nodejs stream or a worker, we will only be able to read it once. This commit let the StreamHelper to "lock" a worker chain to prevent further use.
We were lacking nodejs stream unit tests.
Closed
|
The build fails in the generation of the dist/ files with node 0.10 (triggered by the |
|
The build pass now. |
On grunt-contrib-uglify, the `preserveComments: 'some'` option has some issues. We can remove all comments but re-add the banner.
To get reproducible zip files, we need to use UTC instead of the local timezone. That means my pc (Europe/Paris) and the test server (UTC) should generate the same zip file.
This object now requires internal objects to work, exposing it is not useful anymore.
The API was awkward (the `decode` function returned a binary string) and base64 input/output provided by other methods should be enough.
It already worked, but now it is documented and tested. Before that, people used `JSZip.base64` to encode binary contents.
The new forEach method is aware of the root folder level: it will only show entries that start with `zip.root`. When used in generate methods, it means the generated zip file will be restricted to the current subfolder, fix Stuk#203.
This is the compagnion of 4950449, "[breaking change] Change the createFolders default value". The previous commit didn't use the default value for folders, this is now the case.
I forgot a file in 5fcfa93, here it is.
It uses a dependency which requires node >=0.12. On node 0.10 (used in the
travis builds) this reporter break the buid with a
Object #<Object> has no method 'gzipSync'
This reporter needs the `--verbose` flag on grunt to display its
result, this commit won't change anything to the default output.
dduponchel
force-pushed
the
breaking_changes
branch
from
1f38116 to
431a1ce
Compare
dduponchel
added a commit
that referenced
this pull request
Mar 22, 2016
Implement breaking changes for JSZip v3.
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request implements most of the changes discussed in #224:
It also add the related changes:
Regarding the nodejs support, I restricted the type to
nodebuffer:An object stream streaming blob/arraybuffer/uint8array/string is strange and
I don't know if it would be useful. I prefer disabling this feature and
wait for a real use case over this feature, finding out that that wasn't
a good idea and deprecate an awkward method.
This pull request is quite big, sorry. Each commit should be self-sufficient
and it may be easier to read them individually.