Support Update Display Data (#10874)

* Bunch of logging and getting azureml debug bits * Get rid of second view * Implement update_display_data * Add news entry * Remove unnecessary jquery * Remove stylesheet from html * Add test * Add execute_result for update_display_data handling * Fix dependencies and only pull in the type from jupyterlab services * Fix importing jupyterlab services * Prettier format * Fix prettier again * Fix hygiene * Potential fix for functional failures * More hygiene issues * Fix functional tests * Fix unit tests
microsoft · Mar 31, 2020 · 7a58ce8 · 7a58ce8
1 parent 69ac30d
commit 7a58ce8
Show file tree

Hide file tree

Showing 23 changed files with 1,268 additions and 1,103 deletions.
diff --git a/.prettierrc.js b/.prettierrc.js
@@ -2,6 +2,7 @@ module.exports = {
     singleQuote: true,
     printWidth: 120,
     tabWidth: 4,
+    endOfLine: 'auto',
     trailingComma: 'none',
     overrides: [
         {

diff --git a/news/2 Fixes/10873.md b/news/2 Fixes/10873.md
@@ -0,0 +1 @@
+Handle display.update inside of cells.
diff --git a/package.datascience-ui.dependencies.json b/package.datascience-ui.dependencies.json
@@ -34,6 +34,7 @@
     "bail",
     "base16",
     "bintrees",
+    "bootstrap",
     "bootstrap-less",
     "base64-js",
     "create-react-context",

diff --git a/src/client/common/application/webPanels/webPanel.ts b/src/client/common/application/webPanels/webPanel.ts
@@ -159,9 +159,9 @@ export class WebPanel implements IWebPanel {
                 }; default-src 'unsafe-inline' 'unsafe-eval' vscode-resource: data: https: http: blob:;">
                 <meta name="theme-color" content="#000000">
                 <meta name="theme" content="${Identifiers.GeneratedThemeName}"/>
-                <title>React App</title>
+                <title>VS Code Python React UI</title>
                 <base href="${uriBase}${uriBase.endsWith('/') ? '' : '/'}"/>
-            </head>
+                </head>
             <body>
                 <noscript>You need to enable JavaScript to run this app.</noscript>
                 <div id="root"></div>
@@ -197,7 +197,7 @@ export class WebPanel implements IWebPanel {
                 <meta http-equiv="Content-Security-Policy" content="img-src 'self' data: https: http: blob:; default-src 'unsafe-inline' 'unsafe-eval' vscode-resource: data: https: http: blob:;">
                 <meta name="theme-color" content="#000000">
                 <meta name="theme" content="${Identifiers.GeneratedThemeName}"/>
-                <title>React App</title>
+                <title>VS Code Python React UI</title>
                 <base href="${uriBase}"/>
             </head>
             <body>

diff --git a/src/client/common/application/webPanels/webPanelServer.ts b/src/client/common/application/webPanels/webPanelServer.ts
@@ -142,7 +142,7 @@ export class WebPanelServer {
                 <meta http-equiv="Content-Security-Policy" content="img-src 'self' data: https: http: blob:; default-src 'unsafe-inline' 'unsafe-eval' vscode-resource: data: https: http: blob:;">
                 <meta name="theme-color" content="#000000">
                 <meta name="theme" content="${Identifiers.GeneratedThemeName}"/>
-                <title>React App</title>
+                <title>VS Code Python React UI</title>
             </head>
             <body class='${query.baseTheme}'>
                 <noscript>You need to enable JavaScript to run this app.</noscript>

diff --git a/src/client/datascience/interactive-common/interactiveBase.ts b/src/client/datascience/interactive-common/interactiveBase.ts
@@ -4,6 +4,7 @@
 import '../../common/extensions';
 
 import { nbformat } from '@jupyterlab/coreutils';
+import type { KernelMessage } from '@jupyterlab/services';
 import { injectable, unmanaged } from 'inversify';
 import * as os from 'os';
 import * as path from 'path';
@@ -1068,7 +1069,7 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
         return localizedUri;
     }
 
-    private async listenToNotebookStatus(notebook: INotebook): Promise<void> {
+    private async listenToNotebookEvents(notebook: INotebook): Promise<void> {
         const statusChangeHandler = async (status: ServerStatus) => {
             const kernelSpec = notebook.getKernelSpec();
 
@@ -1087,6 +1088,9 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
 
         // Fire the status changed handler at least once (might have already been running and so won't show a status update)
         statusChangeHandler(notebook.status).ignoreErrors();
+
+        // Also listen to iopub messages so we can update other cells on update_display_data
+        notebook.registerIOPubListener(this.handleKernelMessage.bind(this));
     }
 
     private async ensureNotebookImpl(server: INotebookServer): Promise<void> {
@@ -1108,7 +1112,7 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
                     this._notebook.identity.toString()
                 ).ignoreErrors();
 
-                return this.listenToNotebookStatus(this._notebook);
+                return this.listenToNotebookEvents(this._notebook);
             }
         }
     }
@@ -1400,4 +1404,18 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
             commands.executeCommand('workbench.action.openSettings');
         }
     }
+
+    private async handleKernelMessage(msg: KernelMessage.IIOPubMessage, _requestId: string) {
+        // Only care about one sort of message, UpdateDisplayData
+        // tslint:disable-next-line: no-require-imports
+        const jupyterLab = require('@jupyterlab/services') as typeof import('@jupyterlab/services'); // NOSONAR
+        if (jupyterLab.KernelMessage.isUpdateDisplayDataMsg(msg)) {
+            return this.handleUpdateDisplayData(msg as KernelMessage.IUpdateDisplayDataMsg);
+        }
+    }
+
+    private async handleUpdateDisplayData(msg: KernelMessage.IUpdateDisplayDataMsg) {
+        // Send to the UI to handle
+        return this.postMessage(InteractiveWindowMessages.UpdateDisplayData, msg).ignoreErrors();
+    }
 }
diff --git a/src/client/datascience/interactive-common/interactiveWindowTypes.ts b/src/client/datascience/interactive-common/interactiveWindowTypes.ts
@@ -100,7 +100,8 @@ export enum InteractiveWindowMessages {
     SelectJupyterServer = 'select_jupyter_server',
     UpdateModel = 'update_model',
     ReceivedUpdateModel = 'received_update_model',
-    OpenSettings = 'open_settings'
+    OpenSettings = 'open_settings',
+    UpdateDisplayData = 'update_display_data'
 }
 
 export enum IPyWidgetMessages {
@@ -604,4 +605,5 @@ export class IInteractiveWindowMapping {
     public [InteractiveWindowMessages.ReceivedUpdateModel]: never | undefined;
     public [SharedMessages.UpdateSettings]: string;
     public [SharedMessages.LocInit]: string;
+    public [InteractiveWindowMessages.UpdateDisplayData]: KernelMessage.IUpdateDisplayDataMsg;
 }
diff --git a/src/client/datascience/interactive-common/synchronization.ts b/src/client/datascience/interactive-common/synchronization.ts
@@ -165,6 +165,7 @@ const messageWithMessageTypes: MessageMapping<IInteractiveWindowMapping> & Messa
     [InteractiveWindowMessages.UpdateCell]: MessageType.syncAcrossSameNotebooks | MessageType.syncWithLiveShare,
     [InteractiveWindowMessages.UpdateModel]: MessageType.syncAcrossSameNotebooks | MessageType.syncWithLiveShare,
     [InteractiveWindowMessages.UpdateKernel]: MessageType.syncAcrossSameNotebooks | MessageType.syncWithLiveShare,
+    [InteractiveWindowMessages.UpdateDisplayData]: MessageType.syncWithLiveShare,
     [InteractiveWindowMessages.VariableExplorerToggle]: MessageType.other,
     [InteractiveWindowMessages.VariablesComplete]: MessageType.other,
     // Types from CssMessages

diff --git a/src/client/datascience/ipywidgets/ipyWidgetMessageDispatcher.ts b/src/client/datascience/ipywidgets/ipyWidgetMessageDispatcher.ts
@@ -4,8 +4,10 @@
 'use strict';
 
 import { Kernel, KernelMessage } from '@jupyterlab/services';
+import * as util from 'util';
 import * as uuid from 'uuid/v4';
 import { Event, EventEmitter, Uri } from 'vscode';
+import { traceInfo } from '../../common/logger';
 import { IDisposable } from '../../common/types';
 import { createDeferred, Deferred } from '../../common/utils/async';
 import { IInteractiveWindowMapping, IPyWidgetMessages } from '../interactive-common/interactiveWindowTypes';
@@ -45,6 +47,7 @@ export class IPyWidgetMessageDispatcher implements IIPyWidgetMessageDispatcher {
     }
 
     public receiveMessage(message: IPyWidgetMessage): void {
+        traceInfo(`IPyWidgetMessage: ${util.inspect(message)}`);
         switch (message.message) {
             case IPyWidgetMessages.IPyWidgets_ShellSend:
                 this.sendIPythonShellMsg(message.payload);

diff --git a/src/client/datascience/jupyter/jupyterNotebook.ts b/src/client/datascience/jupyter/jupyterNotebook.ts
@@ -830,7 +830,7 @@ export class JupyterNotebookBase implements INotebook {
                           allow_stdin: true, // Allow when silent too in case runStartupCommands asks for a password
                           store_history: !silent // Silent actually means don't output anything. Store_history is what affects execution_count
                       },
-                      true
+                      silent // Dispose only silent futures. Otherwise update_display_data doesn't finda future for a previous cell.
                   )
                 : undefined;
         } catch (exc) {
@@ -1183,6 +1183,8 @@ export class JupyterNotebookBase implements INotebook {
                 output_type: 'execute_result',
                 data: msg.content.data,
                 metadata: msg.content.metadata,
+                // tslint:disable-next-line: no-any
+                transient: msg.content.transient as any, // NOSONAR
                 execution_count: msg.content.execution_count
             },
             clearState
@@ -1284,19 +1286,15 @@ export class JupyterNotebookBase implements INotebook {
         const output: nbformat.IDisplayData = {
             output_type: 'display_data',
             data: msg.content.data,
-            metadata: msg.content.metadata
+            metadata: msg.content.metadata,
+            // tslint:disable-next-line: no-any
+            transient: msg.content.transient as any // NOSONAR
         };
         this.addToCellData(cell, output, clearState);
     }
 
-    private handleUpdateDisplayData(msg: KernelMessage.IUpdateDisplayDataMsg, _clearState: RefBool, cell: ICell) {
-        // Should already have a display data output in our cell.
-        const data: nbformat.ICodeCell = cell.data as nbformat.ICodeCell;
-        const output = data.outputs.find((o) => o.output_type === 'display_data');
-        if (output) {
-            output.data = msg.content.data;
-            output.metadata = msg.content.metadata;
-        }
+    private handleUpdateDisplayData(_msg: KernelMessage.IUpdateDisplayDataMsg, _clearState: RefBool, _cell: ICell) {
+        // Updates should be handled outside.
     }
 
     private handleClearOutput(msg: KernelMessage.IClearOutputMsg, clearState: RefBool, cell: ICell) {

diff --git a/src/datascience-ui/history-react/index.html b/src/datascience-ui/history-react/index.html
@@ -5,6 +5,7 @@
         <meta name="viewport" content="width=device-width,initial-scale=1,shrink-to-fit=no" />
         <meta name="theme-color" content="#000000" />
         <title>React App</title>
+        <link rel="stylesheet" href="https://stackpath.bootstrapcdn.com/bootstrap/4.4.1/css/bootstrap.min.css" integrity="sha384-Vkoo8x4CGsO3+Hhxv8T/Q5PaXtkKtu6ug5TOeNV6gBiFeWPGFN9MuhOf23Q9Ifjh" crossorigin="anonymous">
         <style id="default-styles">
             :root {
                 --background-color: #ffffff;

diff --git a/src/datascience-ui/history-react/redux/reducers/index.ts b/src/datascience-ui/history-react/redux/reducers/index.ts
@@ -62,5 +62,6 @@ export const reducerMap: Partial<IInteractiveActionMapping> = {
     [InteractiveWindowMessages.StopDebugging]: Execution.stopDebugging,
     [InteractiveWindowMessages.ScrollToCell]: Effects.scrollToCell,
     [InteractiveWindowMessages.UpdateKernel]: Kernel.updateStatus,
-    [SharedMessages.LocInit]: CommonEffects.handleLocInit
+    [SharedMessages.LocInit]: CommonEffects.handleLocInit,
+    [InteractiveWindowMessages.UpdateDisplayData]: CommonEffects.handleUpdateDisplayData
 };
diff --git a/src/datascience-ui/interactive-common/cellOutput.tsx b/src/datascience-ui/interactive-common/cellOutput.tsx
@@ -575,7 +575,7 @@ export class CellOutput extends React.Component<ICellOutputProps> {
                         );
                     }
                 }
-            } else if (mimetype.startsWith('application/scrapbook.scrap.')) {
+            } else if (!mimetype || mimetype.startsWith('application/scrapbook.scrap.')) {
                 // Silently skip rendering of these mime types, render an empty div so the user sees the cell was executed.
                 buffer.push(<div key={index}></div>);
             } else {

diff --git a/src/datascience-ui/interactive-common/redux/reducers/commonEffects.ts b/src/datascience-ui/interactive-common/redux/reducers/commonEffects.ts
@@ -1,14 +1,18 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 'use strict';
+import { nbformat } from '@jupyterlab/coreutils';
+import { KernelMessage } from '@jupyterlab/services';
 import { Identifiers } from '../../../../client/datascience/constants';
 import { InteractiveWindowMessages } from '../../../../client/datascience/interactive-common/interactiveWindowTypes';
 import { IGetCssResponse } from '../../../../client/datascience/messages';
 import { IGetMonacoThemeResponse } from '../../../../client/datascience/monacoMessages';
-import { IMainState } from '../../../interactive-common/mainState';
+import { ICell } from '../../../../client/datascience/types';
+import { ICellViewModel, IMainState } from '../../../interactive-common/mainState';
 import { Helpers } from '../../../interactive-common/redux/reducers/helpers';
 import { storeLocStrings } from '../../../react-common/locReactSide';
 import { postActionToExtension } from '../helpers';
+import { Transfer } from './transfer';
 import { CommonActionType, CommonReducerArg, IOpenSettingsAction } from './types';
 
 export namespace CommonEffects {
@@ -128,4 +132,73 @@ export namespace CommonEffects {
         postActionToExtension(arg, InteractiveWindowMessages.OpenSettings, arg.payload.data.setting);
         return arg.prevState;
     }
+
+    export function handleUpdateDisplayData(
+        arg: CommonReducerArg<CommonActionType, KernelMessage.IUpdateDisplayDataMsg>
+    ): IMainState {
+        const newCells: ICell[] = [];
+        const oldCells: ICell[] = [];
+
+        // Find any cells that have this display_id
+        const newVMs = arg.prevState.cellVMs.map((c: ICellViewModel) => {
+            if (c.cell.data.cell_type === 'code') {
+                let isMatch = false;
+                const data: nbformat.ICodeCell = c.cell.data as nbformat.ICodeCell;
+                const changedOutputs = data.outputs.map((o) => {
+                    if (
+                        (o.output_type === 'display_data' || o.output_type === 'execute_result') &&
+                        o.transient &&
+                        // tslint:disable-next-line: no-any
+                        (o.transient as any).display_id === arg.payload.data.content.transient.display_id
+                    ) {
+                        // Remember this as a match
+                        isMatch = true;
+
+                        // If the output has this display_id, update the output
+                        return {
+                            ...o,
+                            data: arg.payload.data.content.data,
+                            metadata: arg.payload.data.content.metadata
+                        };
+                    } else {
+                        return o;
+                    }
+                });
+
+                // Save in our new cell list so we can tell the extension
+                // about our update
+                const newCell = isMatch
+                    ? Helpers.asCell({
+                          ...c.cell,
+                          data: {
+                              ...c.cell.data,
+                              outputs: changedOutputs
+                          }
+                      })
+                    : c.cell;
+                if (isMatch) {
+                    newCells.push(newCell);
+                } else {
+                    oldCells.push(newCell);
+                }
+                return Helpers.asCellViewModel({
+                    ...c,
+                    cell: newCell
+                });
+            } else {
+                oldCells.push(c.cell);
+                return c;
+            }
+        });
+
+        // If we found the display id, then an update happened. Tell the model about it
+        if (newCells.length) {
+            Transfer.postModelCellUpdate(arg, newCells, oldCells);
+        }
+
+        return {
+            ...arg.prevState,
+            cellVMs: newVMs
+        };
+    }
 }
diff --git a/src/datascience-ui/interactive-common/redux/reducers/transfer.ts b/src/datascience-ui/interactive-common/redux/reducers/transfer.ts
@@ -190,6 +190,21 @@ export namespace Transfer {
         });
     }
 
+    export function postModelCellUpdate<T>(
+        arg: CommonReducerArg<CommonActionType, T>,
+        newCells: ICell[],
+        oldCells: ICell[]
+    ) {
+        postModelUpdate(arg, {
+            source: 'user',
+            kind: 'modify',
+            newCells,
+            oldCells,
+            oldDirty: arg.prevState.dirty,
+            newDirty: true
+        });
+    }
+
     export function postModelRemoveAll<T>(arg: CommonReducerArg<CommonActionType, T>, newCellId: string) {
         postModelUpdate(arg, {
             source: 'user',

diff --git a/src/datascience-ui/ipywidgets/container.tsx b/src/datascience-ui/ipywidgets/container.tsx
@@ -9,6 +9,8 @@ import { AllowedIPyWidgetMessages } from '../interactive-common/redux/postOffice
 import { PostOffice } from '../react-common/postOffice';
 import { WidgetManager } from './manager';
 
+import 'bootstrap/dist/css/bootstrap.css';
+
 type Props = {
     postOffice: PostOffice;
     widgetContainerId: string;