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

Commit ff88bb4

Browse files
committed
fix(tasks): do not drain the microtask queue early.
Fixes a bug where a event task invoked inside another task would drain the microtask queue too early. This would mean that microtasks would be called unexpectedly in the middle of what should have been a block of synchronous code.
1 parent d4a1436 commit ff88bb4

File tree

3 files changed

+29
-11
lines changed

3 files changed

+29
-11
lines changed

lib/jasmine/jasmine.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,16 @@
102102
execute() {
103103
if(Zone.current !== ambientZone) throw new Error("Unexpected Zone: " + Zone.current.name);
104104
testProxyZone = ambientZone.fork(new ProxyZoneSpec());
105-
super.execute();
105+
if (!Zone.currentTask) {
106+
// if we are not running in a task then if someone would register a
107+
// element.addEventListener and then calling element.click() the
108+
// addEventListener callback would think that it is the top most task and would
109+
// drain the microtask queue on element.click() which would be incorrect.
110+
// For this reason we always force a task when running jasmine tests.
111+
Zone.current.scheduleMicroTask('jasmine.execute().forceTask', () => super.execute());
112+
} else {
113+
super.execute();
114+
}
106115
}
107116
};
108117
})();

test/browser/element.spec.ts

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,16 +51,15 @@ describe('element', function () {
5151
});
5252

5353
it('should not call microtasks early when an event is invoked', function(done) {
54-
// we have to run this test in setTimeout to guarantee that we are running in an existing task
55-
setTimeout(() => {
56-
var log = '';
54+
var log = '';
55+
button.addEventListener('click', () => {
5756
Zone.current.scheduleMicroTask('test', () => log += 'microtask;');
58-
button.addEventListener('click', () => log += 'click;');
59-
button.click();
60-
61-
expect(log).toEqual('click;');
62-
done();
57+
log += 'click;'
6358
});
59+
button.click();
60+
61+
expect(log).toEqual('click;');
62+
done();
6463
});
6564

6665
it('should call microtasks early when an event is invoked', function(done) {
@@ -80,11 +79,16 @@ describe('element', function () {
8079
* eager drainage.
8180
* 3. Pay the cost of throwing an exception in event tasks and verifying that we are the
8281
* top most frame.
82+
*
83+
* For now we are choosing to ignore it and assume that this arrises in tests only.
84+
* As an added measure we make sure that all jasmine tests always run in a task. See: jasmine.ts
8385
*/
8486
global[Zone['__symbol__']('setTimeout')](() => {
8587
var log = '';
86-
Zone.current.scheduleMicroTask('test', () => log += 'microtask;');
87-
button.addEventListener('click', () => log += 'click;');
88+
button.addEventListener('click', () => {
89+
Zone.current.scheduleMicroTask('test', () => log += 'microtask;');
90+
log += 'click;'
91+
});
8892
button.click();
8993

9094
expect(log).toEqual('click;microtask;');

test/jasmine-patch.spec.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
beforeEach(() => {
2+
// assert that each jasmine run has a task, so that drainMicrotask works properly.
3+
expect(Zone.currentTask).toBeTruthy();
4+
});
5+
16
describe('jasmine', () => {
27
let throwOnAsync = false;
38
let beforeEachZone: Zone = null;

0 commit comments

Comments
 (0)