Fix download task cleanup & browser compatibility

Improve types

Author: vdsabev

USERGUIDE

@@ -22,9 +22,10 @@ We're looking into adding such capabilities to Image Downloader in the future. I
 
 📝 Change Log
 ━━━━━━━
-3.3.0:
+3.3.1:
 - Disable slider handles when the checkbox is unchecked
 - Add tooltips to slider handles, slider checkboxes, and the Download button
+- Fix some issues with downloads not being cleaned up
 
 3.2.3:
 - Download in the background - you can now close the popup and the images will keep downloading!

manifest.json

@@ -1,7 +1,7 @@
 {
   "name": "Image Downloader",
   "description": "Browse and download images on the web",
-  "version": "3.3.0",
+  "version": "3.3.1",
   "minimum_chrome_version": "72",
   "manifest_version": 2,
   "icons": {

package-lock.json

@@ -1,7 +1,7 @@
 {
   "private": true,
   "name": "image-downloader",
-  "version": "3.3.0",
+  "version": "3.3.1",
   "license": "MIT",
   "scripts": {
     "start": "npm run build && node scripts/watch.js",

package.json

@@ -191,7 +191,11 @@ const useSlider = (dimension, options, setOptions) => {
       }));
     });
 
-    return () => slider.noUiSlider?.destroy();
+    return () => {
+      if (slider.noUiSlider) {
+        slider.noUiSlider.destroy();
+      }
+    };
   }, []);
 
   useEffect(() => {
@@ -212,12 +216,18 @@ const useSlider = (dimension, options, setOptions) => {
   ]);
 
   useDisableSliderHandle(
-    () => sliderRef.current?.querySelectorAll('.noUi-origin')[0],
+    () =>
+      sliderRef.current
+        ? sliderRef.current.querySelectorAll('.noUi-origin')[0]
+        : undefined,
     options[`filter_min_${dimension}_enabled`]
   );
 
   useDisableSliderHandle(
-    () => sliderRef.current?.querySelectorAll('.noUi-origin')[1],
+    () =>
+      sliderRef.current
+        ? sliderRef.current.querySelectorAll('.noUi-origin')[1]
+        : undefined,
     options[`filter_max_${dimension}_enabled`]
   );
 

src/Popup/AdvancedFilters.js

@@ -20,7 +20,9 @@ export const OpenImageButton = ({ imageUrl, onClick, ...props }) => {
       style=${{ backgroundImage: `url("/images/open.svg")` }}
       onClick=${(e) => {
         chrome.tabs.create({ url: imageUrl, active: false });
-        onClick?.(e);
+        if (onClick) {
+          onClick(e);
+        }
       }}
       ...${props}
     />
@@ -40,7 +42,9 @@ export const DownloadImageButton = ({
       style=${{ backgroundImage: `url("/images/download.svg")` }}
       onClick=${(e) => {
         actions.downloadImages([imageUrl], options);
-        onClick?.(e);
+        if (onClick) {
+          onClick(e);
+        }
       }}
       ...${props}
     />

src/Popup/ImageActions.js

@@ -1,13 +1,21 @@
-/** @type {Set<{ numberOfProcessedImages: number, imagesToDownload: string[], options: any, next: () => void }>} */
+// @ts-check
+/** @typedef {{ numberOfProcessedImages: number, imagesToDownload: string[], options: any, next: () => void }} Task */
+
+/** @type {Set<Task>} */
 const tasks = new Set();
 
-// NOTE: Don't directly use an `async` function as a listener:
-// https://stackoverflow.com/a/56483156
 chrome.runtime.onMessage.addListener(startDownload);
 chrome.downloads.onDeterminingFilename.addListener(suggestNewFilename);
 
-function startDownload(message, sender, resolve) {
-  if (message?.type !== 'downloadImages') return;
+// NOTE: Don't directly use an `async` function as a listener for `onMessage`:
+// https://stackoverflow.com/a/56483156
+// https://developer.chrome.com/docs/extensions/reference/runtime/#event-onMessage
+function startDownload(
+  /** @type {any} */ message,
+  /** @type {chrome.runtime.MessageSender} */ sender,
+  /** @type {(response?: any) => void} */ resolve
+) {
+  if (!(message && message.type === 'downloadImages')) return;
 
   downloadImages({
     numberOfProcessedImages: 0,
@@ -21,22 +29,24 @@ function startDownload(message, sender, resolve) {
     },
   }).then(resolve);
 
-  return true;
+  return true; // Keeps the message channel open until `resolve` is called
 }
 
-async function downloadImages(task) {
+async function downloadImages(/** @type {Task} */ task) {
   tasks.add(task);
   for (const image of task.imagesToDownload) {
     await new Promise((resolve) => {
       chrome.downloads.download({ url: image }, resolve);
     });
     if (chrome.runtime.lastError) {
       console.error(`${chrome.runtime.lastError.message}: ${image}`);
-      task.next();
     }
+    task.next();
   }
 }
 
+// https://developer.chrome.com/docs/extensions/reference/downloads/#event-onDeterminingFilename
+/** @type {Parameters<chrome.downloads.DownloadDeterminingFilenameEvent['addListener']>[0]} */
 function suggestNewFilename(item, suggest) {
   const task = [...tasks][0];
   if (!task) {
@@ -62,7 +72,6 @@ function suggestNewFilename(item, suggest) {
   }
 
   suggest({ filename: normalizeSlashes(newFilename) });
-  task.next();
 }
 
 function normalizeSlashes(filename) {

src/background/downloadImages.js

@@ -1,3 +1,4 @@
+// @ts-check
 chrome.runtime.onInstalled.addListener((details) => {
   if (details.reason === 'install') {
     // Open the options page after install