diff --git a/src/vs/base/common/async.ts b/src/vs/base/common/async.ts index 6925d74b686..bb39088be12 100644 --- a/src/vs/base/common/async.ts +++ b/src/vs/base/common/async.ts @@ -170,6 +170,25 @@ export class Sequencer { } } +export class SequencerByKey { + + private promiseMap = new Map>(); + + queue(key: TKey, promiseTask: ITask>): Promise { + const runningPromise = this.promiseMap.get(key) ?? Promise.resolve(); + const newPromise = runningPromise + .catch(() => { }) + .then(promiseTask) + .finally(() => { + if (this.promiseMap.get(key) === newPromise) { + this.promiseMap.delete(key); + } + }); + this.promiseMap.set(key, newPromise); + return newPromise; + } +} + /** * A helper to delay execution of a task that is being requested often. * diff --git a/src/vs/base/test/common/async.test.ts b/src/vs/base/test/common/async.test.ts index 25e7d83cc7b..9d133c91dfa 100644 --- a/src/vs/base/test/common/async.test.ts +++ b/src/vs/base/test/common/async.test.ts @@ -688,4 +688,22 @@ suite('Async', () => { assert.ok(Date.now() - now < 100); assert.equal(timedout, false); }); + + test('SequencerByKey', async () => { + const s = new async.SequencerByKey(); + + const r1 = await s.queue('key1', () => Promise.resolve('hello')); + assert.equal(r1, 'hello'); + + await s.queue('key2', () => Promise.reject(new Error('failed'))).then(() => { + throw new Error('should not be resolved'); + }, err => { + // Expected error + assert.equal(err.message, 'failed'); + }); + + // Still works after a queued promise is rejected + const r3 = await s.queue('key2', () => Promise.resolve('hello')); + assert.equal(r3, 'hello'); + }); }); diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index e65eca7edf8..bffba22197b 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -8,6 +8,7 @@ import * as DOM from 'vs/base/browser/dom'; import { IMouseWheelEvent, StandardMouseEvent } from 'vs/base/browser/mouseEvent'; import { IListContextMenuEvent } from 'vs/base/browser/ui/list/list'; import { IAction, Separator } from 'vs/base/common/actions'; +import { SequencerByKey } from 'vs/base/common/async'; import { CancellationTokenSource } from 'vs/base/common/cancellation'; import { Color, RGBA } from 'vs/base/common/color'; import { onUnexpectedError } from 'vs/base/common/errors'; @@ -51,7 +52,7 @@ import { CodeCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewMod import { NotebookEventDispatcher, NotebookLayoutChangedEvent } from 'vs/workbench/contrib/notebook/browser/viewModel/eventDispatcher'; import { CellViewModel, IModelDecorationsChangeAccessor, INotebookEditorViewState, NotebookViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; -import { CellKind, CellToolbarLocKey, ICellRange, IInsetRenderOutput, INotebookKernelInfo, INotebookKernelInfo2, INotebookKernelInfoDto, IProcessedOutput, NotebookCellRunState, NotebookRunState, ShowCellStatusbarKey } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellKind, CellToolbarLocKey, ICellRange, IInsetRenderOutput, INotebookKernelInfo, INotebookKernelInfo2, INotebookKernelInfoDto, IProcessedOutput, isTransformedDisplayOutput, NotebookCellRunState, NotebookRunState, ShowCellStatusbarKey } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { NotebookProviderInfo } from 'vs/workbench/contrib/notebook/common/notebookProvider'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; import { editorGutterModifiedBackground } from 'vs/workbench/contrib/scm/browser/dirtydiffDecorator'; @@ -101,6 +102,8 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor private readonly _onWillDispose = this._register(new Emitter()); public readonly onWillDispose: Event = this._onWillDispose.event; + private readonly _insetModifyQueueByOutputId = new SequencerByKey(); + set scrollTop(top: number) { if (this._list) { this._list.scrollTop = top; @@ -1577,30 +1580,37 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor this._list?.triggerScrollFromMouseWheelEvent(event); } - async createInset(cell: CodeCellViewModel, output: IInsetRenderOutput, offset: number) { - if (!this._webview) { - return; - } + async createInset(cell: CodeCellViewModel, output: IInsetRenderOutput, offset: number): Promise { + this._insetModifyQueueByOutputId.queue(output.source.outputId, async () => { + if (!this._webview) { + return; + } - await this._resolveWebview(); + await this._resolveWebview(); - if (!this._webview!.insetMapping.has(output.source)) { - const cellTop = this._list?.getAbsoluteTopOfElement(cell) || 0; - await this._webview!.createInset(cell, output, cellTop, offset); - } else { - const cellTop = this._list?.getAbsoluteTopOfElement(cell) || 0; - const scrollTop = this._list?.scrollTop || 0; + if (!this._webview!.insetMapping.has(output.source)) { + const cellTop = this._list?.getAbsoluteTopOfElement(cell) || 0; + await this._webview!.createInset(cell, output, cellTop, offset); + } else { + const cellTop = this._list?.getAbsoluteTopOfElement(cell) || 0; + const scrollTop = this._list?.scrollTop || 0; - this._webview!.updateViewScrollTop(-scrollTop, true, [{ cell, output: output.source, cellTop }]); - } + this._webview!.updateViewScrollTop(-scrollTop, true, [{ cell, output: output.source, cellTop }]); + } + }); } removeInset(output: IProcessedOutput) { - if (!this._webview || !this._webviewResolved) { + if (!isTransformedDisplayOutput(output)) { return; } - this._webview!.removeInset(output); + this._insetModifyQueueByOutputId.queue(output.outputId, async () => { + if (!this._webview || !this._webviewResolved) { + return; + } + this._webview!.removeInset(output); + }); } hideInset(output: IProcessedOutput) { @@ -1608,7 +1618,13 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor return; } - this._webview!.hideInset(output); + if (!isTransformedDisplayOutput(output)) { + return; + } + + this._insetModifyQueueByOutputId.queue(output.outputId, async () => { + this._webview!.hideInset(output); + }); } getOutputRenderer(): OutputRenderer { diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/codeCell.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/codeCell.ts index 9ed49e93300..2917b4b0ed1 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/codeCell.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/codeCell.ts @@ -33,8 +33,6 @@ export class CodeCell extends Disposable { private outputResizeListeners = new Map(); private outputElements = new Map(); - private modifyInsetQueue = Promise.resolve(); - constructor( private notebookEditor: INotebookEditor, private viewCell: CodeCellViewModel, @@ -173,7 +171,7 @@ export class CodeCell extends Disposable { removedKeys.push(key); // remove element from DOM this.templateData?.outputContainer?.removeChild(value.element); - this.modifyInsetQueue = this.modifyInsetQueue.finally(() => this.notebookEditor.removeInset(key)); + this.notebookEditor.removeInset(key); } }); @@ -326,7 +324,7 @@ export class CodeCell extends Disposable { const renderedOutput = this.outputElements.get(currOutput); if (renderedOutput) { if (renderedOutput.renderResult.type !== RenderOutputType.None) { - this.modifyInsetQueue = this.modifyInsetQueue.finally(() => this.notebookEditor.createInset(this.viewCell, renderedOutput.renderResult as IInsetRenderOutput, this.viewCell.getOutputOffset(index))); + this.notebookEditor.createInset(this.viewCell, renderedOutput.renderResult as IInsetRenderOutput, this.viewCell.getOutputOffset(index)); } else { // Anything else, just update the height this.viewCell.updateOutputHeight(index, renderedOutput.element.clientHeight); @@ -515,7 +513,7 @@ export class CodeCell extends Disposable { if (result.type !== RenderOutputType.None) { this.viewCell.selfSizeMonitoring = true; - this.modifyInsetQueue = this.modifyInsetQueue.finally(() => this.notebookEditor.createInset(this.viewCell, result as any, this.viewCell.getOutputOffset(index))); + this.notebookEditor.createInset(this.viewCell, result as any, this.viewCell.getOutputOffset(index)); } else { DOM.addClass(outputItemDiv, 'foreground'); DOM.addClass(outputItemDiv, 'output-element'); @@ -610,7 +608,7 @@ export class CodeCell extends Disposable { const element = this.outputElements.get(output)?.element; if (element) { this.templateData?.outputContainer?.removeChild(element); - await (this.modifyInsetQueue = this.modifyInsetQueue.finally(() => this.notebookEditor.removeInset(output))); + this.notebookEditor.removeInset(output); } output.pickedMimeTypeIndex = pick; diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index c6be8ff48ab..2cf77f6ce69 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -215,6 +215,10 @@ export interface ITransformedDisplayOutputDto { pickedMimeTypeIndex?: number; } +export function isTransformedDisplayOutput(thing: unknown): thing is ITransformedDisplayOutputDto { + return (thing as ITransformedDisplayOutputDto).outputKind === CellOutputKind.Rich && !!(thing as ITransformedDisplayOutputDto).outputId; +} + export interface IGenericOutput { outputKind: CellOutputKind; pickedMimeType?: string; @@ -313,14 +317,14 @@ export interface IRenderNoOutput { export interface IRenderPlainHtmlOutput { type: RenderOutputType.Html; - source: IProcessedOutput; + source: ITransformedDisplayOutputDto; htmlContent: string; hasDynamicHeight: boolean; } export interface IRenderOutputViaExtension { type: RenderOutputType.Extension; - source: IProcessedOutput; + source: ITransformedDisplayOutputDto; mimeType: string; renderer: INotebookRendererInfo; }