Skip to content
This repository was archived by the owner on Feb 26, 2024. It is now read-only.

Commit 1ed83d0

Browse files
JiaLiPassionmhevery
authored andcommitted
feat(rxjs): fix #830, monkey patch rxjs to make rxjs run in correct zone (#843)
* feat(rxjs): fix #830, monkey patch rxjs to make rxjs run in correct zone * make test pass * add karma config * pass compile-esm * change rxjs to umd * change spec to pass test * add comment, patch Observable.create * refactor, not create a new closure _subscribe everytime * fix set wrong object * begin to apply observable methods, 1: bindCallback * fix async scheduler reset zone issue * change require to import * still need require in rxjs.spec * make build and dist pass test * remove meta def * make import work on both build and dist * continue to add cases
1 parent ab82852 commit 1ed83d0

18 files changed

+760
-7
lines changed

Diff for: gulpfile.js

+10
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,14 @@ gulp.task('build/sync-test.js', ['compile-esm'], function(cb) {
199199
return generateScript('./lib/zone-spec/sync-test.ts', 'sync-test.js', false, cb);
200200
});
201201

202+
gulp.task('build/rxjs.js', ['compile-esm'], function(cb) {
203+
return generateScript('./lib/rxjs/rxjs.ts', 'zone-patch-rxjs.js', false, cb);
204+
});
205+
206+
gulp.task('build/rxjs.min.js', ['compile-esm'], function(cb) {
207+
return generateScript('./lib/rxjs/rxjs.ts', 'zone-patch-rxjs.min.js', true, cb);
208+
});
209+
202210
gulp.task('build/closure.js', function() {
203211
return gulp.src('./lib/closure/zone_externs.js')
204212
.pipe(gulp.dest('./dist'));
@@ -237,6 +245,8 @@ gulp.task('build', [
237245
'build/async-test.js',
238246
'build/fake-async-test.js',
239247
'build/sync-test.js',
248+
'build/rxjs.js',
249+
'build/rxjs.min.js',
240250
'build/closure.js'
241251
]);
242252

Diff for: karma-base.conf.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ module.exports = function (config) {
1313
'node_modules/systemjs/dist/system-polyfills.js',
1414
'node_modules/systemjs/dist/system.src.js',
1515
'node_modules/whatwg-fetch/fetch.js',
16+
{pattern: 'node_modules/rxjs/bundles/Rx.js', watched: true, served: true, included: false},
1617
{pattern: 'test/assets/**/*.*', watched: true, served: true, included: false},
1718
{pattern: 'build/**/*.js.map', watched: true, served: true, included: false},
1819
{pattern: 'build/**/*.js', watched: true, served: true, included: false}
@@ -24,7 +25,7 @@ module.exports = function (config) {
2425
require('karma-sourcemap-loader')
2526
],
2627

27-
preprocessors: {
28+
preprocessors: {
2829
'**/*.js': ['sourcemap']
2930
},
3031

Diff for: karma-dist.conf.js

+2
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,7 @@ module.exports = function (config) {
1818
config.files.push('dist/sync-test.js');
1919
config.files.push('dist/task-tracking.js');
2020
config.files.push('dist/wtf.js');
21+
config.files.push('node_modules/rxjs/bundles/Rx.js');
22+
config.files.push('dist/zone-patch-rxjs.js');
2123
config.files.push('build/test/main.js');
2224
};

Diff for: lib/rxjs/rxjs.ts

+234
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,234 @@
1+
/**
2+
* @license
3+
* Copyright Google Inc. All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import * as Rx from 'rxjs/Rx';
10+
11+
(Zone as any).__load_patch('rxjs', (global: any, Zone: ZoneType, api: any) => {
12+
const symbol: (symbolString: string) => string = (Zone as any).__symbol__;
13+
const subscribeSource = 'rxjs.subscribe';
14+
const nextSource = 'rxjs.Subscriber.next';
15+
const errorSource = 'rxjs.Subscriber.error';
16+
const completeSource = 'rxjs.Subscriber.complete';
17+
const unsubscribeSource = 'rxjs.Subscriber.unsubscribe';
18+
const teardownSource = 'rxjs.Subscriber.teardownLogic';
19+
20+
const patchObservableInstance = function(observable: any) {
21+
observable._zone = Zone.current;
22+
// patch inner function this._subscribe to check
23+
// SubscriptionZone is same with ConstuctorZone or not
24+
if (observable._subscribe && typeof observable._subscribe === 'function' &&
25+
!observable._originalSubscribe) {
26+
observable._originalSubscribe = observable._subscribe;
27+
observable._subscribe = _patchedSubscribe;
28+
}
29+
};
30+
31+
const _patchedSubscribe = function() {
32+
const currentZone = Zone.current;
33+
const _zone = this._zone;
34+
35+
const args = Array.prototype.slice.call(arguments);
36+
const subscriber = args.length > 0 ? args[0] : undefined;
37+
// also keep currentZone in Subscriber
38+
// for later Subscriber.next/error/complete method
39+
if (subscriber && !subscriber._zone) {
40+
subscriber._zone = currentZone;
41+
}
42+
// _subscribe should run in ConstructorZone
43+
// but for performance concern, we should check
44+
// whether ConsturctorZone === Zone.current here
45+
const tearDownLogic = _zone !== Zone.current ?
46+
_zone.run(this._originalSubscribe, this, args, subscribeSource) :
47+
this._originalSubscribe.apply(this, args);
48+
if (tearDownLogic && typeof tearDownLogic === 'function') {
49+
const patchedTearDownLogic = function() {
50+
// tearDownLogic should also run in ConstructorZone
51+
// but for performance concern, we should check
52+
// whether ConsturctorZone === Zone.current here
53+
if (_zone && _zone !== Zone.current) {
54+
return _zone.run(tearDownLogic, this, arguments, teardownSource);
55+
} else {
56+
return tearDownLogic.apply(this, arguments);
57+
}
58+
};
59+
return patchedTearDownLogic;
60+
}
61+
return tearDownLogic;
62+
};
63+
64+
const patchObservable = function(Rx: any, observableType: string) {
65+
const symbolObservable = symbol(observableType);
66+
67+
const Observable = Rx[observableType];
68+
if (!Observable || Observable[symbolObservable]) {
69+
// the subclass of Observable not loaded or have been patched
70+
return;
71+
}
72+
73+
// monkey-patch Observable to save the
74+
// current zone as ConstructorZone
75+
const patchedObservable: any = Rx[observableType] = function() {
76+
Observable.apply(this, arguments);
77+
patchObservableInstance(this);
78+
return this;
79+
};
80+
81+
patchedObservable.prototype = Observable.prototype;
82+
patchedObservable[symbolObservable] = Observable;
83+
84+
Object.keys(Observable).forEach(key => {
85+
patchedObservable[key] = Observable[key];
86+
});
87+
88+
const ObservablePrototype: any = Observable.prototype;
89+
const symbolSubscribe = symbol('subscribe');
90+
91+
if (!ObservablePrototype[symbolSubscribe]) {
92+
const subscribe = ObservablePrototype[symbolSubscribe] = ObservablePrototype.subscribe;
93+
// patch Observable.prototype.subscribe
94+
// if SubscripitionZone is different with ConstructorZone
95+
// we should run _subscribe in ConstructorZone and
96+
// create sinke in SubscriptionZone,
97+
// and tearDown should also run into ConstructorZone
98+
Observable.prototype.subscribe = function() {
99+
const _zone = this._zone;
100+
const currentZone = Zone.current;
101+
102+
// if operator is involved, we should also
103+
// patch the call method to save the Subscription zone
104+
if (this.operator && _zone && _zone !== currentZone) {
105+
const call = this.operator.call;
106+
this.operator.call = function() {
107+
const args = Array.prototype.slice.call(arguments);
108+
const subscriber = args.length > 0 ? args[0] : undefined;
109+
if (!subscriber._zone) {
110+
subscriber._zone = currentZone;
111+
}
112+
return _zone.run(call, this, args, subscribeSource);
113+
};
114+
}
115+
const result = subscribe.apply(this, arguments);
116+
// the result is the subscriber sink,
117+
// we save the current Zone here
118+
if (!result._zone) {
119+
result._zone = currentZone;
120+
}
121+
return result;
122+
};
123+
}
124+
125+
const symbolLift = symbol('lift');
126+
if (!ObservablePrototype[symbolLift]) {
127+
const lift = ObservablePrototype[symbolLift] = ObservablePrototype.lift;
128+
129+
// patch lift method to save ConstructorZone of Observable
130+
Observable.prototype.lift = function() {
131+
const observable = lift.apply(this, arguments);
132+
patchObservableInstance(observable);
133+
134+
return observable;
135+
};
136+
}
137+
138+
const symbolCreate = symbol('create');
139+
if (!patchedObservable[symbolCreate]) {
140+
const create = patchedObservable[symbolCreate] = Observable.create;
141+
// patch create method to save ConstructorZone of Observable
142+
Rx.Observable.create = function() {
143+
const observable = create.apply(this, arguments);
144+
patchObservableInstance(observable);
145+
146+
return observable;
147+
};
148+
}
149+
};
150+
151+
const patchSubscriber = function() {
152+
const Subscriber = Rx.Subscriber;
153+
154+
const next = Subscriber.prototype.next;
155+
const error = Subscriber.prototype.error;
156+
const complete = Subscriber.prototype.complete;
157+
const unsubscribe = Subscriber.prototype.unsubscribe;
158+
159+
// patch Subscriber.next to make sure it run
160+
// into SubscriptionZone
161+
Subscriber.prototype.next = function() {
162+
const currentZone = Zone.current;
163+
const subscriptionZone = this._zone;
164+
165+
// for performance concern, check Zone.current
166+
// equal with this._zone(SubscriptionZone) or not
167+
if (subscriptionZone && subscriptionZone !== currentZone) {
168+
return subscriptionZone.run(next, this, arguments, nextSource);
169+
} else {
170+
return next.apply(this, arguments);
171+
}
172+
};
173+
174+
Subscriber.prototype.error = function() {
175+
const currentZone = Zone.current;
176+
const subscriptionZone = this._zone;
177+
178+
// for performance concern, check Zone.current
179+
// equal with this._zone(SubscriptionZone) or not
180+
if (subscriptionZone && subscriptionZone !== currentZone) {
181+
return subscriptionZone.run(error, this, arguments, errorSource);
182+
} else {
183+
return error.apply(this, arguments);
184+
}
185+
};
186+
187+
Subscriber.prototype.complete = function() {
188+
const currentZone = Zone.current;
189+
const subscriptionZone = this._zone;
190+
191+
// for performance concern, check Zone.current
192+
// equal with this._zone(SubscriptionZone) or not
193+
if (subscriptionZone && subscriptionZone !== currentZone) {
194+
return subscriptionZone.run(complete, this, arguments, completeSource);
195+
} else {
196+
return complete.apply(this, arguments);
197+
}
198+
};
199+
200+
Subscriber.prototype.unsubscribe = function() {
201+
const currentZone = Zone.current;
202+
const subscriptionZone = this._zone;
203+
204+
// for performance concern, check Zone.current
205+
// equal with this._zone(SubscriptionZone) or not
206+
if (subscriptionZone && subscriptionZone !== currentZone) {
207+
return subscriptionZone.run(unsubscribe, this, arguments, unsubscribeSource);
208+
} else {
209+
return unsubscribe.apply(this, arguments);
210+
}
211+
};
212+
};
213+
214+
const patchObservableFactoryCreator = function(obj: any, factoryName: string) {
215+
const symbolFactory: string = symbol(factoryName);
216+
if (obj[symbolFactory]) {
217+
return;
218+
}
219+
const factoryCreator: any = obj[symbolFactory] = obj[factoryName];
220+
obj[factoryName] = function() {
221+
const factory: any = factoryCreator.apply(this, arguments);
222+
return function() {
223+
const observable = factory.apply(this, arguments);
224+
patchObservableInstance(observable);
225+
return observable;
226+
};
227+
};
228+
};
229+
230+
patchObservable(Rx, 'Observable');
231+
patchSubscriber();
232+
patchObservableFactoryCreator(Rx.Observable, 'bindCallback');
233+
patchObservableFactoryCreator(Rx.Observable, 'bindNodeCallback');
234+
});

Diff for: package.json

+3-2
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@
3232
"webdriver-sauce-test": "node test/webdriver/test.sauce.js",
3333
"ws-client": "node ./test/ws-client.js",
3434
"ws-server": "node ./test/ws-server.js",
35-
"tsc": "tsc",
36-
"tsc:w": "tsc -w",
35+
"tsc": "tsc -p .",
36+
"tsc:w": "tsc -w -p .",
3737
"test": "npm run tsc && concurrently \"npm run tsc:w\" \"npm run ws-server\" \"npm run karma-jasmine\"",
3838
"test:phantomjs": "npm run tsc && concurrently \"npm run tsc:w\" \"npm run ws-server\" \"npm run karma-jasmine:phantomjs\"",
3939
"test:phantomjs-single": "concurrently \"npm run ws-server\" \"npm run karma-jasmine-phantomjs:autoclose\"",
@@ -87,6 +87,7 @@
8787
"phantomjs": "^2.1.7",
8888
"promises-aplus-tests": "^2.1.2",
8989
"pump": "^1.0.1",
90+
"rxjs": "^5.4.2",
9091
"selenium-webdriver": "^3.4.0",
9192
"systemjs": "^0.19.37",
9293
"ts-loader": "^0.6.0",

Diff for: test/browser-zone-setup.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,5 @@ import '../lib/zone-spec/proxy';
1717
import '../lib/zone-spec/sync-test';
1818
import '../lib/zone-spec/task-tracking';
1919
import '../lib/zone-spec/wtf';
20-
import '../lib/extra/cordova';
20+
import '../lib/extra/cordova';
21+
import '../lib/rxjs/rxjs';

Diff for: test/browser_entry_point.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,5 @@ import './browser/MediaQuery.spec';
2323
import './browser/Notification.spec';
2424
import './mocha-patch.spec';
2525
import './jasmine-patch.spec';
26-
import './extra/cordova.spec';
26+
import './extra/cordova.spec';
27+
import './rxjs/rxjs.spec';

Diff for: test/common_tests.ts

+1
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,6 @@ import './zone-spec/sync-test.spec';
2121
import './zone-spec/fake-async-test.spec';
2222
import './zone-spec/proxy.spec';
2323
import './zone-spec/task-tracking.spec';
24+
import './rxjs/rxjs.spec';
2425

2526
Error.stackTraceLimit = Number.POSITIVE_INFINITY;

Diff for: test/global-rxjs.ts

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
/**
2+
* @license
3+
* Copyright Google Inc. All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
const globalRx: any = (window as any).Rx;
9+
exports.Observable = globalRx.Observable;
10+
exports.Subject = globalRx.Subject;
11+
exports.Scheduler = globalRx.Scheduler;

Diff for: test/main.ts

+8-1
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,18 @@ declare const __karma__: {
1515
__karma__.loaded = function() {};
1616
(window as any).global = window;
1717

18-
System.config({defaultJSExtensions: true});
1918
let browserPatchedPromise: any = null;
2019
if ((window as any)[(Zone as any).__symbol__('setTimeout')]) {
20+
System.config({
21+
defaultJSExtensions: true,
22+
map: {'rxjs/Rx': 'base/build/test/global-rxjs.js'},
23+
});
2124
browserPatchedPromise = Promise.resolve('browserPatched');
2225
} else {
26+
System.config({
27+
defaultJSExtensions: true,
28+
map: {'rxjs/Rx': 'base/node_modules/rxjs/bundles/Rx.js'},
29+
});
2330
// this means that Zone has not patched the browser yet, which means we must be running in
2431
// build mode and need to load the browser patch.
2532
browserPatchedPromise = System.import('/base/build/test/browser-zone-setup');

Diff for: test/node_entry_point.ts

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import '../lib/zone-spec/proxy';
2222
import '../lib/zone-spec/sync-test';
2323
import '../lib/zone-spec/task-tracking';
2424
import '../lib/zone-spec/wtf';
25+
import '../lib/rxjs/rxjs';
2526

2627
// Setup test environment
2728
import './test-env-setup-jasmine';

0 commit comments

Comments
 (0)