From 448267744255bfc58c0fa36f5082fe25bd752bfc Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Tue, 20 Jul 2021 09:52:31 -0700 Subject: [PATCH] notebooks: update renderer messaging api to feedback --- .../platform/extensions/common/extensions.ts | 5 +++++ src/vs/vscode.d.ts | 16 ++++++++------ .../browser/mainThreadNotebookRenderers.ts | 4 ++-- .../workbench/api/common/extHost.api.impl.ts | 2 +- .../workbench/api/common/extHost.protocol.ts | 2 +- .../api/common/extHostNotebookRenderers.ts | 10 +++++++-- .../notebookRendererMessagingServiceImpl.ts | 21 ++++++++++++------- .../view/renderers/backLayerWebView.ts | 15 +++++++++---- .../notebookRendererMessagingService.ts | 10 +++++---- .../notebookRendererMessagingService.test.ts | 2 -- 10 files changed, 58 insertions(+), 29 deletions(-) diff --git a/src/vs/platform/extensions/common/extensions.ts b/src/vs/platform/extensions/common/extensions.ts index da8a2c098a6..100ff885f3f 100644 --- a/src/vs/platform/extensions/common/extensions.ts +++ b/src/vs/platform/extensions/common/extensions.ts @@ -142,6 +142,10 @@ export interface IStartEntry { readonly category: 'file' | 'folder' | 'notebook'; } +export interface INotebookRendererContribution { + readonly id: string; +} + export interface IExtensionContributions { commands?: ICommand[]; configuration?: IConfiguration | IConfiguration[]; @@ -164,6 +168,7 @@ export interface IExtensionContributions { authentication?: IAuthenticationContribution[]; walkthroughs?: IWalkthrough[]; startEntries?: IStartEntry[]; + readonly notebookRenderer?: INotebookRendererContribution[]; } export interface IExtensionCapabilities { diff --git a/src/vs/vscode.d.ts b/src/vs/vscode.d.ts index afb75829156..2bd0c5e27bb 100644 --- a/src/vs/vscode.d.ts +++ b/src/vs/vscode.d.ts @@ -11553,6 +11553,8 @@ declare module 'vscode' { /** * Represents a notebook editor that is attached to a {@link NotebookDocument notebook}. + * Additional properties of the NotebookEditor are available in the proposed + * API, which will be finalized later. */ export interface NotebookEditor { @@ -11566,12 +11568,12 @@ declare module 'vscode' { /** * Editor that sent the message. */ - editor: NotebookEditor; + readonly editor: NotebookEditor; /** * Message sent from the webview. */ - message: any; + readonly message: any; } /** @@ -11582,14 +11584,15 @@ declare module 'vscode' { /** * Events that fires when a message is received from a renderer. */ - onDidReceiveMessage: Event; + readonly onDidReceiveMessage: Event; /** * Sends a message to the renderer. * @param editor Editor to target with the message * @param message Message to send + * @returns a boolean indicating whether the message was successfully delivered */ - postMessage(editor: NotebookEditor, message: unknown): void; + postMessage(editor: NotebookEditor, message: any): Thenable; } /** @@ -12329,8 +12332,9 @@ declare module 'vscode' { /** * Creates a new messaging instance used to communicate with a specific - * renderer. The renderer only has access to messaging if `requiresMessaging` - * is set to `always` or `optional` in its `notebookRenderer ` contribution. + * renderer defined in this extension's package.json. The renderer only + * has access to messaging if `requiresMessaging` is set to `always` or + * `optional` in its `notebookRenderer ` contribution. * * @see https://github.com/microsoft/vscode/issues/123601 * @param rendererId The renderer ID to communicate with diff --git a/src/vs/workbench/api/browser/mainThreadNotebookRenderers.ts b/src/vs/workbench/api/browser/mainThreadNotebookRenderers.ts index 5adf89ce7a7..6ebc512ddbe 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebookRenderers.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebookRenderers.ts @@ -23,7 +23,7 @@ export class MainThreadNotebookRenderers extends Disposable implements MainThrea })); } - $postMessage(editorId: string, rendererId: string, message: unknown): void { - this.messaging.fireDidReceiveMessage(editorId, rendererId, message); + $postMessage(editorId: string, rendererId: string, message: unknown): Promise { + return this.messaging.receiveMessage(editorId, rendererId, message); } } diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index c15d58daee0..b2f9bbac361 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -1086,7 +1086,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I return extHostNotebookEditors.createNotebookEditorDecorationType(options); }, createRendererMessaging(rendererId) { - return extHostNotebookRenderers.createRendererMessaging(!!extension.enableProposedApi, rendererId); + return extHostNotebookRenderers.createRendererMessaging(extension, rendererId); }, onDidChangeNotebookDocumentMetadata(listener, thisArgs?, disposables?) { checkProposedApiEnabled(extension); diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 951e83cd9d8..2478a77de03 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -933,7 +933,7 @@ export interface MainThreadNotebookKernelsShape extends IDisposable { } export interface MainThreadNotebookRenderersShape extends IDisposable { - $postMessage(editorId: string, rendererId: string, message: unknown): void; + $postMessage(editorId: string, rendererId: string, message: unknown): Promise; } export interface MainThreadInteractiveShape extends IDisposable { diff --git a/src/vs/workbench/api/common/extHostNotebookRenderers.ts b/src/vs/workbench/api/common/extHostNotebookRenderers.ts index 75ce3b9c964..edda61984d0 100644 --- a/src/vs/workbench/api/common/extHostNotebookRenderers.ts +++ b/src/vs/workbench/api/common/extHostNotebookRenderers.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { Emitter } from 'vs/base/common/event'; +import { IExtensionManifest } from 'vs/platform/extensions/common/extensions'; import { ExtHostNotebookRenderersShape, IMainContext, MainContext, MainThreadNotebookRenderersShape } from 'vs/workbench/api/common/extHost.protocol'; import { ExtHostNotebookController } from 'vs/workbench/api/common/extHostNotebook'; import { ExtHostNotebookEditor } from 'vs/workbench/api/common/extHostNotebookEditor'; @@ -23,9 +24,14 @@ export class ExtHostNotebookRenderers implements ExtHostNotebookRenderersShape { this._rendererMessageEmitters.get(rendererId)?.fire({ editor: editor.apiEditor, message }); } - public createRendererMessaging(notebookEditorVisible: boolean, rendererId: string): vscode.NotebookRendererMessaging { + public createRendererMessaging(manifest: IExtensionManifest, rendererId: string): vscode.NotebookRendererMessaging { + if (!manifest.contributes?.notebookRenderer?.some(r => r.id === rendererId)) { + throw new Error(`Extensions may only call createRendererMessaging() for renderers they contribute (got ${rendererId})`); + } + // In the stable API, the editor is given as an empty object, and this map // is used to maintain references. This can be removed after editor finalization. + const notebookEditorVisible = !!manifest.enableProposedApi; const notebookEditorAliases = new WeakMap<{}, vscode.NotebookEditor>(); const messaging: vscode.NotebookRendererMessaging = { @@ -45,7 +51,7 @@ export class ExtHostNotebookRenderers implements ExtHostNotebookRenderersShape { throw new Error(`The first argument to postMessage() must be a NotebookEditor`); } - this.proxy.$postMessage(extHostEditor.id, rendererId, message); + return this.proxy.$postMessage(extHostEditor.id, rendererId, message); }, }; diff --git a/src/vs/workbench/contrib/notebook/browser/notebookRendererMessagingServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookRendererMessagingServiceImpl.ts index 0d816f42fb5..eb7fe07898d 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookRendererMessagingServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookRendererMessagingServiceImpl.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { Emitter, Event } from 'vs/base/common/event'; +import { Emitter } from 'vs/base/common/event'; import { INotebookRendererMessagingService, IScopedRendererMessaging } from 'vs/workbench/contrib/notebook/common/notebookRendererMessagingService'; import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; @@ -16,16 +16,15 @@ export class NotebookRendererMessagingService implements INotebookRendererMessag * be sent once activation finishes, or undefined if activation is complete. */ private readonly activations = new Map(); - private readonly receiveMessageEmitter = new Emitter<{ editorId: string; rendererId: string, message: unknown }>(); - public readonly onDidReceiveMessage = this.receiveMessageEmitter.event; + private readonly scopedMessaging = new Map(); private readonly postMessageEmitter = new Emitter(); public readonly onShouldPostMessage = this.postMessageEmitter.event; constructor(@IExtensionService private readonly extensionService: IExtensionService) { } /** @inheritdoc */ - public fireDidReceiveMessage(editorId: string, rendererId: string, message: unknown): void { - this.receiveMessageEmitter.fire({ editorId, rendererId, message }); + public receiveMessage(editorId: string, rendererId: string, message: unknown): Promise { + return this.scopedMessaging.get(editorId)?.receiveMessageHandler?.(rendererId, message) ?? Promise.resolve(false); } /** @inheritdoc */ @@ -48,10 +47,18 @@ export class NotebookRendererMessagingService implements INotebookRendererMessag /** @inheritdoc */ public getScoped(editorId: string): IScopedRendererMessaging { - return { - onDidReceiveMessage: Event.filter(this.onDidReceiveMessage, e => e.editorId === editorId), + const existing = this.scopedMessaging.get(editorId); + if (existing) { + return existing; + } + + const messaging: IScopedRendererMessaging = { postMessage: (rendererId, message) => this.postMessage(editorId, rendererId, message), + dispose: () => this.scopedMessaging.delete(editorId), }; + + this.scopedMessaging.set(editorId, messaging); + return messaging; } private postMessage(editorId: string, rendererId: string, message: unknown): void { diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts index e16fdf953fe..58539657764 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts @@ -111,14 +111,21 @@ export class BackLayerWebView extends Disposable { this.element.style.position = 'absolute'; if (rendererMessaging) { - this._register(rendererMessaging.onDidReceiveMessage(evt => { + this._register(rendererMessaging); + rendererMessaging.receiveMessageHandler = (rendererId, message) => { + if (!this.webview || this._disposed) { + return Promise.resolve(false); + } + this._sendMessageToWebview({ __vscode_notebook_message: true, type: 'customRendererMessage', - rendererId: evt.rendererId, - message: evt.message + rendererId: rendererId, + message: message }); - })); + + return Promise.resolve(true); + }; } } diff --git a/src/vs/workbench/contrib/notebook/common/notebookRendererMessagingService.ts b/src/vs/workbench/contrib/notebook/common/notebookRendererMessagingService.ts index 2264baa168f..f5701025e74 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookRendererMessagingService.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookRendererMessagingService.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { Event } from 'vs/base/common/event'; +import { IDisposable } from 'vs/base/common/lifecycle'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; export const INotebookRendererMessagingService = createDecorator('INotebookRendererMessagingService'); @@ -28,14 +29,15 @@ export interface INotebookRendererMessagingService { /** * Called when the main thread gets a message for a renderer. */ - fireDidReceiveMessage(editorId: string, rendererId: string, message: unknown): void; + receiveMessage(editorId: string, rendererId: string, message: unknown): Promise; } -export interface IScopedRendererMessaging { +export interface IScopedRendererMessaging extends IDisposable { /** - * Event that fires when a message is received. + * Method called when a message is received. Should return a boolean + * indicating whether a renderer received it. */ - onDidReceiveMessage: Event<{ rendererId: string; message: unknown }>; + receiveMessageHandler?: (rendererId: string, message: unknown) => Promise; /** * Sends a message to an extension from a renderer. diff --git a/src/vs/workbench/contrib/notebook/test/notebookRendererMessagingService.test.ts b/src/vs/workbench/contrib/notebook/test/notebookRendererMessagingService.test.ts index b5c2774c766..715d65994cc 100644 --- a/src/vs/workbench/contrib/notebook/test/notebookRendererMessagingService.test.ts +++ b/src/vs/workbench/contrib/notebook/test/notebookRendererMessagingService.test.ts @@ -13,14 +13,12 @@ suite('NotebookRendererMessaging', () => { let extService: NullExtensionService; let m: NotebookRendererMessagingService; let sent: unknown[] = []; - let received: unknown[] = []; setup(() => { sent = []; extService = new NullExtensionService(); m = new NotebookRendererMessagingService(extService); m.onShouldPostMessage(e => sent.push(e)); - m.onDidReceiveMessage(e => received.push(e)); }); test('activates on prepare', () => {