From bc7e1fc60904d89b111bb107f6fa0b9f7b331d3a Mon Sep 17 00:00:00 2001 From: Henning Dieterichs Date: Fri, 16 Feb 2024 10:20:57 +0100 Subject: [PATCH] ObservablePromise cleanup --- src/vs/base/common/observable.ts | 9 ++- .../base/common/observableInternal/derived.ts | 2 +- .../base/common/observableInternal/promise.ts | 71 +++++++++++++++---- .../browser/multiDiffEditorInput.ts | 8 +-- 4 files changed, 70 insertions(+), 20 deletions(-) diff --git a/src/vs/base/common/observable.ts b/src/vs/base/common/observable.ts index 978c212d765..155ad2aa4f6 100644 --- a/src/vs/base/common/observable.ts +++ b/src/vs/base/common/observable.ts @@ -49,15 +49,20 @@ export { } from 'vs/base/common/observableInternal/utils'; export { ObservableLazy, - ObservableLazyStatefulPromise, + ObservableLazyPromise, ObservablePromise, PromiseResult, waitForState, + derivedWithCancellationToken, } from 'vs/base/common/observableInternal/promise'; import { ConsoleObservableLogger, setLogger } from 'vs/base/common/observableInternal/logging'; -const enableLogging = false; +// Remove "//" in the next line to enable logging +const enableLogging = false + // || Boolean("true") // done "weirdly" so that a lint warning prevents you from pushing this + ; + if (enableLogging) { setLogger(new ConsoleObservableLogger()); } diff --git a/src/vs/base/common/observableInternal/derived.ts b/src/vs/base/common/observableInternal/derived.ts index cab2c7c4bc1..70dc7168418 100644 --- a/src/vs/base/common/observableInternal/derived.ts +++ b/src/vs/base/common/observableInternal/derived.ts @@ -10,7 +10,7 @@ import { DebugNameData, IDebugNameData, Owner } from 'vs/base/common/observableI import { getLogger } from 'vs/base/common/observableInternal/logging'; export type EqualityComparer = (a: T, b: T) => boolean; -const defaultEqualityComparer: EqualityComparer = (a, b) => a === b; +export const defaultEqualityComparer: EqualityComparer = (a, b) => a === b; /** * Creates an observable that is derived from other observables. diff --git a/src/vs/base/common/observableInternal/promise.ts b/src/vs/base/common/observableInternal/promise.ts index 23292cdea50..363ffea7d6e 100644 --- a/src/vs/base/common/observableInternal/promise.ts +++ b/src/vs/base/common/observableInternal/promise.ts @@ -3,8 +3,10 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ import { autorun } from 'vs/base/common/observableInternal/autorun'; -import { IObservable, observableValue } from './base'; -import { derived } from 'vs/base/common/observableInternal/derived'; +import { IObservable, IReader, observableValue, transaction } from './base'; +import { Derived, defaultEqualityComparer, derived } from 'vs/base/common/observableInternal/derived'; +import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation'; +import { DebugNameData, Owner } from 'vs/base/common/observableInternal/debugName'; export class ObservableLazy { private readonly _value = observableValue(this, undefined); @@ -38,15 +40,29 @@ export class ObservableLazy { export class ObservablePromise { private readonly _value = observableValue | undefined>(this, undefined); + /** + * The promise that this object wraps. + */ public readonly promise: Promise; - public readonly value: IObservable | undefined> = this._value; + + /** + * The current state of the promise. + * Is `undefined` if the promise didn't resolve yet. + */ + public readonly promiseResult: IObservable | undefined> = this._value; constructor(promise: Promise) { this.promise = promise.then(value => { - this._value.set(new PromiseResult(value, undefined), undefined); + transaction(tx => { + /** @description onPromiseResolved */ + this._value.set(new PromiseResult(value, undefined), tx); + }); return value; }, error => { - this._value.set(new PromiseResult(undefined, error), undefined); + transaction(tx => { + /** @description onPromiseRejected */ + this._value.set(new PromiseResult(undefined, error), tx); + }); throw error; }); } @@ -58,7 +74,7 @@ export class PromiseResult { * The value of the resolved promise. * Undefined if the promise rejected. */ - public readonly value: T | undefined, + public readonly data: T | undefined, /** * The error in case of a rejected promise. @@ -71,30 +87,30 @@ export class PromiseResult { /** * Returns the value if the promise resolved, otherwise throws the error. */ - public getValue(): T { + public getDataOrThrow(): T { if (this.error) { throw this.error; } - return this.value!; + return this.data!; } } /** * A lazy promise whose state is observable. */ -export class ObservableLazyStatefulPromise { - private readonly _lazyValue = new ObservableLazy(() => new ObservablePromise(this._computeValue())); +export class ObservableLazyPromise { + private readonly _lazyValue = new ObservableLazy(() => new ObservablePromise(this._computePromise())); /** * Does not enforce evaluation of the promise compute function. * Is undefined if the promise has not been computed yet. */ - public readonly cachedValue = derived(this, reader => this._lazyValue.cachedValue.read(reader)?.value.read(reader)); + public readonly cachedPromiseResult = derived(this, reader => this._lazyValue.cachedValue.read(reader)?.promiseResult.read(reader)); - constructor(private readonly _computeValue: () => Promise) { + constructor(private readonly _computePromise: () => Promise) { } - public getValue(): Promise { + public getPromise(): Promise { return this._lazyValue.getValue().promise; } } @@ -139,3 +155,32 @@ export function waitForState(observable: IObservable, predicate: (state: T } }); } + +export function derivedWithCancellationToken(computeFn: (reader: IReader, cancellationToken: CancellationToken) => T): IObservable; +export function derivedWithCancellationToken(owner: object, computeFn: (reader: IReader, cancellationToken: CancellationToken) => T): IObservable; +export function derivedWithCancellationToken(computeFnOrOwner: ((reader: IReader, cancellationToken: CancellationToken) => T) | object, computeFnOrUndefined?: ((reader: IReader, cancellationToken: CancellationToken) => T)): IObservable { + let computeFn: (reader: IReader, store: CancellationToken) => T; + let owner: Owner; + if (computeFnOrUndefined === undefined) { + computeFn = computeFnOrOwner as any; + owner = undefined; + } else { + owner = computeFnOrOwner; + computeFn = computeFnOrUndefined as any; + } + + let cancellationTokenSource: CancellationTokenSource | undefined = undefined; + return new Derived( + new DebugNameData(owner, undefined, computeFn), + r => { + if (cancellationTokenSource) { + cancellationTokenSource.dispose(true); + } + cancellationTokenSource = new CancellationTokenSource(); + return computeFn(r, cancellationTokenSource.token); + }, undefined, + undefined, + () => cancellationTokenSource?.dispose(), + defaultEqualityComparer, + ); +} diff --git a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditorInput.ts b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditorInput.ts index b813d41bff3..cffa05225e6 100644 --- a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditorInput.ts +++ b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditorInput.ts @@ -11,7 +11,7 @@ import { Disposable, DisposableStore, IDisposable, IReference, toDisposable } fr import { parse } from 'vs/base/common/marshalling'; import { Schemas } from 'vs/base/common/network'; import { deepClone } from 'vs/base/common/objects'; -import { ObservableLazyStatefulPromise, autorun, derived, observableFromEvent } from 'vs/base/common/observable'; +import { ObservableLazyPromise, autorun, derived, observableFromEvent } from 'vs/base/common/observable'; import { constObservable, mapObservableArrayCached } from 'vs/base/common/observableInternal/utils'; import { ThemeIcon } from 'vs/base/common/themables'; import { isDefined, isObject } from 'vs/base/common/types'; @@ -131,7 +131,7 @@ export class MultiDiffEditorInput extends EditorInput implements ILanguageSuppor }); private async _createModel(): Promise { - const source = await this._resolvedSource.getValue(); + const source = await this._resolvedSource.getPromise(); const textResourceConfigurationService = this._textResourceConfigurationService; // Enables delayed disposing @@ -207,7 +207,7 @@ export class MultiDiffEditorInput extends EditorInput implements ILanguageSuppor }; } - private readonly _resolvedSource = new ObservableLazyStatefulPromise(async () => { + private readonly _resolvedSource = new ObservableLazyPromise(async () => { const source: IResolvedMultiDiffSource | undefined = this.initialResources ? new ConstResolvedMultiDiffSource(this.initialResources) : await this._multiDiffSourceResolverService.resolve(this.multiDiffSource); @@ -229,7 +229,7 @@ export class MultiDiffEditorInput extends EditorInput implements ILanguageSuppor return false; } - private readonly _resources = derived(this, reader => this._resolvedSource.cachedValue.read(reader)?.value?.resources.read(reader)); + private readonly _resources = derived(this, reader => this._resolvedSource.cachedPromiseResult.read(reader)?.data?.resources.read(reader)); private readonly _isDirtyObservables = mapObservableArrayCached(this, this._resources.map(r => r || []), res => { const isModifiedDirty = res.modified ? isUriDirty(this._textFileService, res.modified) : constObservable(false); const isOriginalDirty = res.original ? isUriDirty(this._textFileService, res.original) : constObservable(false);