Skip to content

Commit 28bf0b1

Browse files
solomatovalexr00
solomatov
authored and
alexr00
committed
Use leading flag set to true in debouncing events from extension trees (microsoft#88051)
* Use leading flag set to true in debouncing events from extension trees * - remove the flag for debounce in extHostTreeView - refactor test extHostTreeView's test to test prod behavior - fix a small bug in debounce * Minor cleanup
1 parent bfee2e5 commit 28bf0b1

4 files changed

Lines changed: 157 additions & 110 deletions

File tree

‎src/vs/base/common/event.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ export namespace Event {
148148

149149
if (leading && !handle) {
150150
emitter.fire(output);
151+
output = undefined;
151152
}
152153

153154
clearTimeout(handle);

‎src/vs/base/test/common/event.test.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,20 @@ suite('Event', function () {
237237
assert.equal(calls, 2);
238238
});
239239

240+
test('Debounce Event - leading reset', async function () {
241+
const emitter = new Emitter<number>();
242+
let debounced = Event.debounce(emitter.event, (l, e) => l ? l + 1 : 1, 0, /*leading=*/true);
243+
244+
let calls: number[] = [];
245+
debounced((e) => calls.push(e));
246+
247+
emitter.fire(1);
248+
emitter.fire(1);
249+
250+
await timeout(1);
251+
assert.deepEqual(calls, [1, 1]);
252+
});
253+
240254
test('Emitter - In Order Delivery', function () {
241255
const a = new Emitter<string>();
242256
const listener2Events: string[] = [];

‎src/vs/workbench/api/common/extHostTreeViews.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ class ExtHostTreeView<T> extends Disposable {
237237
result.message = true;
238238
}
239239
return result;
240-
}, 200)(({ message, elements }) => {
240+
}, 200, true)(({ message, elements }) => {
241241
if (elements.length) {
242242
this.refreshQueue = this.refreshQueue.then(() => {
243243
const _promiseCallback = promiseCallback;

‎src/vs/workbench/test/electron-browser/api/extHostTreeViews.test.ts‎

Lines changed: 141 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { mock } from 'vs/workbench/test/electron-browser/api/mock';
1818
import { TreeItemCollapsibleState, ITreeItem } from 'vs/workbench/common/views';
1919
import { NullLogService } from 'vs/platform/log/common/log';
2020
import { IExtensionDescription } from 'vs/platform/extensions/common/extensions';
21+
import type { IDisposable } from 'vs/base/common/lifecycle';
2122

2223
suite('ExtHostTreeView', function () {
2324

@@ -29,7 +30,9 @@ suite('ExtHostTreeView', function () {
2930
}
3031

3132
$refresh(viewId: string, itemsToRefresh: { [treeItemHandle: string]: ITreeItem }): Promise<void> {
32-
return Promise.resolve(null).then(() => this.onRefresh.fire(itemsToRefresh));
33+
return Promise.resolve(null).then(() => {
34+
this.onRefresh.fire(itemsToRefresh);
35+
});
3336
}
3437

3538
$reveal(): Promise<void> {
@@ -243,46 +246,62 @@ suite('ExtHostTreeView', function () {
243246
onDidChangeTreeNode.fire(getNode('bb'));
244247
});
245248

246-
test('refresh parent and child node trigger refresh only on parent - scenario 1', function (done) {
247-
target.onRefresh.event(actuals => {
248-
assert.deepEqual(['0/0:b', '0/0:a/0:aa'], Object.keys(actuals));
249-
assert.deepEqual(removeUnsetKeys(actuals['0/0:b']), {
250-
handle: '0/0:b',
251-
label: { label: 'b' },
252-
collapsibleState: TreeItemCollapsibleState.Collapsed
249+
async function runWithEventMerging(action: (resolve: () => void) => void) {
250+
await new Promise((resolve) => {
251+
let subscription: IDisposable | undefined = undefined;
252+
subscription = target.onRefresh.event(() => {
253+
subscription!.dispose();
254+
resolve();
253255
});
254-
assert.deepEqual(removeUnsetKeys(actuals['0/0:a/0:aa']), {
255-
handle: '0/0:a/0:aa',
256-
parentHandle: '0/0:a',
257-
label: { label: 'aa' },
258-
collapsibleState: TreeItemCollapsibleState.None
256+
onDidChangeTreeNode.fire(getNode('b'));
257+
});
258+
await new Promise(action);
259+
}
260+
261+
test('refresh parent and child node trigger refresh only on parent - scenario 1', async () => {
262+
return runWithEventMerging((resolve) => {
263+
target.onRefresh.event(actuals => {
264+
assert.deepEqual(['0/0:b', '0/0:a/0:aa'], Object.keys(actuals));
265+
assert.deepEqual(removeUnsetKeys(actuals['0/0:b']), {
266+
handle: '0/0:b',
267+
label: { label: 'b' },
268+
collapsibleState: TreeItemCollapsibleState.Collapsed
269+
});
270+
assert.deepEqual(removeUnsetKeys(actuals['0/0:a/0:aa']), {
271+
handle: '0/0:a/0:aa',
272+
parentHandle: '0/0:a',
273+
label: { label: 'aa' },
274+
collapsibleState: TreeItemCollapsibleState.None
275+
});
276+
resolve();
259277
});
260-
done();
278+
onDidChangeTreeNode.fire(getNode('b'));
279+
onDidChangeTreeNode.fire(getNode('aa'));
280+
onDidChangeTreeNode.fire(getNode('bb'));
261281
});
262-
onDidChangeTreeNode.fire(getNode('b'));
263-
onDidChangeTreeNode.fire(getNode('aa'));
264-
onDidChangeTreeNode.fire(getNode('bb'));
265282
});
266283

267-
test('refresh parent and child node trigger refresh only on parent - scenario 2', function (done) {
268-
target.onRefresh.event(actuals => {
269-
assert.deepEqual(['0/0:a/0:aa', '0/0:b'], Object.keys(actuals));
270-
assert.deepEqual(removeUnsetKeys(actuals['0/0:b']), {
271-
handle: '0/0:b',
272-
label: { label: 'b' },
273-
collapsibleState: TreeItemCollapsibleState.Collapsed
274-
});
275-
assert.deepEqual(removeUnsetKeys(actuals['0/0:a/0:aa']), {
276-
handle: '0/0:a/0:aa',
277-
parentHandle: '0/0:a',
278-
label: { label: 'aa' },
279-
collapsibleState: TreeItemCollapsibleState.None
284+
test('refresh parent and child node trigger refresh only on parent - scenario 2', async () => {
285+
return runWithEventMerging((resolve) => {
286+
target.onRefresh.event(actuals => {
287+
assert.deepEqual(['0/0:a/0:aa', '0/0:b'], Object.keys(actuals));
288+
assert.deepEqual(removeUnsetKeys(actuals['0/0:b']), {
289+
handle: '0/0:b',
290+
label: { label: 'b' },
291+
collapsibleState: TreeItemCollapsibleState.Collapsed
292+
});
293+
assert.deepEqual(removeUnsetKeys(actuals['0/0:a/0:aa']), {
294+
handle: '0/0:a/0:aa',
295+
parentHandle: '0/0:a',
296+
label: { label: 'aa' },
297+
collapsibleState: TreeItemCollapsibleState.None
298+
});
299+
resolve();
280300
});
281-
done();
301+
onDidChangeTreeNode.fire(getNode('bb'));
302+
onDidChangeTreeNode.fire(getNode('aa'));
303+
onDidChangeTreeNode.fire(getNode('b'));
282304
});
283-
onDidChangeTreeNode.fire(getNode('bb'));
284-
onDidChangeTreeNode.fire(getNode('aa'));
285-
onDidChangeTreeNode.fire(getNode('b'));
286305
});
287306

288307
test('refresh an element for label change', function (done) {
@@ -299,63 +318,73 @@ suite('ExtHostTreeView', function () {
299318
onDidChangeTreeNode.fire(getNode('a'));
300319
});
301320

302-
test('refresh calls are throttled on roots', function (done) {
303-
target.onRefresh.event(actuals => {
304-
assert.equal(undefined, actuals);
305-
done();
321+
test('refresh calls are throttled on roots', () => {
322+
return runWithEventMerging((resolve) => {
323+
target.onRefresh.event(actuals => {
324+
assert.equal(undefined, actuals);
325+
resolve();
326+
});
327+
onDidChangeTreeNode.fire(undefined);
328+
onDidChangeTreeNode.fire(undefined);
329+
onDidChangeTreeNode.fire(undefined);
330+
onDidChangeTreeNode.fire(undefined);
306331
});
307-
onDidChangeTreeNode.fire(undefined);
308-
onDidChangeTreeNode.fire(undefined);
309-
onDidChangeTreeNode.fire(undefined);
310-
onDidChangeTreeNode.fire(undefined);
311332
});
312333

313-
test('refresh calls are throttled on elements', function (done) {
314-
target.onRefresh.event(actuals => {
315-
assert.deepEqual(['0/0:a', '0/0:b'], Object.keys(actuals));
316-
done();
317-
});
334+
test('refresh calls are throttled on elements', () => {
335+
return runWithEventMerging((resolve) => {
336+
target.onRefresh.event(actuals => {
337+
assert.deepEqual(['0/0:a', '0/0:b'], Object.keys(actuals));
338+
resolve();
339+
});
318340

319-
onDidChangeTreeNode.fire(getNode('a'));
320-
onDidChangeTreeNode.fire(getNode('b'));
321-
onDidChangeTreeNode.fire(getNode('b'));
322-
onDidChangeTreeNode.fire(getNode('a'));
341+
onDidChangeTreeNode.fire(getNode('a'));
342+
onDidChangeTreeNode.fire(getNode('b'));
343+
onDidChangeTreeNode.fire(getNode('b'));
344+
onDidChangeTreeNode.fire(getNode('a'));
345+
});
323346
});
324347

325-
test('refresh calls are throttled on unknown elements', function (done) {
326-
target.onRefresh.event(actuals => {
327-
assert.deepEqual(['0/0:a', '0/0:b'], Object.keys(actuals));
328-
done();
329-
});
348+
test('refresh calls are throttled on unknown elements', () => {
349+
return runWithEventMerging((resolve) => {
350+
target.onRefresh.event(actuals => {
351+
assert.deepEqual(['0/0:a', '0/0:b'], Object.keys(actuals));
352+
resolve();
353+
});
330354

331-
onDidChangeTreeNode.fire(getNode('a'));
332-
onDidChangeTreeNode.fire(getNode('b'));
333-
onDidChangeTreeNode.fire(getNode('g'));
334-
onDidChangeTreeNode.fire(getNode('a'));
355+
onDidChangeTreeNode.fire(getNode('a'));
356+
onDidChangeTreeNode.fire(getNode('b'));
357+
onDidChangeTreeNode.fire(getNode('g'));
358+
onDidChangeTreeNode.fire(getNode('a'));
359+
});
335360
});
336361

337-
test('refresh calls are throttled on unknown elements and root', function (done) {
338-
target.onRefresh.event(actuals => {
339-
assert.equal(undefined, actuals);
340-
done();
341-
});
362+
test('refresh calls are throttled on unknown elements and root', () => {
363+
return runWithEventMerging((resolve) => {
364+
target.onRefresh.event(actuals => {
365+
assert.equal(undefined, actuals);
366+
resolve();
367+
});
342368

343-
onDidChangeTreeNode.fire(getNode('a'));
344-
onDidChangeTreeNode.fire(getNode('b'));
345-
onDidChangeTreeNode.fire(getNode('g'));
346-
onDidChangeTreeNode.fire(undefined);
369+
onDidChangeTreeNode.fire(getNode('a'));
370+
onDidChangeTreeNode.fire(getNode('b'));
371+
onDidChangeTreeNode.fire(getNode('g'));
372+
onDidChangeTreeNode.fire(undefined);
373+
});
347374
});
348375

349-
test('refresh calls are throttled on elements and root', function (done) {
350-
target.onRefresh.event(actuals => {
351-
assert.equal(undefined, actuals);
352-
done();
353-
});
376+
test('refresh calls are throttled on elements and root', () => {
377+
return runWithEventMerging((resolve) => {
378+
target.onRefresh.event(actuals => {
379+
assert.equal(undefined, actuals);
380+
resolve();
381+
});
354382

355-
onDidChangeTreeNode.fire(getNode('a'));
356-
onDidChangeTreeNode.fire(getNode('b'));
357-
onDidChangeTreeNode.fire(undefined);
358-
onDidChangeTreeNode.fire(getNode('a'));
383+
onDidChangeTreeNode.fire(getNode('a'));
384+
onDidChangeTreeNode.fire(getNode('b'));
385+
onDidChangeTreeNode.fire(undefined);
386+
onDidChangeTreeNode.fire(getNode('a'));
387+
});
359388
});
360389

361390
test('generate unique handles from labels by escaping them', (done) => {
@@ -552,37 +581,40 @@ suite('ExtHostTreeView', function () {
552581
const treeView = testObject.createTreeView('treeDataProvider', { treeDataProvider: aCompleteNodeTreeDataProvider() }, { enableProposedApi: true } as IExtensionDescription);
553582
return loadCompleteTree('treeDataProvider')
554583
.then(() => {
555-
tree = {
556-
'a': {
557-
'aa': {},
558-
'ac': {}
559-
},
560-
'b': {
561-
'ba': {},
562-
'bb': {}
563-
}
564-
};
565-
onDidChangeTreeNode.fire(getNode('a'));
566-
tree = {
567-
'a': {
568-
'aa': {},
569-
'ac': {}
570-
},
571-
'b': {
572-
'ba': {},
573-
'bc': {}
574-
}
575-
};
576-
onDidChangeTreeNode.fire(getNode('b'));
577-
578-
return treeView.reveal({ key: 'bc' })
579-
.then(() => {
580-
assert.ok(revealTarget.calledOnce);
581-
assert.deepEqual('treeDataProvider', revealTarget.args[0][0]);
582-
assert.deepEqual({ handle: '0/0:b/0:bc', label: { label: 'bc' }, collapsibleState: TreeItemCollapsibleState.None, parentHandle: '0/0:b' }, removeUnsetKeys(revealTarget.args[0][1]));
583-
assert.deepEqual([{ handle: '0/0:b', label: { label: 'b' }, collapsibleState: TreeItemCollapsibleState.Collapsed }], (<Array<any>>revealTarget.args[0][2]).map(arg => removeUnsetKeys(arg)));
584-
assert.deepEqual({ select: true, focus: false, expand: false }, revealTarget.args[0][3]);
585-
});
584+
runWithEventMerging((resolve) => {
585+
tree = {
586+
'a': {
587+
'aa': {},
588+
'ac': {}
589+
},
590+
'b': {
591+
'ba': {},
592+
'bb': {}
593+
}
594+
};
595+
onDidChangeTreeNode.fire(getNode('a'));
596+
tree = {
597+
'a': {
598+
'aa': {},
599+
'ac': {}
600+
},
601+
'b': {
602+
'ba': {},
603+
'bc': {}
604+
}
605+
};
606+
onDidChangeTreeNode.fire(getNode('b'));
607+
resolve();
608+
}).then(() => {
609+
return treeView.reveal({ key: 'bc' })
610+
.then(() => {
611+
assert.ok(revealTarget.calledOnce);
612+
assert.deepEqual('treeDataProvider', revealTarget.args[0][0]);
613+
assert.deepEqual({ handle: '0/0:b/0:bc', label: { label: 'bc' }, collapsibleState: TreeItemCollapsibleState.None, parentHandle: '0/0:b' }, removeUnsetKeys(revealTarget.args[0][1]));
614+
assert.deepEqual([{ handle: '0/0:b', label: { label: 'b' }, collapsibleState: TreeItemCollapsibleState.Collapsed }], (<Array<any>>revealTarget.args[0][2]).map(arg => removeUnsetKeys(arg)));
615+
assert.deepEqual({ select: true, focus: false, expand: false }, revealTarget.args[0][3]);
616+
});
617+
});
586618
});
587619
});
588620

0 commit comments

Comments
 (0)