Merge pull request #21002 from NullVoxPopuli/nvp/canary/no-render-while-destroying

Author: kategengler

packages/@ember/-internals/glimmer/lib/renderer.ts

@@ -10,6 +10,7 @@ import {
   associateDestroyableChild,
   destroy,
   isDestroyed,
+  isDestroying,
   registerDestructor,
 } from '@glimmer/destroyable';
 import { DEBUG } from '@glimmer/env';
@@ -158,8 +159,13 @@ class ComponentRootState {
 
       associateDestroyableChild(this, this.#result);
 
-      // override .render function after initial render
-      this.#render = errorLoopTransaction(() => result.rerender({ alwaysRevalidate: false }));
+      this.#render = errorLoopTransaction(() => {
+        if (isDestroying(result) || isDestroyed(result)) return;
+
+        return result.rerender({
+          alwaysRevalidate: false,
+        });
+      });
     });
   }
 
@@ -228,8 +234,13 @@ class ClassicRootState {
 
       associateDestroyableChild(owner, result);
 
-      // override .render function after initial render
-      this.render = errorLoopTransaction(() => result.rerender({ alwaysRevalidate: false }));
+      this.render = errorLoopTransaction(() => {
+        if (isDestroying(result) || isDestroyed(result)) return;
+
+        return result.rerender({
+          alwaysRevalidate: false,
+        });
+      });
     });
   }
 

packages/@ember/-internals/glimmer/tests/integration/components/render-component-test.ts

@@ -498,11 +498,21 @@ moduleFor(
        * https://github.com/emberjs/rfcs/pull/1099/files#diff-2b962105b9083ca84579cdc957f27f49407440f3c5078083fa369ec18cc46da8R365
        *
        * We could later add an option to not do this behavior
+       *
+       *
+       * NOTE: for this verify-steps, we only expect foo:3 once, because the first
+       *       incarnation of renderComponent (back when foo was 2) will not run again, due
+       *       to being destroyed.
        */
-      assert.verifySteps(
-        [`render:root`, `foo:3`, `foo:3`],
-        `Destruction is async, so we get an extra 'foo:3' here, and then because our getter consumes foo (since renderComponent is eager), we dirty the cached getter, and re-run the getter, creating a new renderComponent call, overwriting the existing contents`
-      );
+      assert.verifySteps([`render:root`, `foo:3`]);
+
+      assert.strictEqual(this.element.innerHTML, '<div>3 <button>++</button></div>');
+
+      run(() => {
+        destroy(this.owner);
+      });
+
+      assert.strictEqual(this.element.innerHTML, '');
     }
 
     '@test multiple renderComponents share reactivity'() {