Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent undefined being fired when a listener is disposed #173823

Merged
merged 1 commit into from Feb 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/vs/base/common/event.ts
Expand Up @@ -254,7 +254,7 @@ export namespace Event {
});
},
onWillRemoveListener() {
if (flushOnListenerRemove) {
if (flushOnListenerRemove && numDebouncedCalls > 0) {
doFire?.();
}
},
Expand Down
21 changes: 20 additions & 1 deletion src/vs/base/test/common/event.test.ts
Expand Up @@ -1048,6 +1048,26 @@ suite('Event utils', () => {
});

suite('accumulate', () => {
test('should not fire after a listener is disposed with undefined or []', async () => {
const eventEmitter = new Emitter<number>();
const event = eventEmitter.event;
const accumulated = Event.accumulate(event, 0);

const calls1: number[][] = [];
const calls2: number[][] = [];
const listener1 = accumulated((e) => calls1.push(e));
accumulated((e) => calls2.push(e));

eventEmitter.fire(1);
await timeout(1);
assert.deepStrictEqual(calls1, [[1]]);
assert.deepStrictEqual(calls2, [[1]]);

listener1.dispose();
await timeout(1);
assert.deepStrictEqual(calls1, [[1]]);
assert.deepStrictEqual(calls2, [[1]], 'should not fire after a listener is disposed with undefined or []');
});
test('should accumulate a single event', async () => {
const eventEmitter = new Emitter<number>();
const event = eventEmitter.event;
Expand Down Expand Up @@ -1234,7 +1254,6 @@ suite('Event utils', () => {
assert.deepStrictEqual(calls, [1], 'should fire with the first event, not the second (after listener dispose)');
});


test('should flush events when the emitter is disposed', async () => {
const emitter = new Emitter<number>();
const debounced = Event.debounce(emitter.event, (l, e) => l ? l + 1 : 1, 0);
Expand Down