Skip to content

Commit

Permalink
fix(zone.js): support addEventListener with signal option. (#49595)
Browse files Browse the repository at this point in the history
Close #49591

```
const ac = new AbortController();
addEventListener(eventName, handler, {signal: ac.signal);`
ac.abort();
```

Currently `zone.js` doesn't support the `signal` option, this PR allows
the user to use AbortContoller to remove the event listener.

PR Close #49595
  • Loading branch information
JiaLiPassion authored and thePunderWoman committed Dec 18, 2023
1 parent 1b65931 commit d4973ff
Show file tree
Hide file tree
Showing 3 changed files with 146 additions and 18 deletions.
37 changes: 37 additions & 0 deletions packages/zone.js/lib/common/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,16 @@ export function patchEventTarget(
// if task is not marked as isRemoved, this call is directly
// from Zone.prototype.cancelTask, we should remove the task
// from tasksList of target first
const signal = task?.options?.signal;
if (typeof signal === 'object' && signal?.tasks) {
const abortTasks = signal.tasks;
for (let i = 0; i < abortTasks?.length || 0; i++) {
if (abortTasks[i] === task) {
abortTasks.splice(i, 1);
break;
}
}
}
if (!task.isRemoved) {
const symbolEventNames = zoneSymbolEventNames[task.eventName];
let symbolEventName;
Expand Down Expand Up @@ -394,6 +404,11 @@ export function patchEventTarget(
const passive =
passiveSupported && !!passiveEvents && passiveEvents.indexOf(eventName) !== -1;
const options = buildEventListenerOptions(arguments[2], passive);
const signal = typeof options === 'object' && options?.signal;
if (typeof signal === 'object' && signal?.aborted) {
// the signal is an aborted one, just return without attaching the event listener.
return;
}

if (unpatchedEvents) {
// check unpatched list
Expand Down Expand Up @@ -465,9 +480,31 @@ export function patchEventTarget(
(data as any).taskData = taskData;
}

if (signal && typeof signal === 'object') {
// if addEventListener with signal options, we don't pass it to
// native addEventListener, instead we keep the signal setting
// and handle ourselves.
taskData.options.signal = undefined;
}
const task: any =
zone.scheduleEventTask(source, delegate, data, customScheduleFn, customCancelFn);

if (signal && typeof signal === 'object') {
// after task is scheduled, we need to store the signal back to task.options
taskData.options.signal = signal;
const tasks = signal.tasks || [];
tasks.push(task);
signal.tasks = tasks;
if (!signal[Zone.__symbol__('abortListener')]) {
signal[Zone.__symbol__('abortListener')] = true;
nativeListener.call(signal, 'abort', function() {
const sTasks = signal.tasks.slice();
sTasks.forEach((task: Task) => task.zone.cancelTask(task));
signal.tasks.length = 0;
});
}
}

// should clear taskData.target to avoid memory leak
// issue, https://github.com/angular/angular/issues/20442
taskData.target = null;
Expand Down
20 changes: 2 additions & 18 deletions packages/zone.js/lib/common/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,23 +29,7 @@ Zone.__load_patch('fetch', (global: any, Zone: ZoneType, api: _ZonePrivate) => {
const fetchTaskAborting = api.symbol('fetchTaskAborting');
const OriginalAbortController = global['AbortController'];
const supportAbort = typeof OriginalAbortController === 'function';
let abortNative: Function|null = null;
if (supportAbort) {
global['AbortController'] = function() {
const abortController = new OriginalAbortController();
const signal = abortController.signal;
signal.abortController = abortController;
return abortController;
};
abortNative = api.patchMethod(
OriginalAbortController.prototype, 'abort',
(delegate: Function) => (self: any, args: any) => {
if (self.task) {
return self.task.zone.cancelTask(self.task);
}
return delegate.apply(self, args);
});
}
let abortNative: Function|null = OriginalAbortController?.prototype[api.symbol('abort')];
const placeholder = function() {};
global['fetch'] = function() {
const args = Array.prototype.slice.call(arguments);
Expand Down Expand Up @@ -105,7 +89,7 @@ Zone.__load_patch('fetch', (global: any, Zone: ZoneType, api: _ZonePrivate) => {
}
});
if (signal && signal.abortController) {
signal.abortController.task = task;
signal.abortController.tasks = [task];
}
});
};
Expand Down
107 changes: 107 additions & 0 deletions packages/zone.js/test/browser/browser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1762,6 +1762,113 @@ describe('Zone', function() {
expect(logs).toEqual(['click2']);
});

it('should support remove event listeners via AbortController', function() {
let logs: string[] = [];
const ac = new AbortController();

button.addEventListener('click', function() {
logs.push('click1');
}, {signal: ac.signal});
button.addEventListener('click', function() {
logs.push('click2');
});
button.addEventListener('click', function() {
logs.push('click3');
}, {signal: ac.signal});
let listeners = button.eventListeners!('click');
expect(listeners.length).toBe(3);

button.dispatchEvent(clickEvent);
expect(logs.length).toBe(3);
expect(logs).toEqual(['click1', 'click2', 'click3']);
ac.abort();
logs = [];

listeners = button.eventListeners!('click');
button.dispatchEvent(clickEvent);
expect(logs.length).toBe(1);
expect(listeners.length).toBe(1);
expect(logs).toEqual(['click2']);
});

it('should support remove event listeners with AbortController', function() {
let logs: string[] = [];
const ac = new AbortController();

const listener1 = function() {
logs.push('click1');
};
button.addEventListener('click', listener1, {signal: ac.signal});
button.addEventListener('click', function() {
logs.push('click2');
});
let listeners = button.eventListeners!('click');
expect(listeners.length).toBe(2);

button.dispatchEvent(clickEvent);
expect(logs.length).toBe(2);
expect(logs).toEqual(['click1', 'click2']);

button.removeEventListener('click', listener1);
listeners = button.eventListeners!('click');
expect(listeners.length).toBe(1);

logs = [];

listeners = button.eventListeners!('click');
button.dispatchEvent(clickEvent);
expect(logs.length).toBe(1);
expect(listeners.length).toBe(1);
expect(logs).toEqual(['click2']);

ac.abort();
expect(logs).toEqual(['click2']);
});

it('should not add event listeners with aborted signal', function() {
let logs: string[] = [];

button.addEventListener('click', function() {
logs.push('click1');
}, {signal: AbortSignal.abort()});
button.addEventListener('click', function() {
logs.push('click2');
});
let listeners = button.eventListeners!('click');
expect(listeners.length).toBe(1);

button.dispatchEvent(clickEvent);
expect(logs.length).toBe(1);
expect(logs).toEqual(['click2']);
});

it('should remove event listeners with timeout signal',
ifEnvSupportsWithDone(
() => typeof AbortSignal.timeout === 'function', function(done: DoneFn) {
let logs: string[] = [];

button.addEventListener('click', function() {
logs.push('click1');
}, {signal: AbortSignal.timeout(1)});
button.addEventListener('click', function() {
logs.push('click2');
});
let listeners = button.eventListeners!('click');
expect(listeners.length).toBe(2);

button.dispatchEvent(clickEvent);
expect(logs.length).toBe(2);
expect(logs).toEqual(['click1', 'click2']);

setTimeout(() => {
logs = [];
button.dispatchEvent(clickEvent);
expect(logs.length).toBe(1);
expect(logs).toEqual(['click2']);
done();
}, 10);
}));

it('should support reschedule eventTask',
ifEnvSupports(supportEventListenerOptions, function() {
let hookSpy1 = jasmine.createSpy('spy1');
Expand Down

0 comments on commit d4973ff

Please sign in to comment.