From db9a43608a4f61e29d46f7487027787c33c275c3 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Fri, 11 Feb 2022 14:54:12 +0100 Subject: [PATCH] swap deprecation note with explainer --- src/vs/base/common/event.ts | 44 ++++++++++++------- src/vs/base/common/lifecycle.ts | 2 +- .../electron-sandbox/nativeHostService.ts | 2 +- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/vs/base/common/event.ts b/src/vs/base/common/event.ts index f6aa1dc1f18..ab883e5b9c9 100644 --- a/src/vs/base/common/event.ts +++ b/src/vs/base/common/event.ts @@ -50,21 +50,27 @@ export namespace Event { } /** - * @deprecated DO NOT use, this leaks memory + * *NOTE* that this function returns an `Event` and it MUST be called with a `DisposableStore` whenever the returned + * event is accessible to "third parties", e.g the event is a public property. Otherwise a leaked listener on the + * returned event causes this utility to leak a listener on the original event. */ export function map(event: Event, map: (i: I) => O, disposable?: DisposableStore): Event { return snapshot((listener, thisArgs = null, disposables?) => event(i => listener.call(thisArgs, map(i)), null, disposables), disposable); } /** - * @deprecated DO NOT use, this leaks memory + * *NOTE* that this function returns an `Event` and it MUST be called with a `DisposableStore` whenever the returned + * event is accessible to "third parties", e.g the event is a public property. Otherwise a leaked listener on the + * returned event causes this utility to leak a listener on the original event. */ export function forEach(event: Event, each: (i: I) => void, disposable?: DisposableStore): Event { return snapshot((listener, thisArgs = null, disposables?) => event(i => { each(i); listener.call(thisArgs, i); }, null, disposables), disposable); } /** - * @deprecated DO NOT use, this leaks memory + * *NOTE* that this function returns an `Event` and it MUST be called with a `DisposableStore` whenever the returned + * event is accessible to "third parties", e.g the event is a public property. Otherwise a leaked listener on the + * returned event causes this utility to leak a listener on the original event. */ export function filter(event: Event, filter: (e: T | U) => e is T, disposable?: DisposableStore): Event; export function filter(event: Event, filter: (e: T) => boolean, disposable?: DisposableStore): Event; @@ -91,7 +97,9 @@ export namespace Event { } /** - * @deprecated DO NOT use, this leaks memory + * *NOTE* that this function returns an `Event` and it MUST be called with a `DisposableStore` whenever the returned + * event is accessible to "third parties", e.g the event is a public property. Otherwise a leaked listener on the + * returned event causes this utility to leak a listener on the original event. */ export function reduce(event: Event, merge: (last: O | undefined, event: I) => O, initial?: O, disposable?: DisposableStore): Event { let output: O | undefined = initial; @@ -103,19 +111,19 @@ export namespace Event { } function snapshot(event: Event, disposable: DisposableStore | undefined): Event { - // let stack = Stacktrace.create(); - // let count = 0; + let stack = Stacktrace.create(); + let count = 0; let listener: IDisposable; const emitter = new Emitter({ onFirstListenerAdd() { listener = event(emitter.fire, emitter); }, - // onListenerDidAdd() { - // if (++count === 2) { - // console.warn('snapshotted emitter LIKELY used public and SHOULD HAVE BEEN created with DisposableStore. snapshotted here'); - // stack.print(); - // } - // }, + onListenerDidAdd() { + if (++count === 2) { + console.warn('snapshotted emitter LIKELY used public and SHOULD HAVE BEEN created with DisposableStore. snapshotted here'); + stack.print(); + } + }, onLastListenerRemove() { listener.dispose(); } @@ -213,7 +221,9 @@ export namespace Event { } /** - * @deprecated DO NOT use, this leaks memory + * *NOTE* that this function returns an `Event` and it MUST be called with a `DisposableStore` whenever the returned + * event is accessible to "third parties", e.g the event is a public property. Otherwise a leaked listener on the + * returned event causes this utility to leak a listener on the original event. */ export function latch(event: Event, equals: (a: T, b: T) => boolean = (a, b) => a === b, disposable?: DisposableStore): Event { let firstCall = true; @@ -228,7 +238,9 @@ export namespace Event { } /** - * @deprecated DO NOT use, this leaks memory + * *NOTE* that this function returns an `Event` and it MUST be called with a `DisposableStore` whenever the returned + * event is accessible to "third parties", e.g the event is a public property. Otherwise a leaked listener on the + * returned event causes this utility to leak a listener on the original event. */ export function split(event: Event, isT: (e: T | U) => e is T, disposable?: DisposableStore): [Event, Event] { return [ @@ -238,7 +250,9 @@ export namespace Event { } /** - * @deprecated DO NOT use, this leaks memory + * *NOTE* that this function returns an `Event` and it MUST be called with a `DisposableStore` whenever the returned + * event is accessible to "third parties", e.g the event is a public property. Otherwise a leaked listener on the + * returned event causes this utility to leak a listener on the original event. */ export function buffer(event: Event, flushAfterTimeout = false, _buffer: T[] = []): Event { let buffer: T[] | null = _buffer.slice(); diff --git a/src/vs/base/common/lifecycle.ts b/src/vs/base/common/lifecycle.ts index 471d98b6487..3283d07c575 100644 --- a/src/vs/base/common/lifecycle.ts +++ b/src/vs/base/common/lifecycle.ts @@ -242,7 +242,7 @@ export abstract class Disposable implements IDisposable { static readonly None = Object.freeze({ dispose() { } }); - private readonly _store = new DisposableStore(); + protected readonly _store = new DisposableStore(); constructor() { trackDisposable(this); diff --git a/src/vs/workbench/services/host/electron-sandbox/nativeHostService.ts b/src/vs/workbench/services/host/electron-sandbox/nativeHostService.ts index 81576aadd15..cbe6beb7e38 100644 --- a/src/vs/workbench/services/host/electron-sandbox/nativeHostService.ts +++ b/src/vs/workbench/services/host/electron-sandbox/nativeHostService.ts @@ -43,7 +43,7 @@ class WorkbenchHostService extends Disposable implements IHostService { private _onDidChangeFocus: Event = Event.latch(Event.any( Event.map(Event.filter(this.nativeHostService.onDidFocusWindow, id => id === this.nativeHostService.windowId), () => this.hasFocus), Event.map(Event.filter(this.nativeHostService.onDidBlurWindow, id => id === this.nativeHostService.windowId), () => this.hasFocus) - )); + ), undefined, this._store); get hasFocus(): boolean { return document.hasFocus();