diff --git a/.travis.yml b/.travis.yml index fc734678d..a9f810124 100644 --- a/.travis.yml +++ b/.travis.yml @@ -29,6 +29,7 @@ script: - node_modules/.bin/gulp filesize - scripts/closure/closure_compiler.sh - node_modules/.bin/gulp promisetest + - npm run promisefinallytest - npm run test:phantomjs-single - node_modules/.bin/karma start karma-dist-sauce-jasmine.conf.js --single-run - node_modules/.bin/karma start karma-build-sauce-mocha.conf.js --single-run diff --git a/gulpfile.js b/gulpfile.js index a35ba26e6..44cbf142d 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -427,6 +427,8 @@ gulp.task('promisetest', ['build/zone-node.js'], (cb) => { promisesAplusTests(adapter, { reporter: "dot" }, function (err) { if (err) { cb(err); + } else { + cb(); } }); }); diff --git a/lib/common/promise.ts b/lib/common/promise.ts index 75e0766a9..f0e4995d7 100644 --- a/lib/common/promise.ts +++ b/lib/common/promise.ts @@ -5,6 +5,10 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +interface Promise { + finally(onFinally?: () => U | PromiseLike): Promise; +} + Zone.__load_patch('ZoneAwarePromise', (global: any, Zone: ZoneType, api: _ZonePrivate) => { const ObjectGetOwnPropertyDescriptor = Object.getOwnPropertyDescriptor; const ObjectDefineProperty = Object.defineProperty; @@ -88,6 +92,9 @@ Zone.__load_patch('ZoneAwarePromise', (global: any, Zone: ZoneType, api: _ZonePr const symbolState: string = __symbol__('state'); const symbolValue: string = __symbol__('value'); + const symbolFinally: string = __symbol__('finally'); + const symbolParentPromiseValue: string = __symbol__('parentPromiseValue'); + const symbolParentPromiseState: string = __symbol__('parentPromiseState'); const source: string = 'Promise.then'; const UNRESOLVED: null = null; const RESOLVED = true; @@ -163,6 +170,16 @@ Zone.__load_patch('ZoneAwarePromise', (global: any, Zone: ZoneType, api: _ZonePr const queue = (promise as any)[symbolValue]; (promise as any)[symbolValue] = value; + if ((promise as any)[symbolFinally] === symbolFinally) { + // the promise is generated by Promise.prototype.finally + if (state === RESOLVED) { + // the state is resolved, should ignore the value + // and use parent promise value + (promise as any)[symbolState] = (promise as any)[symbolParentPromiseState]; + (promise as any)[symbolValue] = (promise as any)[symbolParentPromiseValue]; + } + } + // record task information in value when error occurs, so we can // do some additional work such as render longStackTrace if (state === REJECTED && value instanceof Error) { @@ -231,14 +248,24 @@ Zone.__load_patch('ZoneAwarePromise', (global: any, Zone: ZoneType, api: _ZonePr promise: ZoneAwarePromise, zone: AmbientZone, chainPromise: ZoneAwarePromise, onFulfilled?: (value: R) => U1, onRejected?: (error: any) => U2): void { clearRejectedNoCatch(promise); - const delegate = (promise as any)[symbolState] ? + const promiseState = (promise as any)[symbolState]; + const delegate = promiseState ? (typeof onFulfilled === 'function') ? onFulfilled : forwardResolution : (typeof onRejected === 'function') ? onRejected : forwardRejection; zone.scheduleMicroTask(source, () => { try { - resolvePromise( - chainPromise, true, zone.run(delegate, undefined, [(promise as any)[symbolValue]])); + const parentPromiseValue = (promise as any)[symbolValue]; + const isFinallyPromise = chainPromise && symbolFinally === (chainPromise as any)[symbolFinally]; + if (isFinallyPromise) { + // if the promise is generated from finally call, keep parent promise's state and value + (chainPromise as any)[symbolParentPromiseValue] = parentPromiseValue; + (chainPromise as any)[symbolParentPromiseState] = promiseState; + } + // should not pass value to finally callback + const value = zone.run(delegate, undefined, isFinallyPromise && delegate !== forwardRejection && delegate !== forwardResolution ? [] : [parentPromiseValue]); + resolvePromise(chainPromise, true, value); } catch (error) { + // if error occurs, should always return this error resolvePromise(chainPromise, false, error); } }); @@ -345,6 +372,19 @@ Zone.__load_patch('ZoneAwarePromise', (global: any, Zone: ZoneType, api: _ZonePr null): Promise { return this.then(null, onRejected); } + + finally(onFinally?: () => U | PromiseLike): Promise { + const chainPromise: Promise = + new (this.constructor as typeof ZoneAwarePromise)(null); + (chainPromise as any)[symbolFinally] = symbolFinally; + const zone = Zone.current; + if ((this as any)[symbolState] == UNRESOLVED) { + ((this as any)[symbolValue]).push(zone, chainPromise, onFinally, onFinally); + } else { + scheduleResolveOrReject(this, zone, chainPromise, onFinally, onFinally); + } + return chainPromise; + } } // Protect against aggressive optimizers dropping seemingly unused properties. // E.g. Closure Compiler in advanced mode. diff --git a/package-lock.json b/package-lock.json index 6f7fe34c9..47fd50988 100644 --- a/package-lock.json +++ b/package-lock.json @@ -257,6 +257,15 @@ "integrity": "sha1-VZvhg3bQik7E2+gId9J4GGObLfc=", "dev": true }, + "assert": { + "version": "1.4.1", + "resolved": "https://registry.npmjs.org/assert/-/assert-1.4.1.tgz", + "integrity": "sha1-mZEtWRg2tab1s0XA8H7vwI/GXZE=", + "dev": true, + "requires": { + "util": "https://registry.npmjs.org/util/-/util-0.10.3.tgz" + } + }, "assert-plus": { "version": "https://registry.npmjs.org/assert-plus/-/assert-plus-0.1.5.tgz", "integrity": "sha1-7nQAlBMALYTOxyGcasgRgS5yMWA=", @@ -7187,7 +7196,7 @@ }, "sax": { "version": "https://registry.npmjs.org/sax/-/sax-1.2.4.tgz", - "integrity": "sha1-KBYjTiN4vdxOU1T6tcqold9xANk=", + "integrity": "sha512-NqVDv9TpANUjFm0N8uM5GxL36UgKi9/atZw+x7YFnQ8ckwFGKrl4xX4yWtrey3UJm5nP1kUbnYgLopqWNSRhWw==", "dev": true }, "selenium-webdriver": { diff --git a/package.json b/package.json index 8faa94ece..1a3e65d9f 100644 --- a/package.json +++ b/package.json @@ -27,6 +27,7 @@ "lint": "gulp lint", "prepublish": "tsc && gulp build", "promisetest": "gulp promisetest", + "promisefinallytest": "mocha promise.finally.spec.js", "webdriver-start": "webdriver-manager update && webdriver-manager start", "webdriver-http": "node simple-server.js", "webdriver-test": "node test/webdriver/test.js", @@ -59,6 +60,7 @@ "@types/jasmine": "2.2.33", "@types/node": "^6.0.96", "@types/systemjs": "^0.19.30", + "assert": "^1.4.1", "clang-format": "1.0.46", "concurrently": "^2.2.0", "conventional-changelog": "^1.1.7", diff --git a/promise.finally.spec.js b/promise.finally.spec.js new file mode 100644 index 000000000..35a5c4e57 --- /dev/null +++ b/promise.finally.spec.js @@ -0,0 +1,345 @@ +'use strict'; + +var assert = require('assert'); +var adapter = require('./promise-adapter'); +var P = global['__zone_symbol__Promise']; + +var someRejectionReason = {message: 'some rejection reason'}; +var anotherReason = {message: 'another rejection reason'}; +process.on('unhandledRejection', function(reason, promise) { + console.log('unhandledRejection', reason); +}); + +describe('mocha promise sanity check', () => { + it('passes with a resolved promise', () => { + return P.resolve(3); + }); + + it('passes with a rejected then resolved promise', () => { + return P.reject(someRejectionReason).catch(x => 'this should be resolved'); + }); + + var ifPromiseIt = P === Promise ? it : it.skip; + ifPromiseIt('is the native Promise', () => { + assert.equal(P, Promise); + }); +}); + +describe('onFinally', () => { + describe('no callback', () => { + specify('from resolved', (done) => { + adapter.resolved(3) + .then((x) => { + assert.strictEqual(x, 3); + return x; + }) + .finally() + .then(function onFulfilled(x) { + assert.strictEqual(x, 3); + done(); + }, function onRejected() { + done(new Error('should not be called')); + }); + }); + + specify('from rejected', (done) => { + adapter.rejected(someRejectionReason) + .catch((e) => { + assert.strictEqual(e, someRejectionReason); + throw e; + }) + .finally() + .then(function onFulfilled() { + done(new Error('should not be called')); + }, function onRejected(reason) { + assert.strictEqual(reason, someRejectionReason); + done(); + }); + }); + }); + + describe('throws an exception', () => { + specify('from resolved', (done) => { + adapter.resolved(3) + .then((x) => { + assert.strictEqual(x, 3); + return x; + }) + .finally(function onFinally() { + assert(arguments.length === 0); + throw someRejectionReason; + }).then(function onFulfilled() { + done(new Error('should not be called')); + }, function onRejected(reason) { + assert.strictEqual(reason, someRejectionReason); + done(); + }); + }); + + specify('from rejected', (done) => { + adapter.rejected(anotherReason).finally(function onFinally() { + assert(arguments.length === 0); + throw someRejectionReason; + }).then(function onFulfilled() { + done(new Error('should not be called')); + }, function onRejected(reason) { + assert.strictEqual(reason, someRejectionReason); + done(); + }); + }); + }); + + describe('returns a non-promise', () => { + specify('from resolved', (done) => { + adapter.resolved(3) + .then((x) => { + assert.strictEqual(x, 3); + return x; + }) + .finally(function onFinally() { + assert(arguments.length === 0); + return 4; + }).then(function onFulfilled(x) { + assert.strictEqual(x, 3); + done(); + }, function onRejected() { + done(new Error('should not be called')); + }); + }); + + specify('from rejected', (done) => { + adapter.rejected(anotherReason) + .catch((e) => { + assert.strictEqual(e, anotherReason); + throw e; + }) + .finally(function onFinally() { + assert(arguments.length === 0); + throw someRejectionReason; + }).then(function onFulfilled() { + done(new Error('should not be called')); + }, function onRejected(e) { + assert.strictEqual(e, someRejectionReason); + done(); + }); + }); + }); + + describe('returns a pending-forever promise', () => { + specify('from resolved', (done) => { + var timeout; + adapter.resolved(3) + .then((x) => { + assert.strictEqual(x, 3); + return x; + }) + .finally(function onFinally() { + assert(arguments.length === 0); + timeout = setTimeout(done, 0.1e3); + return new P(() => {}); // forever pending + }).then(function onFulfilled(x) { + clearTimeout(timeout); + done(new Error('should not be called')); + }, function onRejected() { + clearTimeout(timeout); + done(new Error('should not be called')); + }); + }); + + specify('from rejected', (done) => { + var timeout; + adapter.rejected(someRejectionReason) + .catch((e) => { + assert.strictEqual(e, someRejectionReason); + throw e; + }) + .finally(function onFinally() { + assert(arguments.length === 0); + timeout = setTimeout(done, 0.1e3); + return new P(() => {}); // forever pending + }).then(function onFulfilled(x) { + clearTimeout(timeout); + done(new Error('should not be called')); + }, function onRejected() { + clearTimeout(timeout); + done(new Error('should not be called')); + }); + }); + }); + + describe('returns an immediately-fulfilled promise', () => { + specify('from resolved', (done) => { + adapter.resolved(3) + .then((x) => { + assert.strictEqual(x, 3); + return x; + }) + .finally(function onFinally() { + assert(arguments.length === 0); + return adapter.resolved(4); + }).then(function onFulfilled(x) { + assert.strictEqual(x, 3); + done(); + }, function onRejected() { + done(new Error('should not be called')); + }); + }); + + specify('from rejected', (done) => { + adapter.rejected(someRejectionReason) + .catch((e) => { + assert.strictEqual(e, someRejectionReason); + throw e; + }) + .finally(function onFinally() { + assert(arguments.length === 0); + return adapter.resolved(4); + }).then(function onFulfilled() { + done(new Error('should not be called')); + }, function onRejected(e) { + assert.strictEqual(e, someRejectionReason); + done(); + }); + }); + }); + + describe('returns an immediately-rejected promise', () => { + specify('from resolved ', (done) => { + adapter.resolved(3) + .then((x) => { + assert.strictEqual(x, 3); + return x; + }) + .finally(function onFinally() { + assert(arguments.length === 0); + return adapter.rejected(4); + }).then(function onFulfilled(x) { + done(new Error('should not be called')); + }, function onRejected(e) { + assert.strictEqual(e, 4); + done(); + }); + }); + + specify('from rejected', (done) => { + const newReason = {}; + adapter.rejected(someRejectionReason) + .catch((e) => { + assert.strictEqual(e, someRejectionReason); + throw e; + }) + .finally(function onFinally() { + assert(arguments.length === 0); + return adapter.rejected(newReason); + }).then(function onFulfilled(x) { + done(new Error('should not be called')); + }, function onRejected(e) { + assert.strictEqual(e, newReason); + done(); + }); + }); + }); + + describe('returns a fulfilled-after-a-second promise', () => { + specify('from resolved', (done) => { + var timeout; + adapter.resolved(3) + .then((x) => { + assert.strictEqual(x, 3); + return x; + }) + .finally(function onFinally() { + assert(arguments.length === 0); + timeout = setTimeout(done, 1.5e3); + return new P((resolve) => { + setTimeout(() => resolve(4), 1e3); + }); + }).then(function onFulfilled(x) { + clearTimeout(timeout); + assert.strictEqual(x, 3); + done(); + }, function onRejected() { + clearTimeout(timeout); + done(new Error('should not be called')); + }); + }); + + specify('from rejected', (done) => { + var timeout; + adapter.rejected(3) + .catch((e) => { + assert.strictEqual(e, 3); + throw e; + }) + .finally(function onFinally() { + assert(arguments.length === 0); + timeout = setTimeout(done, 1.5e3); + return new P((resolve) => { + setTimeout(() => resolve(4), 1e3); + }); + }).then(function onFulfilled() { + clearTimeout(timeout); + done(new Error('should not be called')); + }, function onRejected(e) { + clearTimeout(timeout); + assert.strictEqual(e, 3); + done(); + }); + }); + }); + + describe('returns a rejected-after-a-second promise', () => { + specify('from resolved', (done) => { + var timeout; + adapter.resolved(3) + .then((x) => { + assert.strictEqual(x, 3); + return x; + }) + .finally(function onFinally() { + assert(arguments.length === 0); + timeout = setTimeout(done, 1.5e3); + return new P((resolve, reject) => { + setTimeout(() => reject(4), 1e3); + }); + }).then(function onFulfilled() { + clearTimeout(timeout); + done(new Error('should not be called')); + }, function onRejected(e) { + clearTimeout(timeout); + assert.strictEqual(e, 4); + done(); + }); + }); + + specify('from rejected', (done) => { + var timeout; + adapter.rejected(someRejectionReason) + .catch((e) => { + assert.strictEqual(e, someRejectionReason); + throw e; + }) + .finally(function onFinally() { + assert(arguments.length === 0); + timeout = setTimeout(done, 1.5e3); + return new P((resolve, reject) => { + setTimeout(() => reject(anotherReason), 1e3); + }); + }).then(function onFulfilled() { + clearTimeout(timeout); + done(new Error('should not be called')); + }, function onRejected(e) { + clearTimeout(timeout); + assert.strictEqual(e, anotherReason); + done(); + }); + }); + }); + + specify('has the correct property descriptor', () => { + var descriptor = Object.getOwnPropertyDescriptor(Promise.prototype, 'finally'); + + assert.strictEqual(descriptor.writable, true); + assert.strictEqual(descriptor.configurable, true); + }); +}); \ No newline at end of file diff --git a/test/common/Promise.spec.ts b/test/common/Promise.spec.ts index 6f2ea565f..11f74fe05 100644 --- a/test/common/Promise.spec.ts +++ b/test/common/Promise.spec.ts @@ -215,6 +215,38 @@ describe( expect(reject()).toBe(undefined); }); + it('should work with .finally with resolved promise', function(done) { + let resolve: Function = null; + + testZone.run(function() { + new Promise(function(resolveFn) { + resolve = resolveFn; + }).finally(function() { + expect(arguments.length).toBe(0); + expect(Zone.current).toBe(testZone); + done(); + }); + }); + + resolve('value'); + }); + + it('should work with .finally with rejected promise', function(done) { + let reject: Function = null; + + testZone.run(function() { + new Promise(function(_, rejectFn) { + reject = rejectFn; + }).finally(function() { + expect(arguments.length).toBe(0); + expect(Zone.current).toBe(testZone); + done(); + }); + }); + + reject('error'); + }); + it('should work with Promise.resolve', () => { queueZone.run(() => { let value = null;