From e9fc128f9281766c236505493883f3ad6ced0620 Mon Sep 17 00:00:00 2001 From: David Duponchel Date: Thu, 21 Jul 2016 23:21:32 +0200 Subject: [PATCH 1/2] Change JSZip.external.Promise implementation. Importing `es6-promise` had a side effect: this library replaces the global Promise object (#304) (or tries to, #309). This is an intended side effect from this library and while its version 4 should give us a switch for this behavior, I don't know when it will be out (the master branch of this project still auto replace the Promise). I replaced it by `vow` which match the requirements: - a Promise implementation - a lightweight one - works in IE6 - doesn't have too many dependencies If a global Promise already exists, prefer it: a native promise is likely to be better integrated anyway (unhandledRejection in node) and some libraries (zone.js for example) replace global objects (#303). I think it is enough for a semver minor version. Fix #303 #304 #309 --- documentation/api_jszip/external.md | 5 ++- lib/external.js | 5 ++- package.json | 3 +- test/asserts/external.js | 66 ++++++++++++++++++++--------- test/helpers/test-utils.js | 7 +++ test/index.html | 6 +-- 6 files changed, 67 insertions(+), 25 deletions(-) diff --git a/documentation/api_jszip/external.md b/documentation/api_jszip/external.md index c0ce2381..a480972f 100644 --- a/documentation/api_jszip/external.md +++ b/documentation/api_jszip/external.md @@ -4,12 +4,15 @@ layout: default section: api --- -JSZip uses polyfills of objects that may not exist on every platform. +JSZip uses objects that may not exist on every platform, in which case it uses +a shim. Accessing or replacing these objects can sometimes be useful. JSZip.external contains the following properties : * `Promise` : the [Promise](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise) implementation used. +The global object is prefered when available. + __Example__ ```js diff --git a/lib/external.js b/lib/external.js index cbd69ede..a9559a78 100644 --- a/lib/external.js +++ b/lib/external.js @@ -1,6 +1,9 @@ 'use strict'; -var ES6Promise = require("es6-promise").Promise; +// load the global object first: +// - it should be better integrated in the system (unhandledRejection in node) +// - the environment may have a custom Promise implementation (see zone.js) +var ES6Promise = global.Promise || require("vow").Promise; /** * Let the user use/change some implementations. diff --git a/package.json b/package.json index 0103ce3c..0acf6fdb 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,8 @@ "es6-promise": "~3.0.2", "pako": "~1.0.0", "readable-stream": "~2.0.6", - "asap": "~2.0.3" + "asap": "~2.0.3", + "vow": "~0.4.12" }, "license": "(MIT OR GPL-3.0)" } diff --git a/test/asserts/external.js b/test/asserts/external.js index 10217c2c..e1d522ba 100644 --- a/test/asserts/external.js +++ b/test/asserts/external.js @@ -1,34 +1,62 @@ /* jshint qunit: true */ -/* global JSZip,JSZipTestUtils */ +/* global JSZip,JSZipTestUtils,Promise */ 'use strict'; QUnit.module("external"); +/** + * Creates a wrapper around an existing Promise implementation to count + * calls and detect custom implementations. + * @param {Promise} OriginalPromise the promise to wrap + * @return {Promise} the wrapped promise + */ function createPromiseProxy(OriginalPromise) { - function MyShinyPromise () { - OriginalPromise.apply(this, arguments); + function MyShinyPromise (input) { + if (input.then) { // thenable, we wrap it + this._promise = input; + } else { // executor + this._promise = new OriginalPromise(input); + } MyShinyPromise.calls++; } MyShinyPromise.calls = 0; - MyShinyPromise.prototype = OriginalPromise.prototype; - function proxyMethod(method) { - if (typeof OriginalPromise[method] === "function") { - MyShinyPromise[method] = function () { - MyShinyPromise.calls++; - return OriginalPromise[method].apply(OriginalPromise, arguments); - }; - } - } - MyShinyPromise.prototype.isACustomImplementation = true; - for(var method in OriginalPromise) { - if (!OriginalPromise.hasOwnProperty(method)) { - continue; - } - proxyMethod(method); - } + MyShinyPromise.prototype = { + then: function (onFulfilled, onRejected) { + return new MyShinyPromise(this._promise.then(onFulfilled, onRejected)); + }, + "catch": function (onRejected) { + return new MyShinyPromise(this._promise['catch'](onRejected)); + }, + isACustomImplementation: true + }; + + MyShinyPromise.resolve = function (value) { + return new MyShinyPromise(OriginalPromise.resolve(value)); + }; + MyShinyPromise.reject = function (value) { + return new MyShinyPromise(OriginalPromise.reject(value)); + }; + MyShinyPromise.all = function (value) { + return new MyShinyPromise(OriginalPromise.all(value)); + }; return MyShinyPromise; } +test("JSZip.external.Promise", function (assert) { + assert.ok(JSZip.external.Promise, "JSZip.external.Promise is defined"); + assert.ok(JSZip.external.Promise.resolve, "JSZip.external.Promise looks like a Promise"); + assert.ok(JSZip.external.Promise.reject, "JSZip.external.Promise looks like a Promise"); +}); + +test("load JSZip doesn't override the global Promise", function (assert) { + if (typeof Promise !== "undefined"){ + assert.equal(Promise, JSZipTestUtils.oldPromise, "the previous Promise didn't change"); + assert.equal(Promise, JSZip.external.Promise, "JSZip.external.Promise reused the global Promise"); + } else { + assert.ok(JSZip.external.Promise, "JSZip.external.Promise is defined even if the global Promise doesn't exist"); + } +}); + test("external.Promise can be replaced in .async()", function (assert) { var done = assert.async(); var OriginalPromise = JSZip.external.Promise; diff --git a/test/helpers/test-utils.js b/test/helpers/test-utils.js index ba82717e..bbe541bd 100644 --- a/test/helpers/test-utils.js +++ b/test/helpers/test-utils.js @@ -203,5 +203,12 @@ return base64Dict[input]; }; + + if (global.JSZip) { + throw new Error("JSZip is already defined, we can't capture the global state *before* its load"); + } + + JSZipTestUtils.oldPromise = global.Promise; + global.JSZipTestUtils = JSZipTestUtils; })(typeof window !== "undefined" && window || global); diff --git a/test/index.html b/test/index.html index dfd49876..d5bdd238 100644 --- a/test/index.html +++ b/test/index.html @@ -13,6 +13,9 @@ } + + + - - - From 0d287a1c942b990163bd10b633f8cc43e9ae33d7 Mon Sep 17 00:00:00 2001 From: David Duponchel Date: Tue, 2 Aug 2016 19:13:02 +0200 Subject: [PATCH 2/2] Use lie instead of vow. The API is closer to the ES6 Promise API and it is smaller than vow. The previous version had a lot of npm dependencies (which is why I prefered `vow`) but the latest version fixed that. --- lib/external.js | 2 +- package.json | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/external.js b/lib/external.js index a9559a78..37bd7064 100644 --- a/lib/external.js +++ b/lib/external.js @@ -3,7 +3,7 @@ // load the global object first: // - it should be better integrated in the system (unhandledRejection in node) // - the environment may have a custom Promise implementation (see zone.js) -var ES6Promise = global.Promise || require("vow").Promise; +var ES6Promise = global.Promise || require("lie"); /** * Let the user use/change some implementations. diff --git a/package.json b/package.json index a1293fab..2620913d 100644 --- a/package.json +++ b/package.json @@ -53,9 +53,9 @@ "dependencies": { "core-js": "~2.3.0", "es6-promise": "~3.0.2", + "lie": "~3.1.0", "pako": "~1.0.2", - "readable-stream": "~2.0.6", - "vow": "~0.4.12" + "readable-stream": "~2.0.6" }, "license": "(MIT OR GPL-3.0)" }