fix(): do not re-render controlled node after disposed

Author: weareoutman

packages/runtime/src/internal/Renderer.ts

@@ -610,7 +610,10 @@ async function legacyRenderBrick(
         });
 
         // Ignore stale renders
-        if (renderId === currentRenderId) {
+        if (
+          renderId === currentRenderId &&
+          !(returnNode.tag !== RenderTag.ROOT && returnNode.disposed)
+        ) {
           if (onUnmount) {
             listenerFactory(
               onUnmount,
@@ -1004,6 +1007,11 @@ async function legacyRenderBrick(
                 ({ failed, output: incrementalOutput } = result);
               }
 
+              // istanbul ignore next: covered by e2e tests
+              if (brick.disposed) {
+                return true;
+              }
+
               newControlNode.child = incrementalOutput.node;
               mergeRenderOutput(newOutput, {
                 ...incrementalOutput,

packages/runtime/src/internal/RendererContext.ts

@@ -363,6 +363,21 @@ export class RendererContext {
   #unmountAbstracts(abstracts: Set<RenderAbstract>) {
     for (const item of abstracts) {
       item.disposes?.forEach((dispose) => dispose());
+      delete item.disposes;
+    }
+  }
+
+  #cleanUpNodes(nodes: Set<RenderAbstract | RenderBrick>) {
+    for (const node of nodes) {
+      delete node.child;
+      delete node.sibling;
+      delete (node as Partial<RenderChildNode>).return;
+      node.disposed = true;
+      if (node.tag === RenderTag.BRICK) {
+        delete node.element;
+        delete (node as Partial<RenderBrick>).runtimeContext;
+      }
+      Object.freeze(node);
     }
   }
 
@@ -387,6 +402,10 @@ export class RendererContext {
     node: RenderChildNode,
     oldNode: RenderChildNode
   ) {
+    // istanbul ignore next: defensive check
+    if (returnNode.tag !== RenderTag.ROOT && returnNode.disposed) {
+      return;
+    }
     const [prevLastNormal, prevLastPortal] = findLastChildNodes(oldNode);
     const insertBeforeChild =
       (prevLastNormal
@@ -452,10 +471,8 @@ export class RendererContext {
     }
     node.sibling = oldNode.sibling;
 
-    // Clean up old node
-    delete oldNode.child;
-    delete oldNode.sibling;
-    delete (oldNode as Partial<RenderChildNode>).return;
+    this.#cleanUpNodes(removeAbstracts);
+    this.#cleanUpNodes(removeBricks);
 
     // Resume `return`
     current = node;

packages/runtime/src/internal/interfaces.ts

@@ -68,6 +68,7 @@ export interface RenderAbstract extends BaseRenderNode {
   return: RenderReturnNode;
   iid?: string;
   disposes?: (() => void)[];
+  disposed?: boolean;
 }
 
 export interface BaseRenderNode {
@@ -94,6 +95,7 @@ export interface RuntimeBrick {
   portal?: boolean;
   ref?: string;
   disposes?: (() => void)[];
+  disposed?: boolean;
 }
 
 export type MetaInfoOfEventListener = [

packages/runtime/src/themeAndMode.spec.ts

@@ -167,20 +167,15 @@ describe("variant", () => {
     setThemeVariant("elevo");
     expect(getThemeVariant()).toEqual("elevo");
     expect(document.documentElement.dataset.variant).toBe("elevo");
-  });
 
-  test("should apply the current theme variant", () => {
-    expect(getThemeVariant()).toEqual("default");
-    expect(document.documentElement.dataset.variant).toBe("default");
+    // Set the same variant again
     setThemeVariant("elevo");
-    expect(document.documentElement.dataset.variant).toBe("elevo");
+    expect(getThemeVariant()).toEqual("elevo");
   });
 
-  test("should apply the specified theme variant", () => {
+  test("should fallback to default variant if an invalid one is set", () => {
+    setThemeVariant("oops" as any);
     expect(getThemeVariant()).toEqual("default");
     expect(document.documentElement.dataset.variant).toBe("default");
-    setThemeVariant("elevo");
-    expect(getThemeVariant()).toEqual("elevo");
-    expect(document.documentElement.dataset.variant).toBe("elevo");
   });
 });