Skip to content

Commit

Permalink
Fix issues with nested widgets
Browse files Browse the repository at this point in the history
  • Loading branch information
DonJayamanne committed Dec 20, 2024
1 parent 2cb7164 commit c06e165
Show file tree
Hide file tree
Showing 10 changed files with 285 additions and 28 deletions.
27 changes: 22 additions & 5 deletions src/kernels/execution/cellExecutionMessageHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ import {
NotebookEdit,
NotebookCellOutputItem,
Disposable,
window
window,
extensions
} from 'vscode';

import type { Kernel } from '@jupyterlab/services';
Expand All @@ -41,9 +42,10 @@ import { swallowExceptions } from '../../platform/common/utils/decorators';
import { noop } from '../../platform/common/utils/misc';
import { IKernelController, ITracebackFormatter } from '../../kernels/types';
import { handleTensorBoardDisplayDataOutput } from './executionHelpers';
import { Identifiers, WIDGET_MIMETYPE } from '../../platform/common/constants';
import { Identifiers, RendererExtension, WIDGET_MIMETYPE } from '../../platform/common/constants';
import { CellOutputDisplayIdTracker } from './cellDisplayIdTracker';
import { createDeferred } from '../../platform/common/utils/async';
import { coerce, SemVer } from 'semver';

// Helper interface for the set_next_input execute reply payload
interface ISetNextInputPayload {
Expand Down Expand Up @@ -714,13 +716,15 @@ export class CellExecutionMessageHandler implements IDisposable {
// Jupyter Output widgets cannot be rendered properly by the widget manager,
// We need to render that.
if (typeof data.model_id === 'string' && this.commIdsMappedToWidgetOutputModels.has(data.model_id)) {
return false;
// New version of renderer supports this.
return doesNotebookRendererSupportRenderingNestedOutputsInWidgets();
}
return true;
}
if (mime.startsWith('application/vnd')) {
// Custom vendored mimetypes cannot be rendered by the widget manager, it relies on the output renderers.
return false;
// Custom vendored mimetypes cannot be rendered by the widget manager in older versions, it relies on the output renderers.
// New version of renderer supports this.
return doesNotebookRendererSupportRenderingNestedOutputsInWidgets();
}
// Everything else can be rendered by the Jupyter Lab widget manager.
return true;
Expand Down Expand Up @@ -1209,3 +1213,16 @@ export class CellExecutionMessageHandler implements IDisposable {
this.endTemporaryTask();
}
}

function doesNotebookRendererSupportRenderingNestedOutputsInWidgets() {
const rendererExtension = extensions.getExtension(RendererExtension);
if (!rendererExtension) {
return false;
}

const version = coerce(rendererExtension.packageJSON.version);
if (!version) {
return false;
}
return version.compare(new SemVer('1.0.23')) >= 0;
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { IKernel, IKernelProvider, type IKernelSocket } from '../../../../kernel
import { IIPyWidgetMessageDispatcher, IPyWidgetMessage } from '../types';
import { shouldMessageBeMirroredWithRenderer } from '../../../../kernels/kernel';
import { KernelSocketMap } from '../../../../kernels/kernelSocket';
import type { IDisplayDataMsg } from '@jupyterlab/services/lib/kernel/messages';

type PendingMessage = {
resultPromise: Deferred<void>;
Expand Down Expand Up @@ -52,6 +53,8 @@ export class IPyWidgetMessageDispatcher implements IIPyWidgetMessageDispatcher {
private pendingTargetNames = new Set<string>();
private kernel?: IKernel;
private _postMessageEmitter = new EventEmitter<IPyWidgetMessage>();
private _onDisplayMessage = new EventEmitter<IDisplayDataMsg>();
public readonly onDisplayMessage = this._onDisplayMessage.event;
private messageHooks = new Map<string, (msg: KernelMessage.IIOPubMessage) => boolean | PromiseLike<boolean>>();
private pendingHookRemovals = new Map<string, string>();
private messageHookRequests = new Map<string, Deferred<boolean>>();
Expand Down Expand Up @@ -367,10 +370,17 @@ export class IPyWidgetMessageDispatcher implements IIPyWidgetMessageDispatcher {
const msgUuid = uuid();
const promise = createDeferred<void>();
this.waitingMessageIds.set(msgUuid, { startTime: Date.now(), resultPromise: promise });

let deserializedMessage: KernelMessage.IMessage | undefined = undefined;
if (typeof data === 'string') {
if (shouldMessageBeMirroredWithRenderer(data)) {
this.raisePostMessage(IPyWidgetMessages.IPyWidgets_msg, { id: msgUuid, data });
if (data.includes('display_data')) {
deserializedMessage = this.deserialize(data as any, protocol);
const jupyterLab = require('@jupyterlab/services') as typeof import('@jupyterlab/services');
if (jupyterLab.KernelMessage.isDisplayDataMsg(deserializedMessage)) {
this._onDisplayMessage.fire(deserializedMessage);
}
}
}
} else {
const dataToSend = serializeDataViews([data as any]);
Expand All @@ -392,7 +402,7 @@ export class IPyWidgetMessageDispatcher implements IIPyWidgetMessageDispatcher {
data.includes('comm_close') ||
data.includes('comm_msg');
if (mustDeserialize) {
const message = this.deserialize(data as any, protocol) as any;
const message = deserializedMessage || this.deserialize(data as any, protocol) as any;
if (!shouldMessageBeMirroredWithRenderer(message)) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { IPyWidgetMessages } from '../../../../messageTypes';
import { IKernel, IKernelProvider } from '../../../../kernels/types';
import { IPyWidgetMessageDispatcher } from './ipyWidgetMessageDispatcher';
import { IIPyWidgetMessageDispatcher, IPyWidgetMessage } from '../types';
import type { IDisplayDataMsg } from '@jupyterlab/services/lib/kernel/messages';

/**
* This just wraps the iPyWidgetMessageDispatcher class.
Expand All @@ -19,12 +20,21 @@ class IPyWidgetMessageDispatcherWithOldMessages implements IIPyWidgetMessageDisp
return this._postMessageEmitter.event;
}
private _postMessageEmitter = new EventEmitter<IPyWidgetMessage>();
private _onDisplayMessage = new EventEmitter<IDisplayDataMsg>();
public readonly onDisplayMessage = this._onDisplayMessage.event;
private readonly disposables: IDisposable[] = [];
constructor(
private readonly baseMulticaster: IPyWidgetMessageDispatcher,
private oldMessages: ReadonlyArray<IPyWidgetMessage>
) {
baseMulticaster.postMessage(this.raisePostMessage, this, this.disposables);
baseMulticaster.onDisplayMessage(
(e) => {
this._onDisplayMessage.fire(e);
},
this,
this.disposables
);
}

public dispose() {
Expand Down
2 changes: 2 additions & 0 deletions src/notebooks/controllers/ipywidgets/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { Event, Uri } from 'vscode';
import { IDisposable } from '../../../platform/common/types';
import { IPyWidgetMessages } from '../../../messageTypes';
import { IKernel } from '../../../kernels/types';
import type { IDisplayDataMsg } from '@jupyterlab/services/lib/kernel/messages';

export interface IPyWidgetMessage {
message: IPyWidgetMessages;
Expand All @@ -16,6 +17,7 @@ export interface IPyWidgetMessage {
* Used to send/receive messages related to IPyWidgets
*/
export interface IIPyWidgetMessageDispatcher extends IDisposable {
onDisplayMessage: Event<IDisplayDataMsg>;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
postMessage: Event<IPyWidgetMessage>;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down
20 changes: 19 additions & 1 deletion src/webviews/extension-side/ipywidgets/rendererComms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { dispose } from '../../../platform/common/utils/lifecycle';
import { IDisposable } from '../../../platform/common/types';
import { noop } from '../../../platform/common/utils/misc';
import { logger } from '../../../platform/logging';
import { IPyWidgetMessageDispatcherFactory } from '../../../notebooks/controllers/ipywidgets/message/ipyWidgetMessageDispatcherFactory';

type WidgetData = {
model_id: string;
Expand All @@ -27,7 +28,9 @@ export class IPyWidgetRendererComms implements IExtensionSyncActivationService {
private readonly disposables: IDisposable[] = [];
constructor(
@inject(IKernelProvider) private readonly kernelProvider: IKernelProvider,
@inject(IControllerRegistration) private readonly controllers: IControllerRegistration
@inject(IControllerRegistration) private readonly controllers: IControllerRegistration,
@inject(IPyWidgetMessageDispatcherFactory)
private readonly ipywidgetMessageDispatcher: IPyWidgetMessageDispatcherFactory
) {}
private readonly widgetOutputsPerNotebook = new WeakMap<NotebookDocument, Set<string>>();
public dispose() {
Expand All @@ -51,6 +54,21 @@ export class IPyWidgetRendererComms implements IExtensionSyncActivationService {
return;
}

// If we have an output widget nested within another output widget.
// Then the output output widget will be displayed by us.
// However nested outputs (any) widgets will be displayed by widget manager.
// And in this case, its possible the display_data message is sent to the webview,
// Sooner than we get the messages from the IKernel above.
// Hence we need to hook into the lower level kernel socket messages to see if that happens.
// Else what happens is the display_data is sent to the webview, but the widget manager doesn't know about it.
// Thats because we have not tracked this model and we don't know about it.
const ipyWidgetMessageDispatcher = this.ipywidgetMessageDispatcher.create(kernel.notebook);
this.disposables.push(
ipyWidgetMessageDispatcher.onDisplayMessage((msg) => {
this.trackModelId(kernel.notebook, msg);
})
);

// eslint-disable-next-line @typescript-eslint/no-require-imports
const jupyterLab = require('@jupyterlab/services') as typeof import('@jupyterlab/services');
const handler = (kernelConnection: IKernelConnection, msg: IIOPubMessage<IOPubMessageType>) => {
Expand Down
41 changes: 29 additions & 12 deletions src/webviews/webview-side/ipywidgets/kernel/kernel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,14 @@ class ProxyKernel implements IMessageHandler, Kernel.IKernelConnection {
private hookResults = new Map<string, boolean | PromiseLike<boolean>>();
private websocket: WebSocketWS & { sendEnabled: boolean };
private messageHook: (msg: KernelMessage.IIOPubMessage) => boolean | PromiseLike<boolean>;
private messageHooks: Map<string, (msg: KernelMessage.IIOPubMessage) => boolean | PromiseLike<boolean>>;
private lastHookedMessageId: string | undefined;
private readonly messageHooks = new Map<
string,
{
current: (msg: KernelMessage.IIOPubMessage) => boolean | PromiseLike<boolean>;
previous: ((msg: KernelMessage.IIOPubMessage) => boolean | PromiseLike<boolean>)[];
}
>();
private readonly lastHookedMessageId: string[] = [];
private _options: KernelSocketOptions;
// Messages that are awaiting extension messages to be fully handled
private awaitingExtensionMessage: Map<string, Deferred<void>>;
Expand Down Expand Up @@ -169,7 +175,6 @@ class ProxyKernel implements IMessageHandler, Kernel.IKernelConnection {
postOffice.addHandler(this);
this.websocket = proxySocketInstance;
this.messageHook = this.messageHookInterceptor.bind(this);
this.messageHooks = new Map<string, (msg: KernelMessage.IIOPubMessage) => boolean | PromiseLike<boolean>>();
this.fakeOpenSocket();
}

Expand Down Expand Up @@ -343,7 +348,14 @@ class ProxyKernel implements IMessageHandler, Kernel.IKernelConnection {
this.postOffice.sendMessage<IInteractiveWindowMapping>(IPyWidgetMessages.IPyWidgets_RegisterMessageHook, msgId);

// Save the real hook so we can call it
this.messageHooks.set(msgId, hook);
const item = this.messageHooks.get(msgId);
if (item) {
// Preserve the previous hook and setup a new hook for the same comm msg.
item.previous.push(item.current);
item.current = hook;
} else {
this.messageHooks.set(msgId, { current: hook, previous: [] });
}

// Wrap the hook and send it to the real kernel
this.realKernel.registerMessageHook(msgId, this.messageHook);
Expand All @@ -363,15 +375,20 @@ class ProxyKernel implements IMessageHandler, Kernel.IKernelConnection {

this.postOffice.sendMessage<IInteractiveWindowMapping>(IPyWidgetMessages.IPyWidgets_RemoveMessageHook, {
hookMsgId: msgId,
lastHookedMsgId: this.lastHookedMessageId
lastHookedMsgId: this.lastHookedMessageId.length ? this.lastHookedMessageId.pop() : undefined
});

// Remove our mapping
this.messageHooks.delete(msgId);
this.lastHookedMessageId = undefined;

// Remove from the real kernel
this.realKernel.removeMessageHook(msgId, this.messageHook);
const item = this.messageHooks.get(msgId);
if (item) {
if (item.previous.length > 0) {
item.current = item.previous.pop()!;
} else {
this.messageHooks.delete(msgId);
// Remove from the real kernel
this.realKernel.removeMessageHook(msgId, this.messageHook);
}
}
}

// Called when the extension has finished an operation that we are waiting for in message processing
Expand Down Expand Up @@ -415,12 +432,12 @@ class ProxyKernel implements IMessageHandler, Kernel.IKernelConnection {
try {
// Save the active message that is currently being hooked. The Extension
// side needs this information during removeMessageHook so it can delay removal until after a message is called
this.lastHookedMessageId = msg.header.msg_id;
this.lastHookedMessageId.push(msg.header.msg_id);

const hook = this.messageHooks.get((msg.parent_header as any).msg_id);
if (hook) {
// When the kernel calls the hook, save the result for this message. The other side will ask for it
const result = hook(msg);
const result = hook.current(msg);
this.hookResults.set(msg.header.msg_id, result);
if ((result as any).then) {
return (result as any).then((r: boolean) => {
Expand Down
Loading

0 comments on commit c06e165

Please sign in to comment.