Add an options parameter to folder() - #967
Open
patrickkettner wants to merge 2 commits into
Open
Conversation
package.json maps ./lib/index to ./dist/jszip.min.js for bundlers, so browserify wrapped the previously released bundle instead of building lib/. The browser tests were testing the old code.
folder(name, options) accepts the same options as file() (permissions, date, comment...). The options only apply to the specified folder, not to the automatically created parent folders. Fixes Stuk#341
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Fixes #341
folder(name, options)accepts the same options asfile()(permissions, date, comment...). As discussed in the issue, the options only apply to the specified folder, not to the automatically created parent folders:zip.folder("a/b", options)behaves likezip.folder("a").folder("b", options). Options on an already existing folder are still ignored, the existing test pins that too. Docs andindex.d.tsare updated.The first commit fixes a build problem this PR would otherwise trip over: the
browserfield in package.json redirects./lib/indexto./dist/jszip.min.js, sogrunt buildbundles the previously released code instead oflib/, and the browser tests run against that old bundle (the new tests here fail without it, verified on chromium and firefox). WithbrowserField: falsethe build bundles the current sources and all 3314 browser tests pass. Two side effects to know about before a release build uses this: the unminified bundle grows (~374KB to ~463KB) because the realreadable-streamis bundled instead oflib/readable-stream-browser.js, andJSZip.support.nodestreambecomestruein the browser bundle. Happy to split that commit into its own PR if preferred.dist/is untouched, as it only changes in release commits.