Skip to content

Commit b06b24b

Browse files
JiaLiPassionthePunderWoman
authored andcommitted
fix(zone.js): handle fetch with AbortSignal (#49595)
fetch support AbortSignal, zone.js schedules a macroTask when fetch() ``` fetch(..., {signal: abortSignal}); ``` we should also be able to cancel fetch with `zoneTask.cancel` call. So this commit create an internal AbortSignal to handle `zoneTask.cancel()` call and also delegate the `options.signal` from the user code. PR Close #49595
1 parent d4973ff commit b06b24b

3 files changed

Lines changed: 106 additions & 50 deletions

File tree

packages/zone.js/lib/common/events.ts

Lines changed: 10 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -293,16 +293,6 @@ export function patchEventTarget(
293293
// if task is not marked as isRemoved, this call is directly
294294
// from Zone.prototype.cancelTask, we should remove the task
295295
// from tasksList of target first
296-
const signal = task?.options?.signal;
297-
if (typeof signal === 'object' && signal?.tasks) {
298-
const abortTasks = signal.tasks;
299-
for (let i = 0; i < abortTasks?.length || 0; i++) {
300-
if (abortTasks[i] === task) {
301-
abortTasks.splice(i, 1);
302-
break;
303-
}
304-
}
305-
}
306296
if (!task.isRemoved) {
307297
const symbolEventNames = zoneSymbolEventNames[task.eventName];
308298
let symbolEventName;
@@ -404,8 +394,11 @@ export function patchEventTarget(
404394
const passive =
405395
passiveSupported && !!passiveEvents && passiveEvents.indexOf(eventName) !== -1;
406396
const options = buildEventListenerOptions(arguments[2], passive);
407-
const signal = typeof options === 'object' && options?.signal;
408-
if (typeof signal === 'object' && signal?.aborted) {
397+
const signal = options && typeof options === 'object' && options.signal &&
398+
typeof options.signal === 'object' ?
399+
options.signal :
400+
undefined;
401+
if (signal?.aborted) {
409402
// the signal is an aborted one, just return without attaching the event listener.
410403
return;
411404
}
@@ -480,7 +473,7 @@ export function patchEventTarget(
480473
(data as any).taskData = taskData;
481474
}
482475

483-
if (signal && typeof signal === 'object') {
476+
if (signal) {
484477
// if addEventListener with signal options, we don't pass it to
485478
// native addEventListener, instead we keep the signal setting
486479
// and handle ourselves.
@@ -489,20 +482,12 @@ export function patchEventTarget(
489482
const task: any =
490483
zone.scheduleEventTask(source, delegate, data, customScheduleFn, customCancelFn);
491484

492-
if (signal && typeof signal === 'object') {
485+
if (signal) {
493486
// after task is scheduled, we need to store the signal back to task.options
494487
taskData.options.signal = signal;
495-
const tasks = signal.tasks || [];
496-
tasks.push(task);
497-
signal.tasks = tasks;
498-
if (!signal[Zone.__symbol__('abortListener')]) {
499-
signal[Zone.__symbol__('abortListener')] = true;
500-
nativeListener.call(signal, 'abort', function() {
501-
const sTasks = signal.tasks.slice();
502-
sTasks.forEach((task: Task) => task.zone.cancelTask(task));
503-
signal.tasks.length = 0;
504-
});
505-
}
488+
nativeListener.call(signal, 'abort', () => {
489+
task.zone.cancelTask(task);
490+
}, {once: true});
506491
}
507492

508493
// should clear taskData.target to avoid memory leak

packages/zone.js/lib/common/fetch.ts

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,22 @@ Zone.__load_patch('fetch', (global: any, Zone: ZoneType, api: _ZonePrivate) => {
2626
const ZoneAwarePromise = global.Promise;
2727
const symbolThenPatched = api.symbol('thenPatched');
2828
const fetchTaskScheduling = api.symbol('fetchTaskScheduling');
29-
const fetchTaskAborting = api.symbol('fetchTaskAborting');
30-
const OriginalAbortController = global['AbortController'];
31-
const supportAbort = typeof OriginalAbortController === 'function';
32-
let abortNative: Function|null = OriginalAbortController?.prototype[api.symbol('abort')];
3329
const placeholder = function() {};
3430
global['fetch'] = function() {
3531
const args = Array.prototype.slice.call(arguments);
36-
const options = args.length > 1 ? args[1] : null;
32+
const options = args.length > 1 ? args[1] : {};
3733
const signal = options && options.signal;
34+
const ac = new AbortController();
35+
const fetchSignal = ac.signal;
36+
options.signal = fetchSignal;
37+
args[1] = options;
38+
if (signal) {
39+
const nativeAddEventListener =
40+
signal[Zone.__symbol__('addEventListener')] || signal.addEventListener;
41+
nativeAddEventListener.call(signal, 'abort', function() {
42+
ac!.abort();
43+
}, {once: true});
44+
}
3845
return new Promise((res, rej) => {
3946
const task = Zone.current.scheduleMacroTask(
4047
'fetch', placeholder, {fetchArgs: args} as FetchTaskData,
@@ -72,25 +79,8 @@ Zone.__load_patch('fetch', (global: any, Zone: ZoneType, api: _ZonePrivate) => {
7279
});
7380
},
7481
() => {
75-
if (!supportAbort) {
76-
rej('No AbortController supported, can not cancel fetch');
77-
return;
78-
}
79-
if (signal && signal.abortController && !signal.aborted &&
80-
typeof signal.abortController.abort === 'function' && abortNative) {
81-
try {
82-
(Zone.current as any)[fetchTaskAborting] = true;
83-
abortNative.call(signal.abortController);
84-
} finally {
85-
(Zone.current as any)[fetchTaskAborting] = false;
86-
}
87-
} else {
88-
rej('cancel fetch need a AbortController.signal');
89-
}
82+
ac.abort();
9083
});
91-
if (signal && signal.abortController) {
92-
signal.abortController.tasks = [task];
93-
}
9484
});
9585
};
9686
});

packages/zone.js/test/browser/browser.spec.ts

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1809,7 +1809,7 @@ describe('Zone', function() {
18091809
expect(logs.length).toBe(2);
18101810
expect(logs).toEqual(['click1', 'click2']);
18111811

1812-
button.removeEventListener('click', listener1);
1812+
ac.abort();
18131813
listeners = button.eventListeners!('click');
18141814
expect(listeners.length).toBe(1);
18151815

@@ -1820,11 +1820,92 @@ describe('Zone', function() {
18201820
expect(logs.length).toBe(1);
18211821
expect(listeners.length).toBe(1);
18221822
expect(logs).toEqual(['click2']);
1823+
});
18231824

1824-
ac.abort();
1825+
it('should not break remove event listeners with AbortController', function() {
1826+
let logs: string[] = [];
1827+
const ac = new AbortController();
1828+
1829+
const listener1 = function() {
1830+
logs.push('click1');
1831+
};
1832+
button.addEventListener('click', listener1, {signal: ac.signal});
1833+
button.addEventListener('click', function() {
1834+
logs.push('click2');
1835+
});
1836+
let listeners = button.eventListeners!('click');
1837+
expect(listeners.length).toBe(2);
1838+
1839+
button.dispatchEvent(clickEvent);
1840+
expect(logs.length).toBe(2);
1841+
expect(logs).toEqual(['click1', 'click2']);
1842+
1843+
button.removeEventListener('click', listener1);
1844+
listeners = button.eventListeners!('click');
1845+
expect(listeners.length).toBe(1);
1846+
1847+
logs = [];
1848+
1849+
listeners = button.eventListeners!('click');
1850+
button.dispatchEvent(clickEvent);
1851+
expect(logs.length).toBe(1);
1852+
expect(listeners.length).toBe(1);
18251853
expect(logs).toEqual(['click2']);
18261854
});
18271855

1856+
it('should support remove multiple event listeners with the same AbortController',
1857+
function() {
1858+
let logs: string[] = [];
1859+
const ac = new AbortController();
1860+
const button1 = document.createElement('button');
1861+
const keyEvent: any = document.createEvent('KeyboardEvent');
1862+
keyEvent.initKeyboardEvent(
1863+
'keypress', // typeArg,
1864+
true, // canBubbleArg,
1865+
true, // cancelableArg,
1866+
null, // viewArg, Specifies UIEvent.view. This value may be null.
1867+
'',
1868+
0,
1869+
false, // ctrlKeyArg,
1870+
false, // altKeyArg,
1871+
false, // shiftKeyArg,
1872+
false, // metaKeyArg,
1873+
);
1874+
document.body.appendChild(button1);
1875+
1876+
const listener1 = function() {
1877+
logs.push('click1');
1878+
};
1879+
button.addEventListener('click', listener1, {signal: ac.signal});
1880+
button.addEventListener('click', function() {
1881+
logs.push('click2');
1882+
});
1883+
button1.addEventListener('keypress', () => logs.push('click3'), {signal: ac.signal});
1884+
button1.addEventListener('keypress', function() {
1885+
logs.push('click4');
1886+
});
1887+
let listeners = button.eventListeners!('click');
1888+
expect(listeners.length).toBe(2);
1889+
1890+
button.dispatchEvent(clickEvent);
1891+
expect(logs.length).toBe(2);
1892+
expect(logs).toEqual(['click1', 'click2']);
1893+
button1.dispatchEvent(keyEvent);
1894+
expect(logs.length).toBe(4);
1895+
expect(logs).toEqual(['click1', 'click2', 'click3', 'click4']);
1896+
1897+
logs = [];
1898+
ac.abort();
1899+
listeners = button.eventListeners!('click');
1900+
button.dispatchEvent(clickEvent);
1901+
expect(logs.length).toBe(1);
1902+
expect(listeners.length).toBe(1);
1903+
expect(logs).toEqual(['click2']);
1904+
button1.dispatchEvent(keyEvent);
1905+
expect(logs.length).toBe(2);
1906+
expect(logs).toEqual(['click2', 'click4']);
1907+
});
1908+
18281909
it('should not add event listeners with aborted signal', function() {
18291910
let logs: string[] = [];
18301911

0 commit comments

Comments
 (0)