Tweaks (#46)

* Change listCommands return to a promise

* Send message when ready

* Enable returns from execute

* Add a `ready` promise (#47)

* ready promise

* custom proxy

* update demo

* lint

* Update Playwright Snapshots

* Save video

* Update snapshot names

* lint

* Add structuredClone check

* Change viewport size for tests

* Update tests

---------

Co-authored-by: Greg <[email protected]>
Co-authored-by: Jeremy Tuloup <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Author: 3 people

demo/index.html

@@ -53,12 +53,15 @@ <h1>%VITE_TITLE% Demo</h1>
       </div>
     </div>
 
-    <iframe
-      id="jupyterlab"
-      src="%VITE_DEMO_SRC%"
-      sandbox="allow-scripts allow-same-origin"
-      title="JupyterLab Instance"
-      loading="lazy"
-    ></iframe>
+    <div class="iframe-container">
+      <div id="bridge-status" class="status-indicator">Connecting to JupyterLab...</div>
+      <iframe
+        id="jupyterlab"
+        src="%VITE_DEMO_SRC%"
+        sandbox="allow-scripts allow-same-origin"
+        title="JupyterLab Instance"
+        loading="lazy"
+      ></iframe>
+    </div>
   </body>
 </html>

demo/src/main.js

@@ -4,7 +4,20 @@ import { createBridge } from 'jupyter-iframe-commands-host';
 
 const commandBridge = createBridge({ iframeId: 'jupyterlab' });
 
+const statusIndicator = document.getElementById('bridge-status');
+statusIndicator.style.backgroundColor = '#ffa500'; // Orange for connecting
+
+let bridgeReady = false;
+
 const submitCommand = async (command, args) => {
+  // Don't allow command execution until bridge is ready
+  if (!bridgeReady) {
+    document.getElementById('error-dialog').innerHTML =
+      '<code>Command bridge is not ready yet. Please wait.</code>';
+    errorDialog.showModal();
+    return;
+  }
+
   try {
     await commandBridge.execute(command, args ? JSON.parse(args) : {});
   } catch (e) {
@@ -157,3 +170,10 @@ modeRadios.forEach(radio => {
     iframe.src = currentUrl.toString();
   });
 });
+
+// Wait for the command bridge to be ready
+commandBridge.ready.then(() => {
+  bridgeReady = true;
+  statusIndicator.textContent = 'Connected to JupyterLab';
+  statusIndicator.style.backgroundColor = '#32CD32'; // Green for connected
+});

demo/src/style.css

@@ -258,3 +258,22 @@ dialog button[value='close']:hover {
   font-weight: 500;
   user-select: none;
 }
+
+.status-indicator {
+  padding: 5px 10px;
+  border-radius: 4px;
+  color: #fff;
+  font-size: 14px;
+  font-weight: bold;
+  margin-bottom: 10px;
+  display: inline-block;
+  box-shadow: 0 2px 4px rgba(0, 0, 0, 0.2);
+}
+
+.iframe-container {
+  position: relative;
+  width: 100%;
+  display: flex;
+  flex-direction: column;
+  align-items: center;
+}

packages/extension/src/index.ts

@@ -45,15 +45,29 @@ const plugin: JupyterFrontEndPlugin<void> = {
 
     const api: ICommandBridgeRemote = {
       async execute(command: string, args: ReadonlyPartialJSONObject) {
-        await commands.execute(command, args);
+        const result = await commands.execute(command, args);
+        try {
+          // Attempt to clone result to check if it's serializable
+          const clone = structuredClone(result);
+          return clone;
+        } catch (error) {
+          // Non-serializable values can't be sent over Comlink
+          void error;
+        }
       },
-      listCommands() {
+      async listCommands() {
         return commands.listCommands();
+      },
+      get ready() {
+        return app.started;
       }
     };
 
-    const endpoint = windowEndpoint(self.parent);
-    expose(api, endpoint);
+    app.started.then(() => {
+      const endpoint = windowEndpoint(self.parent);
+      expose(api, endpoint);
+      window.parent?.postMessage('extension-loaded', '*');
+    });
   }
 };
 

packages/extension/src/interface.ts

@@ -12,12 +12,17 @@ export interface ICommandBridgeRemote {
    * @param command - The name of the command to execute.
    * @param args - An object containing arguments for the command.
    */
-  execute(command: string, args: ReadonlyPartialJSONObject): void;
+  execute(command: string, args: ReadonlyPartialJSONObject): Promise<unknown>;
 
   /**
    * Lists all available commands.
    *
    * @returns An array of strings representing the names of all available commands.
    */
-  listCommands(): string[];
+  listCommands(): Promise<string[]>;
+
+  /**
+   * Waits for the JupyterLab environment to be ready.
+   */
+  ready: Promise<void>;
 }

packages/host/src/index.ts

@@ -3,10 +3,15 @@
 import { windowEndpoint, wrap, proxy, ProxyOrClone } from 'comlink';
 
 import { ICommandBridgeRemote } from 'jupyter-iframe-commands';
+
 /**
  * A bridge to expose actions on JupyterLab commands.
  */
-export function createBridge({ iframeId }: { iframeId: string }) {
+export function createBridge({
+  iframeId
+}: {
+  iframeId: string;
+}): ICommandBridgeRemote {
   const iframe = document.getElementById(iframeId) as HTMLIFrameElement;
 
   if (!iframe) {
@@ -21,7 +26,46 @@ export function createBridge({ iframeId }: { iframeId: string }) {
     );
   }
 
-  return wrap<ICommandBridgeRemote>(windowEndpoint(iframe.contentWindow));
+  const wrappedBridge = wrap<ICommandBridgeRemote>(
+    windowEndpoint(iframe.contentWindow)
+  );
+
+  // Create a promise that resolves when the iframe signals it's ready
+  const readyPromise = new Promise<void>(resolve => {
+    const controller = new AbortController();
+    const signal = controller.signal;
+
+    const messageHandler = (event: MessageEvent) => {
+      if (event.data === 'extension-loaded') {
+        controller.abort();
+        resolve();
+      }
+    };
+
+    window.addEventListener('message', messageHandler, { signal });
+
+    wrappedBridge.ready
+      .then(() => {
+        controller.abort();
+        resolve();
+      })
+      .catch(() => {
+        // If ready() fails, we'll still wait for the message
+        // No need to do anything, the message listener is still active
+      });
+  });
+
+  // Create a proxy that intercepts property access
+  return new Proxy(wrappedBridge, {
+    get(target, prop, receiver) {
+      // If the property is 'ready', return the ready promise
+      if (prop === 'ready') {
+        return readyPromise;
+      }
+      // Otherwise delegate to the comlink wrapped bridge
+      return Reflect.get(target, prop, receiver);
+    }
+  });
 }
 
 /**

ui-tests/playwright.config.js

@@ -7,7 +7,8 @@ module.exports = {
   ...baseConfig,
   retries: process.env.CI ? 2 : 0,
   use: {
-    trace: 'on-first-retry'
+    trace: 'on-first-retry',
+    video: 'retain-on-failure'
   },
   webServer: [
     {

ui-tests/tests/hostpage-tests.spec.ts

@@ -1,4 +1,4 @@
-import { test, expect, Page } from '@playwright/test';
+import { expect, Page, test } from '@playwright/test';
 
 const waitForApp = async (page: Page) => {
   const iframe = page.locator('#jupyterlab').contentFrame();
@@ -11,8 +11,8 @@ const waitForApp = async (page: Page) => {
 test.use({
   baseURL: 'http://localhost:8080',
   viewport: {
-    width: 1024,
-    height: 768
+    width: 1280,
+    height: 1024
   }
 });
 /**
@@ -22,36 +22,29 @@ test.describe('Commands from host should affect lab in iframe', () => {
   test.beforeEach(async ({ page }) => {
     await page.goto('index.html');
 
-    const iframe = page.locator('#jupyterlab').contentFrame();
+    const iframe = page
+      .locator('iframe[title="JupyterLab Instance"]')
+      .contentFrame();
 
     await iframe.locator('.jp-LauncherCard-icon').first().waitFor();
 
-    // Make sure left sidebar is hidden
-    await iframe.getByText('View', { exact: true }).click();
-    await iframe.getByText('Appearance').hover();
-    await iframe
-      .locator('#jp-mainmenu-view-appearance')
-      .getByText('Show Left Sidebar')
-      .waitFor();
-
     const leftSidebarOpen = await iframe
-      .getByRole('menuitem', { name: 'Show Left Sidebar Ctrl+B' })
-      .getByRole('img')
+      .getByRole('button', { name: 'New Folder' })
       .isVisible();
 
     if (leftSidebarOpen) {
-      await iframe
-        .locator('#jp-mainmenu-view-appearance')
-        .getByText('Show Left Sidebar')
-        .click();
+      await iframe.getByText('View', { exact: true }).click();
+      await iframe.getByText('Appearance').hover();
+      await iframe.getByText('Show Left Sidebar').waitFor();
+      await iframe.getByText('Show Left Sidebar').click();
     }
 
     await iframe.locator('#jp-MainLogo').click();
 
     await waitForApp(page);
   });
 
-  test('Swich to light theme', async ({ page }) => {
+  test('Switch to light theme', async ({ page }) => {
     await page
       .getByPlaceholder('Enter a command')
       .fill('apputils:change-theme');
@@ -62,10 +55,12 @@ test.describe('Commands from host should affect lab in iframe', () => {
 
     await waitForApp(page);
 
-    await expect(page).toHaveScreenshot('light-theme.png', { timeout: 1500 });
+    await expect(page).toHaveScreenshot('light-theme.png', {
+      timeout: 1500
+    });
   });
 
-  test('Swich to dark theme', async ({ page }) => {
+  test('Switch to dark theme', async ({ page }) => {
     await page
       .getByPlaceholder('Enter a command')
       .fill('apputils:change-theme');
@@ -76,7 +71,9 @@ test.describe('Commands from host should affect lab in iframe', () => {
 
     await waitForApp(page);
 
-    await expect(page).toHaveScreenshot('dark-theme.png', { timeout: 1500 });
+    await expect(page).toHaveScreenshot('dark-theme.png', {
+      timeout: 1500
+    });
   });
 
   test('Open a new notebook', async ({ page }) => {