From e90479a7410e2fa284b43ac9c3e7d37b3c6fa1b5 Mon Sep 17 00:00:00 2001 From: "JiaLi.Passion" Date: Wed, 19 Jul 2017 09:28:33 +0900 Subject: [PATCH] refactor, not create a new closure _subscribe everytime --- karma-build.conf.js | 1 + karma-dist.conf.js | 1 + lib/rxjs/rxjs.ts | 99 +++++++++++++++++++++----------------- test/browser-zone-setup.ts | 3 +- test/rxjs/rxjs.spec.ts | 6 +-- 5 files changed, 62 insertions(+), 48 deletions(-) diff --git a/karma-build.conf.js b/karma-build.conf.js index 87c87a5ea..aa725a75b 100644 --- a/karma-build.conf.js +++ b/karma-build.conf.js @@ -13,5 +13,6 @@ module.exports = function (config) { config.files.push('build/lib/zone.js'); config.files.push('build/lib/common/promise.js'); config.files.push('build/lib/common/error-rewrite.js'); + config.files.push('build/lib/rxjs/rxjs.js'); config.files.push('build/test/main.js'); }; diff --git a/karma-dist.conf.js b/karma-dist.conf.js index 844aef697..7304f4c91 100644 --- a/karma-dist.conf.js +++ b/karma-dist.conf.js @@ -18,5 +18,6 @@ module.exports = function (config) { config.files.push('dist/sync-test.js'); config.files.push('dist/task-tracking.js'); config.files.push('dist/wtf.js'); + config.files.push('dist/zone-patch-rxjs.js'); config.files.push('build/test/main.js'); }; diff --git a/lib/rxjs/rxjs.ts b/lib/rxjs/rxjs.ts index cd74bf402..f1525fb84 100644 --- a/lib/rxjs/rxjs.ts +++ b/lib/rxjs/rxjs.ts @@ -19,6 +19,7 @@ declare let define: any; } }(typeof window !== 'undefined' && window || typeof self !== 'undefined' && self || global, (Rx: any) => { + 'use strict'; Zone.__load_patch('rxjs', (global: any, Zone: ZoneType, api: _ZonePrivate) => { const subscribeSource = 'rxjs.subscribe'; const nextSource = 'rxjs.Subscriber.next'; @@ -33,6 +34,14 @@ declare let define: any; Rx.Observable = function() { Observable.apply(this, arguments); this._zone = Zone.current; + + // patch inner function this._subscribe to check + // SubscriptionZone is same with ConstuctorZone or not + if (this._subscribe && typeof this._subscribe === 'function' && !this._originalSubscribe) { + this._originalSubscribe = this._subscribe; + this._subscribe = _patchedSubscribe; + } + return this; }; @@ -42,6 +51,39 @@ declare let define: any; const lift = Observable.prototype.lift; const create = Observable.create; + const _patchedSubscribe = function() { + const currentZone = Zone.current; + const _zone = this._zone; + + const args = Array.prototype.slice.call(arguments); + const subscriber = args.length > 0 ? args[0] : undefined; + // also keep currentZone in Subscriber + // for later Subscriber.next/error/complete method + if (subscriber && !subscriber._zone) { + subscriber._zone = currentZone; + } + // _subscribe should run in ConstructorZone + // but for performance concern, we should check + // whether ConsturctorZone === Zone.current here + const tearDownLogic = _zone !== Zone.current ? + _zone.run(this._originalSubscribe, this, args) : + this._originalSubscribe.apply(this, args); + if (tearDownLogic && typeof tearDownLogic === 'function') { + const patchedTearDownLogic = function() { + // tearDownLogic should also run in ConstructorZone + // but for performance concern, we should check + // whether ConsturctorZone === Zone.current here + if (_zone && _zone !== Zone.current) { + return _zone.run(tearDownLogic, this, arguments); + } else { + return tearDownLogic.apply(this, arguments); + } + }; + return patchedTearDownLogic; + } + return tearDownLogic; + }; + // patch Observable.prototype.subscribe // if SubscripitionZone is different with ConstructorZone // we should run _subscribe in ConstructorZone and @@ -51,43 +93,6 @@ declare let define: any; const _zone = this._zone; const currentZone = Zone.current; - // patch inner function this._subscribe to check - // SubscriptionZone is same with ConstuctorZone or not - if (this._subscribe && typeof this._subscribe === 'function') { - this._subscribe._zone = this._zone; - const _subscribe = this._subscribe; - if (_zone) { - this._subscribe = function() { - const args = Array.prototype.slice.call(arguments); - const subscriber = args.length > 0 ? args[0] : undefined; - // also keep currentZone in Subscriber - // for later Subscriber.next/error/complete method - if (subscriber && !subscriber._zone) { - subscriber._zone = currentZone; - } - // _subscribe should run in ConstructorZone - // but for performance concern, we should check - // whether ConsturctorZone === Zone.current here - const tearDownLogic = _zone !== Zone.current ? _zone.run(_subscribe, this, args) : - _subscribe.apply(this, args); - if (tearDownLogic && typeof tearDownLogic === 'function') { - const patchedTearDownLogic = function() { - // tearDownLogic should also run in ConstructorZone - // but for performance concern, we should check - // whether ConsturctorZone === Zone.current here - if (_zone && _zone !== Zone.current) { - return _zone.run(tearDownLogic, this, arguments); - } else { - return tearDownLogic.apply(this, arguments); - } - }; - return patchedTearDownLogic; - } - return tearDownLogic; - }; - } - } - // if operator is involved, we should also // patch the call method to save the Subscription zone if (this.operator && _zone && _zone !== currentZone) { @@ -102,12 +107,6 @@ declare let define: any; }; } const result = subscribe.apply(this, arguments); - // clean up _subscribe._zone to prevent - // the same _subscribe being used in multiple - // Observable instances. - if (this._subscribe) { - this._subscribe._zone = undefined; - } // the result is the subscriber sink, // we save the current Zone here result._zone = currentZone; @@ -118,6 +117,13 @@ declare let define: any; Observable.prototype.lift = function() { const observable = lift.apply(this, arguments); observable._zone = Zone.current; + // patch inner function this._subscribe to check + // SubscriptionZone is same with ConstuctorZone or not + if (this._subscribe && typeof this._subscribe === 'function' && !this._originalSubscribe) { + this._originalSubscribe = this._subscribe; + this._subscribe = _patchedSubscribe; + } + return observable; }; @@ -125,6 +131,13 @@ declare let define: any; Rx.Observable.create = function() { const observable = create.apply(this, arguments); observable._zone = Zone.current; + // patch inner function this._subscribe to check + // SubscriptionZone is same with ConstuctorZone or not + if (this._subscribe && typeof this._subscribe === 'function' && !this._originalSubscribe) { + this._originalSubscribe = this._subscribe; + this._subscribe = _patchedSubscribe; + } + return observable; }; diff --git a/test/browser-zone-setup.ts b/test/browser-zone-setup.ts index 57c641595..1c16e96a8 100644 --- a/test/browser-zone-setup.ts +++ b/test/browser-zone-setup.ts @@ -17,5 +17,4 @@ import '../lib/zone-spec/proxy'; import '../lib/zone-spec/sync-test'; import '../lib/zone-spec/task-tracking'; import '../lib/zone-spec/wtf'; -import '../lib/extra/cordova'; -import '../lib/rxjs/rxjs'; \ No newline at end of file +import '../lib/extra/cordova'; \ No newline at end of file diff --git a/test/rxjs/rxjs.spec.ts b/test/rxjs/rxjs.spec.ts index 906d3906f..fffc360cb 100644 --- a/test/rxjs/rxjs.spec.ts +++ b/test/rxjs/rxjs.spec.ts @@ -7,10 +7,10 @@ */ let rxjs; -if (typeof exports === 'object') { - rxjs = require('rxjs'); -} else { +if (typeof window !== 'undefined') { rxjs = (window as any).Rx; +} else if (typeof exports === 'object' && typeof module !== undefined) { + rxjs = require('rxjs'); } const Observable = rxjs.Observable; const Subscriber = rxjs.Subscriber;