fix memory leak + optimization signals

Author: ged-odoo

src/runtime/blockdom/block_compiler.ts

@@ -568,9 +568,7 @@ function createBlockClass(template: HTMLElement, ctx: BlockCtx): BlockClass {
         }
       }
 
-      nodeInsertBefore.call(parent, el, afterNode);
-
-      // preparing all children
+      // preparing all children (off-DOM, before inserting el into the live document)
       if (childN) {
         const children = this.children;
         for (let i = 0; i < childN; i++) {
@@ -584,6 +582,8 @@ function createBlockClass(template: HTMLElement, ctx: BlockCtx): BlockClass {
           }
         }
       }
+
+      nodeInsertBefore.call(parent, el, afterNode);
       this.el = el as HTMLElement;
       this.parentEl = parent;
 

src/runtime/blockdom/list.ts

@@ -90,7 +90,6 @@ class VList {
           beforeRemove.call(ch1[i]);
         }
       }
-
       nodeSetTextContent.call(parent, "");
       nodeAppendChild.call(parent, _anchor);
       return;

src/runtime/component_node.ts

@@ -7,6 +7,7 @@ import {
   ComputationAtom,
   ComputationState,
   createComputation,
+  disposeComputation,
   getCurrentComputation,
   setComputation,
 } from "./reactivity/computations";
@@ -43,6 +44,7 @@ export class ComponentNode implements VNode<ComponentNode> {
   patched: LifecycleHook[] = [];
   willDestroy: LifecycleHook[] = [];
   signalComputation: ComputationAtom;
+  computations: ComputationAtom[] = [];
 
   pluginManager: PluginManager;
 
@@ -204,6 +206,10 @@ export class ComponentNode implements VNode<ComponentNode> {
         this.app.handleError({ error: e, node: this });
       }
     }
+    for (const computation of this.computations) {
+      disposeComputation(computation);
+    }
+    disposeComputation(this.signalComputation);
     this.status = STATUS.DESTROYED;
   }
 

src/runtime/plugin_manager.ts

@@ -1,7 +1,7 @@
 import { OwlError } from "../common/owl_error";
 import { App } from "./app";
 import { contextStack } from "./context";
-import { untrack } from "./reactivity/computations";
+import { ComputationAtom, disposeComputation, untrack } from "./reactivity/computations";
 import { effect } from "./reactivity/effect";
 import { Resource } from "./resource";
 import { STATUS } from "./status";
@@ -38,6 +38,7 @@ export class PluginManager {
   app: App;
   config: Record<string, any>;
   onDestroyCb: Function[] = [];
+  computations: ComputationAtom[] = [];
   plugins: Record<string, Plugin>;
   status: STATUS = STATUS.NEW;
 
@@ -59,7 +60,9 @@ export class PluginManager {
     while (cbs.length) {
       cbs.pop()!();
     }
-
+    for (const computation of this.computations) {
+      disposeComputation(computation);
+    }
     this.status = STATUS.DESTROYED;
   }
 

src/runtime/reactivity/computations.ts

@@ -127,6 +127,23 @@ export function removeSources(computation: ComputationAtom) {
   sources.clear();
 }
 
+export function disposeComputation(computation: ComputationAtom) {
+  for (const source of computation.sources) {
+    source.observers.delete(computation);
+    // Recursively dispose derived computations that lost all observers
+    if (
+      "compute" in source &&
+      (source as ComputationAtom).isDerived &&
+      source.observers.size === 0
+    ) {
+      disposeComputation(source as ComputationAtom);
+    }
+  }
+  computation.sources.clear();
+  // Mark as stale so it recomputes correctly if ever re-used (shared computed case)
+  computation.state = ComputationState.STALE;
+}
+
 function markDownstream(computation: ComputationAtom) {
   const stack: ComputationAtom[] = [computation];
   let current: ComputationAtom | undefined;

src/runtime/reactivity/computed.ts

@@ -7,6 +7,7 @@ import {
   updateComputation,
   createComputation,
 } from "./computations";
+import { contextStack } from "../context";
 
 interface ComputedOptions<TWrite> {
   set?(value: TWrite): void;
@@ -17,8 +18,11 @@ export function computed<TRead, TWrite = TRead>(
   options: ComputedOptions<TWrite> = {}
 ): ReactiveValue<TRead, TWrite> {
   const computation = createComputation(() => {
-    onWriteAtom(computation);
-    return getter();
+    const newValue = getter();
+    if (!Object.is(computation.value, newValue)) {
+      onWriteAtom(computation);
+    }
+    return newValue;
   }, true);
 
   function readComputed() {
@@ -31,5 +35,14 @@ export function computed<TRead, TWrite = TRead>(
   readComputed[atomSymbol] = computation;
   readComputed.set = options.set ?? (() => {});
 
+  const context = contextStack.at(-1);
+  if (context) {
+    if (context.type === "component") {
+      context.node.computations.push(computation);
+    } else if (context.type === "plugin") {
+      context.manager.computations.push(computation);
+    }
+  }
+
   return readComputed;
 }

src/runtime/rendering/fibers.ts

@@ -1,7 +1,7 @@
 import { OwlError } from "../../common/owl_error";
 import { BDom, mount } from "../blockdom";
 import type { ComponentNode } from "../component_node";
-import { getCurrentComputation, setComputation } from "../reactivity/computations";
+import { getCurrentComputation, removeSources, setComputation } from "../reactivity/computations";
 import { STATUS } from "../status";
 import { fibersInError } from "./error_handling";
 
@@ -136,6 +136,7 @@ export class Fiber {
     if (root) {
       // todo: should use updateComputation somewhere else.
       const c = getCurrentComputation();
+      removeSources(node.signalComputation);
       setComputation(node.signalComputation);
       try {
         (this.bdom as any) = true;

tests/components/__snapshots__/reactivity.test.ts.snap

@@ -40,6 +40,120 @@ exports[`components and computed simple component and computed value 1`] = `
 }"
 `;
 
+exports[`reactive cleanup on component destruction benchmark-like scenario: signal.Array rows with computed isSelected 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  let { prepareList, createComponent, withKey } = helpers;
+  const comp1 = createComponent(app, \`TableRow\`, true, false, false, ["row","selected"]);
+  
+  let block1 = createBlock(\`<table><tbody><block-child-0/></tbody></table>\`);
+  
+  return function template(ctx, node, key = "") {
+    const ctx1 = ctx;
+    const [k_block2, v_block2, l_block2, c_block2] = prepareList(ctx['this'].rows());;
+    for (let i1 = 0; i1 < l_block2; i1++) {
+      let ctx = Object.create(ctx1);
+      ctx[\`row\`] = k_block2[i1];
+      const key1 = ctx['row'].id;
+      c_block2[i1] = withKey(comp1({row: ctx['row'],selected: ctx['this'].selectedId}, key + \`__1__\${key1}\`, node, this, null), key1);
+    }
+    const b2 = list(c_block2);
+    return block1([], [b2]);
+  }
+}"
+`;
+
+exports[`reactive cleanup on component destruction benchmark-like scenario: signal.Array rows with computed isSelected 2`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  let { safeOutput } = helpers;
+  
+  let block1 = createBlock(\`<tr block-attribute-0="class"><td><block-child-0/></td><td><a><block-child-1/></a></td></tr>\`);
+  
+  return function template(ctx, node, key = "") {
+    let attr1 = ctx['this'].isSelected()?'danger':'';
+    const b2 = safeOutput(ctx['this'].p.row.id);
+    const b3 = safeOutput(ctx['this'].p.row.label());
+    return block1([attr1], [b2, b3]);
+  }
+}"
+`;
+
+exports[`reactive cleanup on component destruction destroying a component with computed cleans up signal observers 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  let { createComponent } = helpers;
+  const comp1 = createComponent(app, \`Child\`, true, false, false, ["selected"]);
+  
+  let block1 = createBlock(\`<div><block-child-0/></div>\`);
+  
+  return function template(ctx, node, key = "") {
+    let b2;
+    if (ctx['this'].show()) {
+      b2 = comp1({selected: ctx['this'].selectedId}, key + \`__1\`, node, this, null);
+    }
+    return block1([], [b2]);
+  }
+}"
+`;
+
+exports[`reactive cleanup on component destruction destroying a component with computed cleans up signal observers 2`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  let { safeOutput } = helpers;
+  
+  let block1 = createBlock(\`<span><block-child-0/></span>\`);
+  
+  return function template(ctx, node, key = "") {
+    const b2 = safeOutput(ctx['this'].isSelected());
+    return block1([], [b2]);
+  }
+}"
+`;
+
+exports[`reactive cleanup on component destruction destroying multiple children with computeds cleans up all signal observers 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  let { prepareList, createComponent, withKey } = helpers;
+  const comp1 = createComponent(app, \`Row\`, true, false, false, ["id","selected"]);
+  
+  let block1 = createBlock(\`<div><block-child-0/></div>\`);
+  
+  return function template(ctx, node, key = "") {
+    const ctx1 = ctx;
+    const [k_block2, v_block2, l_block2, c_block2] = prepareList(ctx['this'].rows());;
+    for (let i1 = 0; i1 < l_block2; i1++) {
+      let ctx = Object.create(ctx1);
+      ctx[\`row\`] = k_block2[i1];
+      const key1 = ctx['row'];
+      c_block2[i1] = withKey(comp1({id: ctx['row'],selected: ctx['this'].selectedId}, key + \`__1__\${key1}\`, node, this, null), key1);
+    }
+    const b2 = list(c_block2);
+    return block1([], [b2]);
+  }
+}"
+`;
+
+exports[`reactive cleanup on component destruction destroying multiple children with computeds cleans up all signal observers 2`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  let { safeOutput } = helpers;
+  
+  let block1 = createBlock(\`<span><block-child-0/></span>\`);
+  
+  return function template(ctx, node, key = "") {
+    const b2 = safeOutput(ctx['this'].isSelected());
+    return block1([], [b2]);
+  }
+}"
+`;
+
 exports[`reactivity in lifecycle Child component doesn't render when state they depend on changes but their parent is about to unmount them 1`] = `
 "function anonymous(app, bdom, helpers
 ) {