Skip to content

Commit 2488e52

Browse files
authored
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.
1 parent 23ab5cc commit 2488e52

3 files changed

Lines changed: 38 additions & 44 deletions

File tree

packages/code-studio/public/download/serviceWorker.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,12 @@ self.onmessage = event => {
8686
`service worker setting ${encodedFileName} download stream on service worker`
8787
);
8888
const rs = createReadableStream(ports[0]);
89-
const downloadUrl = `${self.registration.scope}${encodedFileName}`;
89+
const downloadUrl = new URL(encodedFileName, self.registration.scope).href;
9090
tableExportMap.set(downloadUrl, {
9191
readableStream: rs,
9292
port: ports[0],
9393
encodedFileName,
9494
});
95-
ports[0].postMessage({ download: encodedFileName });
95+
ports[0].postMessage({ download: downloadUrl });
9696
}
9797
};

packages/code-studio/src/DownloadServiceWorkerUtils.ts

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,12 @@ import Log from '@deephaven/log';
33
const log = Log.module('DownloadServiceWorkerUtils');
44

55
class DownloadServiceWorkerUtils {
6-
static DOWNLOAD_PATH = '/download/';
6+
static SERVICE_WORKER_URL = new URL(
7+
`./download/serviceWorker.js`,
8+
document.baseURI
9+
);
10+
11+
static serviceWorkerRegistration: ServiceWorkerRegistration | null = null;
712

813
static registerOnLoaded(): void {
914
const publicUrl = new URL(import.meta.env.BASE_URL, window.location.href);
@@ -15,33 +20,28 @@ class DownloadServiceWorkerUtils {
1520
}
1621

1722
if ('serviceWorker' in navigator) {
18-
window.addEventListener('load', () => {
19-
const swUrl = new URL(
20-
`${import.meta.env.BASE_URL ?? ''}download/serviceWorker.js`,
21-
window.location.href
22-
);
23-
24-
navigator.serviceWorker
25-
.register(swUrl)
26-
.then(reg => {
27-
reg.update();
28-
log.info('Registering service worker on ', swUrl, reg);
29-
})
30-
.catch(err => {
31-
log.error('Failed to register service worker', err);
32-
});
33-
});
23+
navigator.serviceWorker
24+
.register(DownloadServiceWorkerUtils.SERVICE_WORKER_URL)
25+
.then(reg => {
26+
reg.update();
27+
DownloadServiceWorkerUtils.serviceWorkerRegistration = reg;
28+
log.info(
29+
'Registering service worker on ',
30+
DownloadServiceWorkerUtils.SERVICE_WORKER_URL,
31+
reg
32+
);
33+
})
34+
.catch(err => {
35+
log.error('Failed to register service worker', err);
36+
});
3437
} else {
3538
log.info('Service worker is not supported.');
3639
}
3740
}
3841

3942
static async getServiceWorker(): Promise<ServiceWorker> {
4043
if ('serviceWorker' in navigator) {
41-
const regs = await navigator.serviceWorker.getRegistrations();
42-
const swReg = regs.find(reg =>
43-
reg.scope.endsWith(DownloadServiceWorkerUtils.DOWNLOAD_PATH)
44-
);
44+
const swReg = DownloadServiceWorkerUtils.serviceWorkerRegistration;
4545
if (swReg && swReg.active) {
4646
log.info('Download service worker is active.');
4747
return swReg.active;
@@ -51,8 +51,8 @@ class DownloadServiceWorkerUtils {
5151
throw new Error('Download service worker is not available.');
5252
}
5353

54-
static unregisterSW(): undefined {
55-
return undefined;
54+
static unregisterSW(): void {
55+
DownloadServiceWorkerUtils.serviceWorkerRegistration?.unregister();
5656
}
5757
}
5858
export default DownloadServiceWorkerUtils;

packages/iris-grid/src/sidebar/TableSaver.tsx

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,6 @@ export default class TableSaver extends PureComponent<
102102
// If the stream doesn't pull for long enough time, chances are the stream is already canceled, so we stop the stream.
103103
// Issue ticket on Chromium: https://bugs.chromium.org/p/chromium/issues/detail?id=638494
104104

105-
this.iframes = [];
106-
107105
this.useBlobFallback = false;
108106
}
109107

@@ -123,11 +121,7 @@ export default class TableSaver extends PureComponent<
123121
}
124122

125123
componentWillUnmount(): void {
126-
if (this.iframes.length > 0) {
127-
this.iframes.forEach(iframe => {
128-
iframe.remove();
129-
});
130-
}
124+
this.removeIframe();
131125
if (this.streamTimeout) clearTimeout(this.streamTimeout);
132126
if (this.snapshotHandlerTimeout) clearTimeout(this.snapshotHandlerTimeout);
133127
}
@@ -182,7 +176,7 @@ export default class TableSaver extends PureComponent<
182176

183177
downloadStartTime?: number;
184178

185-
iframes: HTMLIFrameElement[];
179+
iframe?: HTMLIFrameElement;
186180

187181
useBlobFallback: boolean;
188182

@@ -325,15 +319,9 @@ export default class TableSaver extends PureComponent<
325319
}
326320

327321
cancelDownload(): void {
328-
if (this.table) {
329-
this.table.close();
330-
}
331-
if (this.tableSubscription) {
332-
this.tableSubscription.close();
333-
}
334-
if (this.fileWriter) {
335-
this.fileWriter.abort();
336-
}
322+
this.table?.close();
323+
this.tableSubscription?.close();
324+
this.fileWriter?.abort();
337325

338326
this.cancelableSnapshots.forEach(cancelable => {
339327
if (cancelable) {
@@ -351,6 +339,7 @@ export default class TableSaver extends PureComponent<
351339
this.tableSubscription = undefined;
352340
this.columns = undefined;
353341
this.chunkRows = undefined;
342+
this.removeIframe();
354343

355344
this.gridRanges = [];
356345
this.gridRangeCounter = 0;
@@ -687,10 +676,15 @@ export default class TableSaver extends PureComponent<
687676
// make a return value and make it static method
688677
const iframe = document.createElement('iframe');
689678
iframe.hidden = true;
690-
iframe.src = `download/${src}`;
679+
iframe.src = src;
691680
iframe.name = 'iframe';
692681
document.body.appendChild(iframe);
693-
this.iframes.push(iframe);
682+
this.iframe = iframe;
683+
}
684+
685+
removeIframe(): void {
686+
this.iframe?.remove();
687+
this.iframe = undefined;
694688
}
695689

696690
render(): null {

0 commit comments

Comments
 (0)