Check whether if instance is destroyed before drawing

Author: flekschas

src/index.js

@@ -272,6 +272,7 @@ const createScatterplot = (
   lassoColor = toRgba(lassoColor, true);
   reticleColor = toRgba(reticleColor, true);
 
+  let isDestroyed = false;
   let backgroundColorBrightness = rgbBrightness(backgroundColor);
   let camera;
   let lasso;
@@ -2032,10 +2033,22 @@ const createScatterplot = (
    * @param {import('./types').ScatterplotMethodOptions['draw']} options
    * @returns {Promise<void>}
    */
-  const publicDraw = (newPoints, options = {}) =>
-    toArrayOrientedPoints(newPoints).then(
+  const publicDraw = (newPoints, options = {}) => {
+    if (isDestroyed) {
+      return Promise.reject(new Error('The instance was already destroyed'));
+    }
+    return toArrayOrientedPoints(newPoints).then(
       (points) =>
         new Promise((resolve) => {
+          if (isDestroyed) {
+            // In the special case where the instance was destroyed after
+            // scatterplot.draw() was called but before toArrayOrientedPoints()
+            // resolved, we will _not_ reject the promise as this would be
+            // confusing. Instead we will immediately resolve and return.
+            resolve();
+            return;
+          }
+
           let pointsCached = false;
           if (points) {
             if (options.transition) {
@@ -2085,6 +2098,7 @@ const createScatterplot = (
           }
         })
     );
+  };
 
   /** @type {<F extends Function>(f: F) => (...args: Parameters<F>) => ReturnType<F>} */
   const withDraw =
@@ -3238,6 +3252,7 @@ const createScatterplot = (
   };
 
   const destroy = () => {
+    isDestroyed = true;
     cancelFrameListener();
     window.removeEventListener('keyup', keyUpHandler, false);
     window.removeEventListener('blur', blurHandler, false);
@@ -3261,15 +3276,15 @@ const createScatterplot = (
     pointConnections.destroy();
     reticleHLine.destroy();
     reticleVLine.destroy();
-    pubSub.publish('destroy');
-    pubSub.clear();
     if (!initialProperties.renderer) {
       // Since the user did not pass in an externally created renderer we can
       // assume that the renderer is only used by this scatter plot instance.
       // Therefore it's save to destroy it when this scatter plot instance is
       // destroyed.
       renderer.destroy();
     }
+    pubSub.publish('destroy');
+    pubSub.clear();
   };
 
   init();

tests/index.js

@@ -1163,6 +1163,47 @@ test(
   })
 );
 
+test(
+  'throw an error when calling draw() after destroy()',
+  catchError(async (t) => {
+    const canvas = createCanvas(200, 200);
+    const scatterplot = createScatterplot({ canvas, width: 200, height: 200 });
+
+    await scatterplot.draw([[0, 0]]);
+
+    scatterplot.destroy();
+
+    try {
+      await scatterplot.draw([[0, 0]]);
+      t.fail(
+        'should have thrown an error because the scatterplot instance is destroyed'
+      );
+    } catch (e) {
+      t.eq(e.message, 'The instance was already destroyed');
+    }
+  })
+);
+
+test(
+  'do _not_ throw an error when calling draw() _before_ destroy()',
+  catchError(async (t) => {
+    const canvas = createCanvas(200, 200);
+    const scatterplot = createScatterplot({ canvas, width: 200, height: 200 });
+
+    let numDraws = 0;
+    scatterplot.subscribe('draw', () => ++numDraws);
+
+    scatterplot.draw([[0, 0]]);
+    scatterplot.destroy();
+
+    t.equal(
+      numDraws,
+      0,
+      'draw call should have been canceled due to the destroy call'
+    );
+  })
+);
+
 test('tests involving mouse events', async (t2) => {
   await t2.test(
     'draw(), clear(), publish("select")',