refactor: Improve table saver to always use the correct service worker (

#1515) Fixes #766 BREAKING CHANGE: `TableSaver` now expects the service worker to send it a complete URL for download instead of just a file name. DHE will need to adjust its `serviceWorker.js` to incorporate the same changes from this PR.
deephaven · Sep 14, 2023 · 2488e52 · 2488e52
1 parent 23ab5cc
commit 2488e52
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 44 deletions.
diff --git a/packages/code-studio/public/download/serviceWorker.js b/packages/code-studio/public/download/serviceWorker.js
@@ -86,12 +86,12 @@ self.onmessage = event => {
       `service worker setting ${encodedFileName} download stream on service worker`
     );
     const rs = createReadableStream(ports[0]);
-    const downloadUrl = `${self.registration.scope}${encodedFileName}`;
+    const downloadUrl = new URL(encodedFileName, self.registration.scope).href;
     tableExportMap.set(downloadUrl, {
       readableStream: rs,
       port: ports[0],
       encodedFileName,
     });
-    ports[0].postMessage({ download: encodedFileName });
+    ports[0].postMessage({ download: downloadUrl });
   }
 };
diff --git a/packages/code-studio/src/DownloadServiceWorkerUtils.ts b/packages/code-studio/src/DownloadServiceWorkerUtils.ts
@@ -3,7 +3,12 @@ import Log from '@deephaven/log';
 const log = Log.module('DownloadServiceWorkerUtils');
 
 class DownloadServiceWorkerUtils {
-  static DOWNLOAD_PATH = '/download/';
+  static SERVICE_WORKER_URL = new URL(
+    `./download/serviceWorker.js`,
+    document.baseURI
+  );
+
+  static serviceWorkerRegistration: ServiceWorkerRegistration | null = null;
 
   static registerOnLoaded(): void {
     const publicUrl = new URL(import.meta.env.BASE_URL, window.location.href);
@@ -15,33 +20,28 @@ class DownloadServiceWorkerUtils {
     }
 
     if ('serviceWorker' in navigator) {
-      window.addEventListener('load', () => {
-        const swUrl = new URL(
-          `${import.meta.env.BASE_URL ?? ''}download/serviceWorker.js`,
-          window.location.href
-        );
-
-        navigator.serviceWorker
-          .register(swUrl)
-          .then(reg => {
-            reg.update();
-            log.info('Registering service worker on ', swUrl, reg);
-          })
-          .catch(err => {
-            log.error('Failed to register service worker', err);
-          });
-      });
+      navigator.serviceWorker
+        .register(DownloadServiceWorkerUtils.SERVICE_WORKER_URL)
+        .then(reg => {
+          reg.update();
+          DownloadServiceWorkerUtils.serviceWorkerRegistration = reg;
+          log.info(
+            'Registering service worker on ',
+            DownloadServiceWorkerUtils.SERVICE_WORKER_URL,
+            reg
+          );
+        })
+        .catch(err => {
+          log.error('Failed to register service worker', err);
+        });
     } else {
       log.info('Service worker is not supported.');
     }
   }
 
   static async getServiceWorker(): Promise<ServiceWorker> {
     if ('serviceWorker' in navigator) {
-      const regs = await navigator.serviceWorker.getRegistrations();
-      const swReg = regs.find(reg =>
-        reg.scope.endsWith(DownloadServiceWorkerUtils.DOWNLOAD_PATH)
-      );
+      const swReg = DownloadServiceWorkerUtils.serviceWorkerRegistration;
       if (swReg && swReg.active) {
         log.info('Download service worker is active.');
         return swReg.active;
@@ -51,8 +51,8 @@ class DownloadServiceWorkerUtils {
     throw new Error('Download service worker is not available.');
   }
 
-  static unregisterSW(): undefined {
-    return undefined;
+  static unregisterSW(): void {
+    DownloadServiceWorkerUtils.serviceWorkerRegistration?.unregister();
   }
 }
 export default DownloadServiceWorkerUtils;
diff --git a/packages/iris-grid/src/sidebar/TableSaver.tsx b/packages/iris-grid/src/sidebar/TableSaver.tsx
@@ -102,8 +102,6 @@ export default class TableSaver extends PureComponent<
     // If the stream doesn't pull for long enough time, chances are the stream is already canceled, so we stop the stream.
     // Issue ticket on Chromium: https://bugs.chromium.org/p/chromium/issues/detail?id=638494
 
-    this.iframes = [];
-
     this.useBlobFallback = false;
   }
 
@@ -123,11 +121,7 @@ export default class TableSaver extends PureComponent<
   }
 
   componentWillUnmount(): void {
-    if (this.iframes.length > 0) {
-      this.iframes.forEach(iframe => {
-        iframe.remove();
-      });
-    }
+    this.removeIframe();
     if (this.streamTimeout) clearTimeout(this.streamTimeout);
     if (this.snapshotHandlerTimeout) clearTimeout(this.snapshotHandlerTimeout);
   }
@@ -182,7 +176,7 @@ export default class TableSaver extends PureComponent<
 
   downloadStartTime?: number;
 
-  iframes: HTMLIFrameElement[];
+  iframe?: HTMLIFrameElement;
 
   useBlobFallback: boolean;
 
@@ -325,15 +319,9 @@ export default class TableSaver extends PureComponent<
   }
 
   cancelDownload(): void {
-    if (this.table) {
-      this.table.close();
-    }
-    if (this.tableSubscription) {
-      this.tableSubscription.close();
-    }
-    if (this.fileWriter) {
-      this.fileWriter.abort();
-    }
+    this.table?.close();
+    this.tableSubscription?.close();
+    this.fileWriter?.abort();
 
     this.cancelableSnapshots.forEach(cancelable => {
       if (cancelable) {
@@ -351,6 +339,7 @@ export default class TableSaver extends PureComponent<
     this.tableSubscription = undefined;
     this.columns = undefined;
     this.chunkRows = undefined;
+    this.removeIframe();
 
     this.gridRanges = [];
     this.gridRangeCounter = 0;
@@ -687,10 +676,15 @@ export default class TableSaver extends PureComponent<
     // make a return value and make it static method
     const iframe = document.createElement('iframe');
     iframe.hidden = true;
-    iframe.src = `download/${src}`;
+    iframe.src = src;
     iframe.name = 'iframe';
     document.body.appendChild(iframe);
-    this.iframes.push(iframe);
+    this.iframe = iframe;
+  }
+
+  removeIframe(): void {
+    this.iframe?.remove();
+    this.iframe = undefined;
   }
 
   render(): null {