[REF] blockdom: some small optimization

Before this commit, most of the time, components are destroyed when the
virtual dom is patched and a component node is removed. However, since
Owl is asynchronous and a component may take some time to get ready
(with onWillStart), it can happen that a component is created, then
before it is ready, it is recreated.  In that case, the initial instance
has to be destroyed.

Before this commit, the destroy operation was done immediately, when we
cancel the current fibers.  However, this means that we cannot have a
guarantee between micro task ticks that a component has not been
destroyed in the meantime.

For example, in Odoo, it is common to use the rpc service, which will
throw an error if called by a destroyed component. But because of the
possible destruction of a component at any time, the following code is
unsafe:

async loadSomeData() {
  // guaranteed to be called when component is alive
  await Promise.resolve();
  // however here, component may have been destroyed
  this.rpc(...)
}

So, to prevent this issue, we can slightly delay the destroy operation.
It is not entirely trivial, since we need to find a way to neutralize
the component in the meantime. But it seems like performing all that
kind of operation at the "commit" phase (so, the request animation frame
callback) makes sense to me.

So, this commit modifies the code to add a new component status
(cancelled) and use it to cancel components that are waiting to be
destroyed. These components will then be destroyed as soon as the
requestanimation frame starts, before all other dom operations.

doc/reference/component.md

@@ -451,5 +451,6 @@ console.log(status(component));
 // logs either:
 // - 'new', if the component is new and has not been mounted yet
 // - 'mounted', if the component is currently mounted
+// - 'cancelled', if the component has not been mounted yet but will be destroyed soon
 // - 'destroyed' if the component is currently destroyed
 ```

src/compiler/parser.ts

@@ -365,10 +365,8 @@ function parseDOMNode(node: Element, ctx: ParsingContext): AST | null {
       const typeAttr = node.getAttribute("type");
       const isInput = tagName === "input";
       const isSelect = tagName === "select";
-      const isTextarea = tagName === "textarea";
       const isCheckboxInput = isInput && typeAttr === "checkbox";
       const isRadioInput = isInput && typeAttr === "radio";
-      const isOtherInput = isInput && !isCheckboxInput && !isRadioInput;
       const hasLazyMod = attr.includes(".lazy");
       const hasNumberMod = attr.includes(".number");
       const hasTrimMod = attr.includes(".trim");
@@ -381,8 +379,8 @@ function parseDOMNode(node: Element, ctx: ParsingContext): AST | null {
         specialInitTargetAttr: isRadioInput ? "checked" : null,
         eventType,
         hasDynamicChildren: false,
-        shouldTrim: hasTrimMod && (isOtherInput || isTextarea),
-        shouldNumberize: hasNumberMod && (isOtherInput || isTextarea),
+        shouldTrim: hasTrimMod,
+        shouldNumberize: hasNumberMod,
       };
       if (isSelect) {
         // don't pollute the original ctx

src/runtime/blockdom/block_compiler.ts

@@ -93,6 +93,17 @@ function normalizeNode(node: HTMLElement | Text) {
   }
 }
 
+/**
+ * Encode 2 numbers and 1 boolean in a number, using 31 bits:
+ * n1 => encoded in 16 most significant bits
+ * n2 => encoded in 15 next bits
+ * boolean => encoded in last significant bit.
+ * This code assumes that n1 and n2 are small enough to fit in that number of bits
+ */
+function encodeValue(n1: number, n2: number, b: boolean): number {
+  return (((n1 << 15) | n2) << 1) | (b ? 1 : 0);
+}
+
 // -----------------------------------------------------------------------------
 // building a intermediate tree
 // -----------------------------------------------------------------------------
@@ -306,7 +317,7 @@ interface IndexedLocation extends Location {
 
 interface Child {
   parentRefIdx: number;
-  afterRefIdx?: number;
+  afterRefIdx: number;
   isOnlyChild?: boolean;
 }
 
@@ -374,6 +385,7 @@ function updateCtx(ctx: BlockCtx, tree: IntermediateTree) {
           // tree is the parentnode here
           ctx.children[info.idx] = {
             parentRefIdx: info.refIdx!,
+            afterRefIdx: 0,
             isOnlyChild: true,
           };
         } else {
@@ -501,7 +513,6 @@ function createBlockClass(template: HTMLElement, ctx: BlockCtx): BlockClass {
   }));
   const locN = locations.length;
   const childN = children.length;
-  const childrenLocs = children;
   const isDynamic = refN > 0;
 
   // these values are defined here to make them faster to lookup in the class
@@ -556,6 +567,19 @@ function createBlockClass(template: HTMLElement, ctx: BlockCtx): BlockClass {
   }
 
   if (isDynamic) {
+    const nextSibling = nodeGetNextSibling;
+    const firstChild = nodeGetFirstChild;
+    const bitPackedCollectors = new Uint32Array(
+      collectors.map((c) => {
+        return encodeValue(c.idx, c.prevIdx, c.getVal === nextSibling);
+      })
+    );
+    const childrenLocs = new Uint32Array(
+      children.map((c) => {
+        return encodeValue(c.afterRefIdx, c.parentRefIdx, Boolean(c.isOnlyChild));
+      })
+    );
+
     Block.prototype.mount = function mount(parent: HTMLElement, afterNode: Node | null) {
       const el = nodeCloneNode.call(template, true);
       // collecting references
@@ -563,12 +587,17 @@ function createBlockClass(template: HTMLElement, ctx: BlockCtx): BlockClass {
       this.refs = refs;
       refs[0] = el;
       for (let i = 0; i < colN; i++) {
-        const w = collectors[i];
-        refs[w.idx] = w.getVal.call(refs[w.prevIdx]);
+        let info = bitPackedCollectors[i];
+        // decode info
+        const fn = (info & 1) === 1 ? nextSibling : firstChild;
+        info = info >> 1;
+        const prevIdx = info & 0b111111111111111;
+        const idx = info >> 15;
+        refs[idx] = fn.call(refs[prevIdx]);
       }
 
       // applying data to all update points
-      if (locN) {
+      if (locN !== 0) {
         const data = this.data!;
         for (let i = 0; i < locN; i++) {
           const loc = locations[i];
@@ -579,15 +608,21 @@ function createBlockClass(template: HTMLElement, ctx: BlockCtx): BlockClass {
       nodeInsertBefore.call(parent, el, afterNode);
 
       // preparing all children
-      if (childN) {
+      if (childN !== 0) {
         const children = this.children;
         for (let i = 0; i < childN; i++) {
           const child = children![i];
-          if (child) {
-            const loc = childrenLocs[i];
-            const afterNode = loc.afterRefIdx ? refs[loc.afterRefIdx] : null;
-            child.isOnlyChild = loc.isOnlyChild;
-            child.mount(refs[loc.parentRefIdx] as any, afterNode);
+          if (child !== undefined) {
+            let info = childrenLocs[i];
+            // decode info
+            const isOnlyChild = info & 1;
+            info = info >> 1;
+            const parentRefIdx = info & 0b111111111111111;
+            const afterRefIdx = info >> 15;
+
+            const afterNode = afterRefIdx !== 0 ? refs[afterRefIdx] : null;
+            child.isOnlyChild = isOnlyChild as any;
+            child.mount(refs[parentRefIdx] as any, afterNode);
           }
         }
       }
@@ -601,7 +636,7 @@ function createBlockClass(template: HTMLElement, ctx: BlockCtx): BlockClass {
       }
       const refs = this.refs!;
       // update texts/attributes/
-      if (locN) {
+      if (locN !== 0) {
         const data1 = this.data!;
         const data2 = other.data!;
         for (let i = 0; i < locN; i++) {
@@ -616,14 +651,14 @@ function createBlockClass(template: HTMLElement, ctx: BlockCtx): BlockClass {
       }
 
       // update children
-      if (childN) {
+      if (childN !== 0) {
         let children1 = this.children;
         const children2 = other.children;
         for (let i = 0; i < childN; i++) {
           const child1 = children1![i];
           const child2 = children2![i];
-          if (child1) {
-            if (child2) {
+          if (child1 !== undefined) {
+            if (child2 !== undefined) {
               child1.patch(child2, withBeforeRemove);
             } else {
               if (withBeforeRemove) {
@@ -632,10 +667,15 @@ function createBlockClass(template: HTMLElement, ctx: BlockCtx): BlockClass {
               child1.remove();
               children1![i] = undefined;
             }
-          } else if (child2) {
-            const loc = childrenLocs[i];
-            const afterNode = loc.afterRefIdx ? refs[loc.afterRefIdx] : null;
-            child2.mount(refs[loc.parentRefIdx] as any, afterNode);
+          } else if (child2 !== undefined) {
+            let info = childrenLocs[i];
+            // decode info
+            info = info >> 1;
+            const parentRefIdx = info & 0b111111111111111;
+            const afterRefIdx = info >> 15;
+
+            const afterNode = afterRefIdx !== 0 ? refs[afterRefIdx] : null;
+            child2.mount(refs[parentRefIdx] as any, afterNode);
             children1![i] = child2;
           }
         }

src/runtime/blockdom/events.ts

@@ -41,7 +41,7 @@ function createElementHandler(evName: string, capture: boolean = false): EventHa
   }
 
   function remove(this: HTMLElement) {
-    delete (this as any)[eventKey];
+    (this as any)[eventKey] = false;
     this.removeEventListener(evName, listener, { capture });
   }
   function update(this: HTMLElement, data: any) {

src/runtime/blockdom/list.ts

@@ -28,7 +28,7 @@ class VList {
     this.anchor = _anchor;
     nodeInsertBefore.call(parent, _anchor, afterNode);
     const l = children.length;
-    if (l) {
+    if (l !== 0) {
       const mount = children[0].mount;
       for (let i = 0; i < l; i++) {
         mount.call(children[i], parent, _anchor);
@@ -186,7 +186,7 @@ class VList {
       } else {
         for (let i = startIdx1; i <= endIdx1; i++) {
           let ch = ch1[i];
-          if (ch) {
+          if (ch !== null) {
             if (withBeforeRemove) {
               beforeRemove.call(ch);
             }
@@ -200,7 +200,7 @@ class VList {
   beforeRemove() {
     const children = this.children;
     const l = children.length;
-    if (l) {
+    if (l !== 0) {
       const beforeRemove = children[0].beforeRemove;
       for (let i = 0; i < l; i++) {
         beforeRemove.call(children[i]);
@@ -215,7 +215,7 @@ class VList {
     } else {
       const children = this.children;
       const l = children.length;
-      if (l) {
+      if (l !== 0) {
         const remove = children[0].remove;
         for (let i = 0; i < l; i++) {
           remove.call(children[i]);
@@ -240,7 +240,7 @@ export function list(children: VNode[]): VNode<VList> {
 }
 
 function createMapping(ch1: any[], startIdx1: number, endIdx2: number): { [key: string]: any } {
-  let mapping: any = {};
+  const mapping: any = {};
   for (let i = startIdx1; i <= endIdx2; i++) {
     mapping[ch1[i].key] = i;
   }

src/runtime/blockdom/multi.ts

@@ -26,7 +26,7 @@ export class VMulti {
     const anchors = new Array(l);
     for (let i = 0; i < l; i++) {
       let child = children[i];
-      if (child) {
+      if (child !== undefined) {
         child.mount(parent, afterNode);
       } else {
         const childAnchor = document.createTextNode("");
@@ -44,7 +44,7 @@ export class VMulti {
     const anchors = this.anchors;
     for (let i = 0, l = children.length; i < l; i++) {
       let child = children[i];
-      if (child) {
+      if (child !== undefined) {
         child.moveBeforeDOMNode(node, parent);
       } else {
         const anchor = anchors![i];
@@ -56,14 +56,14 @@ export class VMulti {
   moveBeforeVNode(other: VMulti | null, afterNode: Node | null) {
     if (other) {
       const next = other!.children[0];
-      afterNode = (next ? next.firstNode() : other!.anchors![0]) || null;
+      afterNode = (next !== undefined ? next.firstNode() : other!.anchors![0]) || null;
     }
     const children = this.children;
     const parent = this.parentEl;
     const anchors = this.anchors;
     for (let i = 0, l = children.length; i < l; i++) {
       let child = children[i];
-      if (child) {
+      if (child !== undefined) {
         child.moveBeforeVNode(null, afterNode);
       } else {
         const anchor = anchors![i];
@@ -83,8 +83,8 @@ export class VMulti {
     for (let i = 0, l = children1.length; i < l; i++) {
       const vn1 = children1[i];
       const vn2 = children2[i];
-      if (vn1) {
-        if (vn2) {
+      if (vn1 !== undefined) {
+        if (vn2 !== undefined) {
           vn1.patch(vn2, withBeforeRemove);
         } else {
           const afterNode = vn1.firstNode()!;
@@ -97,7 +97,7 @@ export class VMulti {
           vn1.remove();
           children1[i] = undefined;
         }
-      } else if (vn2) {
+      } else if (vn2 !== undefined) {
         children1[i] = vn2;
         const anchor = anchors[i];
         vn2.mount(parentEl, anchor);
@@ -110,7 +110,7 @@ export class VMulti {
     const children = this.children;
     for (let i = 0, l = children.length; i < l; i++) {
       const child = children[i];
-      if (child) {
+      if (child !== undefined) {
         child.beforeRemove();
       }
     }
@@ -125,7 +125,7 @@ export class VMulti {
       const anchors = this.anchors;
       for (let i = 0, l = children.length; i < l; i++) {
         const child = children[i];
-        if (child) {
+        if (child !== undefined) {
           child.remove();
         } else {
           nodeRemoveChild.call(parentEl, anchors![i]);
@@ -136,7 +136,7 @@ export class VMulti {
 
   firstNode(): Node | undefined {
     const child = this.children[0];
-    return child ? child.firstNode() : this.anchors![0];
+    return child !== undefined ? child.firstNode() : this.anchors![0];
   }
 
   toString(): string {

src/runtime/component_node.ts

@@ -145,6 +145,9 @@ export class ComponentNode<P extends Props = any, E = any> implements VNode<Comp
   }
 
   async render(deep: boolean) {
+    if (this.status >= STATUS.CANCELLED) {
+      return;
+    }
     let current = this.fiber;
     if (current && (current.root!.locked || (current as any).bdom === true)) {
       await Promise.resolve();
@@ -171,7 +174,7 @@ export class ComponentNode<P extends Props = any, E = any> implements VNode<Comp
 
     this.app.scheduler.addFiber(fiber);
     await Promise.resolve();
-    if (this.status === STATUS.DESTROYED) {
+    if (this.status >= STATUS.CANCELLED) {
       return;
     }
     // We only want to actually render the component if the following two
@@ -190,6 +193,20 @@ export class ComponentNode<P extends Props = any, E = any> implements VNode<Comp
     }
   }
 
+  cancel() {
+    this._cancel();
+    delete this.parent!.children[this.parentKey!];
+    this.app.scheduler.scheduleDestroy(this);
+  }
+
+  _cancel() {
+    this.status = STATUS.CANCELLED;
+    const children = this.children;
+    for (let childKey in children) {
+      children[childKey]._cancel();
+    }
+  }
+
   destroy() {
     let shouldRemove = this.status === STATUS.MOUNTED;
     this._destroy();

src/runtime/error_handling.ts

@@ -51,17 +51,19 @@ export function handleError(params: ErrorParams) {
     );
   }
   const node = "node" in params ? params.node : params.fiber.node;
-  const fiber = "fiber" in params ? params.fiber : node.fiber!;
+  const fiber = "fiber" in params ? params.fiber : node.fiber;
 
-  // resets the fibers on components if possible. This is important so that
-  // new renderings can be properly included in the initial one, if any.
-  let current: Fiber | null = fiber;
-  do {
-    current.node.fiber = current;
-    current = current.parent;
-  } while (current);
+  if (fiber) {
+    // resets the fibers on components if possible. This is important so that
+    // new renderings can be properly included in the initial one, if any.
+    let current: Fiber | null = fiber;
+    do {
+      current.node.fiber = current;
+      current = current.parent;
+    } while (current);
 
-  fibersInError.set(fiber.root!, error);
+    fibersInError.set(fiber.root!, error);
+  }
 
   const handled = _handleError(node, error);
   if (!handled) {

src/runtime/fibers.ts

@@ -55,8 +55,7 @@ function cancelFibers(fibers: Fiber[]): number {
     let node = fiber.node;
     fiber.render = throwOnRender;
     if (node.status === STATUS.NEW) {
-      node.destroy();
-      delete node.parent!.children[node.parentKey!];
+      node.cancel();
     }
     node.fiber = null;
     if (fiber.bdom) {

src/runtime/scheduler.ts

@@ -1,3 +1,4 @@
+import type { ComponentNode } from "./component_node";
 import { fibersInError } from "./error_handling";
 import { Fiber, RootFiber } from "./fibers";
 import { STATUS } from "./status";
@@ -14,6 +15,7 @@ export class Scheduler {
   requestAnimationFrame: Window["requestAnimationFrame"];
   frame: number = 0;
   delayedRenders: Fiber[] = [];
+  cancelledNodes: Set<ComponentNode> = new Set();
 
   constructor() {
     this.requestAnimationFrame = Scheduler.requestAnimationFrame;
@@ -23,6 +25,13 @@ export class Scheduler {
     this.tasks.add(fiber.root!);
   }
 
+  scheduleDestroy(node: ComponentNode) {
+    this.cancelledNodes.add(node);
+    if (this.frame === 0) {
+      this.frame = this.requestAnimationFrame(() => this.processTasks());
+    }
+  }
+
   /**
    * Process all current tasks. This only applies to the fibers that are ready.
    * Other tasks are left unchanged.
@@ -39,15 +48,23 @@ export class Scheduler {
     }
 
     if (this.frame === 0) {
-      this.frame = this.requestAnimationFrame(() => {
-        this.frame = 0;
-        this.tasks.forEach((fiber) => this.processFiber(fiber));
-        for (let task of this.tasks) {
-          if (task.node.status === STATUS.DESTROYED) {
-            this.tasks.delete(task);
-          }
-        }
-      });
+      this.frame = this.requestAnimationFrame(() => this.processTasks());
+    }
+  }
+
+  processTasks() {
+    this.frame = 0;
+    for (let node of this.cancelledNodes) {
+      node._destroy();
+    }
+    this.cancelledNodes.clear();
+    for (let task of this.tasks) {
+      this.processFiber(task);
+    }
+    for (let task of this.tasks) {
+      if (task.node.status === STATUS.DESTROYED) {
+        this.tasks.delete(task);
+      }
     }
   }
 

src/runtime/status.ts

@@ -7,15 +7,20 @@ import type { Component } from "./component";
 export const enum STATUS {
   NEW,
   MOUNTED, // is ready, and in DOM. It has a valid el
+  // component has been created, but has been replaced by a newer component before being mounted
+  // it is cancelled until the next animation frame where it will be destroyed
+  CANCELLED,
   DESTROYED,
 }
 
-type STATUS_DESCR = "new" | "mounted" | "destroyed";
+type STATUS_DESCR = "new" | "mounted" | "cancelled" | "destroyed";
 
 export function status(component: Component): STATUS_DESCR {
   switch (component.__owl__.status) {
     case STATUS.NEW:
       return "new";
+    case STATUS.CANCELLED:
+      return "cancelled";
     case STATUS.MOUNTED:
       return "mounted";
     case STATUS.DESTROYED:

src/runtime/template_helpers.ts

@@ -30,7 +30,7 @@ function callSlot(
   const slots = ctx.props.slots || {};
   const { __render, __ctx, __scope } = slots[name] || {};
   const slotScope = ObjectCreate(__ctx || {});
-  if (__scope) {
+  if (__scope !== undefined) {
     slotScope[__scope] = extra;
   }
   const slotBDom = __render ? __render(slotScope, parent, key) : null;

src/runtime/utils.ts

@@ -9,20 +9,13 @@ export type Callback = () => void;
  * @returns a batched version of the original callback
  */
 export function batched(callback: Callback): Callback {
-  let called = false;
-  return async () => {
-    // This await blocks all calls to the callback here, then releases them sequentially
-    // in the next microtick. This line decides the granularity of the batch.
-    await Promise.resolve();
-    if (!called) {
-      called = true;
-      // wait for all calls in this microtick to fall through before resetting "called"
-      // so that only the first call to the batched function calls the original callback.
-      // Schedule this before calling the callback so that calls to the batched function
-      // within the callback will proceed only after resetting called to false, and have
-      // a chance to execute the callback again
-      Promise.resolve().then(() => (called = false));
-      callback();
+  let scheduled = false;
+  return async (...args) => {
+    if (scheduled === false) {
+      scheduled = true;
+      await Promise.resolve();
+      scheduled = false;
+      callback(...args);
     }
   };
 }

tests/compiler/parser.test.ts

@@ -1991,8 +1991,8 @@ describe("qweb parser", () => {
       baseExpr: "state",
       expr: "'stuff'",
       eventType: "click",
-      shouldNumberize: false,
-      shouldTrim: false,
+      shouldNumberize: true,
+      shouldTrim: true,
       targetAttr: "value",
       hasDynamicChildren: false,
       specialInitTargetAttr: "checked",

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

@@ -212,6 +212,73 @@ exports[`changing state before first render does not trigger a render 1`] = `
 }"
 `;
 
+exports[`component destroyed just after render 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  const comp1 = app.createComponent(\`B\`, true, false, false, []);
+  
+  return function template(ctx, node, key = \\"\\") {
+    return comp1({}, key + \`__1\`, node, this, null);
+  }
+}"
+`;
+
+exports[`component destroyed just after render 2`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    const b2 = text(\`B\`);
+    const b3 = text(ctx['state'].value);
+    return multi([b2, b3]);
+  }
+}"
+`;
+
+exports[`components are not destroyed between animation frame 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  const comp1 = app.createComponent(\`B\`, true, false, false, []);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let b2,b3;
+    b2 = text(\`A\`);
+    if (ctx['state'].flag) {
+      b3 = comp1({}, key + \`__1\`, node, this, null);
+    }
+    return multi([b2, b3]);
+  }
+}"
+`;
+
+exports[`components are not destroyed between animation frame 2`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  const comp1 = app.createComponent(\`C\`, true, false, false, []);
+  
+  return function template(ctx, node, key = \\"\\") {
+    const b2 = text(\`B\`);
+    const b3 = comp1({}, key + \`__1\`, node, this, null);
+    return multi([b2, b3]);
+  }
+}"
+`;
+
+exports[`components are not destroyed between animation frame 3`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    return text(\`C\`);
+  }
+}"
+`;
+
 exports[`concurrent renderings scenario 1 1`] = `
 "function anonymous(app, bdom, helpers
 ) {

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

@@ -468,6 +468,36 @@ exports[`t-model directive t-model on select with static options 1`] = `
 }"
 `;
 
+exports[`t-model directive t-model with dynamic number values on select options in foreach 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  let { toNumber, prepareList, withKey } = helpers;
+  
+  let block1 = createBlock(\`<select block-handler-0=\\"change\\"><block-child-0/></select>\`);
+  let block3 = createBlock(\`<option block-attribute-0=\\"value\\" block-attribute-1=\\"selected\\"><block-text-2/></option>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    const bExpr1 = ctx['state'];
+    const expr1 = 'value';
+    const bValue1 = bExpr1[expr1];
+    let hdlr1 = [(ev) => { bExpr1[expr1] = toNumber(ev.target.value); }];
+    ctx = Object.create(ctx);
+    const [k_block2, v_block2, l_block2, c_block2] = prepareList(ctx['state'].options);;
+    for (let i1 = 0; i1 < l_block2; i1++) {
+      ctx[\`o\`] = v_block2[i1];
+      const key1 = ctx['o'].value;
+      let attr1 = ctx['o'].value;
+      let attr2 = bValue1 === ctx['o'].value;
+      let txt1 = ctx['o'].value;
+      c_block2[i1] = withKey(block3([attr1, attr2, txt1]), key1);
+    }
+    const b2 = list(c_block2);
+    return block1([hdlr1], [b2]);
+  }
+}"
+`;
+
 exports[`t-model directive t-model with dynamic values on select options -- 2 1`] = `
 "function anonymous(app, bdom, helpers
 ) {

tests/components/concurrency.test.ts

@@ -115,13 +115,7 @@ test("destroying/recreating a subwidget with different props (if start is not ov
   await nextMicroTick();
   expect(n).toBe(2);
 
-  expect([
-    "Child:willDestroy",
-    "W:willRender",
-    "Child:setup",
-    "Child:willStart",
-    "W:rendered",
-  ]).toBeLogged();
+  expect(["W:willRender", "Child:setup", "Child:willStart", "W:rendered"]).toBeLogged();
 
   def.resolve();
   await nextTick();
@@ -130,6 +124,7 @@ test("destroying/recreating a subwidget with different props (if start is not ov
   expect([
     "Child:willRender",
     "Child:rendered",
+    "Child:willDestroy",
     "W:willPatch",
     "Child:mounted",
     "W:patched",
@@ -178,13 +173,13 @@ test("destroying/recreating a subcomponent, other scenario", async () => {
     "Child:setup",
     "Child:willStart",
     "Parent:rendered",
-    "Child:willDestroy",
     "Parent:willRender",
     "Child:setup",
     "Child:willStart",
     "Parent:rendered",
     "Child:willRender",
     "Child:rendered",
+    "Child:willDestroy",
     "Parent:willPatch",
     "Child:mounted",
     "Parent:patched",
@@ -251,13 +246,13 @@ test("creating two async components, scenario 1", async () => {
   await nextTick();
   expect(fixture.innerHTML).toBe("");
   expect([
-    "ChildA:willDestroy",
     "Parent:willRender",
     "ChildA:setup",
     "ChildA:willStart",
     "ChildB:setup",
     "ChildB:willStart",
     "Parent:rendered",
+    "ChildA:willDestroy",
   ]).toBeLogged();
 
   defB.resolve();
@@ -703,13 +698,13 @@ test("rendering component again in next microtick", async () => {
     "Child:setup",
     "Child:willStart",
     "Parent:rendered",
-    "Child:willDestroy",
     "Parent:willRender",
     "Child:setup",
     "Child:willStart",
     "Parent:rendered",
     "Child:willRender",
     "Child:rendered",
+    "Child:willDestroy",
     "Parent:willPatch",
     "Child:mounted",
     "Parent:patched",
@@ -1732,9 +1727,9 @@ test("concurrent renderings scenario 10", async () => {
   expect(fixture.innerHTML).toBe("<div><p></p></div>");
   expect([
     "ComponentA:willRender",
-    "ComponentC:willDestroy",
     "ComponentB:willUpdateProps",
     "ComponentA:rendered",
+    "ComponentC:willDestroy",
   ]).toBeLogged();
 
   defB.resolve();
@@ -2282,7 +2277,6 @@ test("concurrent renderings scenario 16", async () => {
     "D:setup",
     "D:willStart",
     "C:rendered",
-    "D:willDestroy",
     "B:willRender",
     "C:willUpdateProps",
     "B:rendered",
@@ -2290,6 +2284,7 @@ test("concurrent renderings scenario 16", async () => {
     "D:setup",
     "D:willStart",
     "C:rendered",
+    "D:willDestroy",
   ]).toBeLogged();
 
   // at this point, C rendering is still pending, and nothing should have been
@@ -2997,11 +2992,11 @@ test("t-key on dom node having a component", async () => {
 
   expect(fixture.innerHTML).toBe("<div>3</div>");
   expect([
-    "Child (2):willDestroy",
     "Child (3):setup",
     "Child (3):willStart",
     "Child (3):willRender",
     "Child (3):rendered",
+    "Child (2):willDestroy",
     "Child (1):willUnmount",
     "Child (1):willDestroy",
     "Child (3):mounted",
@@ -3055,11 +3050,11 @@ test("t-key on dynamic async component (toggler is never patched)", async () =>
 
   expect(fixture.innerHTML).toBe("<div>3</div>");
   expect([
-    "Child (2):willDestroy",
     "Child (3):setup",
     "Child (3):willStart",
     "Child (3):willRender",
     "Child (3):rendered",
+    "Child (2):willDestroy",
     "Child (1):willUnmount",
     "Child (1):willDestroy",
     "Child (3):mounted",
@@ -3114,11 +3109,11 @@ test("t-foreach with dynamic async component", async () => {
 
   expect(fixture.innerHTML).toBe("<div>3</div>");
   expect([
-    "Child (2):willDestroy",
     "Child (3):setup",
     "Child (3):willStart",
     "Child (3):willRender",
     "Child (3):rendered",
+    "Child (2):willDestroy",
     "Child (1):willUnmount",
     "Child (1):willDestroy",
     "Child (3):mounted",
@@ -3801,7 +3796,7 @@ test("destroyed component causes other soon to be destroyed component to rerende
     static template = xml`<t t-esc="state.val + props.value"/>`;
     state = useState({ val: 0 });
     setup() {
-      c = this;
+      c = c || this;
       useLogLifecycle();
     }
   }
@@ -3846,8 +3841,6 @@ test("destroyed component causes other soon to be destroyed component to rerende
   parent.state.valueB = 2;
   await nextTick();
   expect([
-    "B:willDestroy",
-    "C:willDestroy",
     "A:willRender",
     "B:setup",
     "B:willStart",
@@ -3858,6 +3851,8 @@ test("destroyed component causes other soon to be destroyed component to rerende
     "B:rendered",
     "C:willRender",
     "C:rendered",
+    "B:willDestroy",
+    "C:willDestroy",
     "A:willPatch",
     "C:mounted",
     "B:mounted",
@@ -4200,6 +4195,116 @@ test("delayed render is not cancelled by upcoming render", async () => {
   ]).toBeLogged();
 });
 
+test("components are not destroyed between animation frame", async () => {
+  const def = makeDeferred();
+  class C extends Component {
+    static template = xml`C`;
+    setup() {
+      useLogLifecycle();
+    }
+  }
+  class B extends Component {
+    static template = xml`B<C/>`;
+    static components = { C };
+    setup() {
+      useLogLifecycle();
+      onWillStart(() => {
+        return def;
+      });
+    }
+  }
+  class A extends Component {
+    static template = xml`A<B t-if="state.flag"/>`;
+    static components = { B };
+
+    state = useState({ flag: false });
+    setup() {
+      useLogLifecycle();
+    }
+  }
+  const a = await mount(A, fixture);
+  expect(fixture.innerHTML).toBe("A");
+  expect(["A:setup", "A:willStart", "A:willRender", "A:rendered", "A:mounted"]).toBeLogged();
+
+  // turn the flag on, this will render A and stops at B because of def
+  a.state.flag = true;
+  await nextTick();
+  expect(["A:willRender", "B:setup", "B:willStart", "A:rendered"]).toBeLogged();
+
+  // force a render of A
+  //  => owl will need to create a new B component
+  //  => initial B component will be cancelled
+  a.render();
+  await nextMicroTick();
+  expect([
+    // note that B is not destroyed here. It is cancelled instead
+    "A:willRender",
+    "B:setup",
+    "B:willStart",
+    "A:rendered",
+  ]).toBeLogged();
+
+  // resolve def, so B render is unblocked
+  def.resolve();
+  await nextTick();
+  expect([
+    "B:willRender",
+    "C:setup",
+    "C:willStart",
+    "B:rendered",
+    "C:willRender",
+    "C:rendered",
+    // animation frame callback starts here
+    "B:willDestroy", // B is destroyed here
+    "A:willPatch",
+    "C:mounted",
+    "B:mounted",
+    "A:patched",
+  ]).toBeLogged();
+});
+
+test("component destroyed just after render", async () => {
+  let stateB: any;
+
+  class B extends Component {
+    static template = xml`B<t t-esc="state.value"/>`;
+    state = useState({ value: 1 });
+    setup() {
+      stateB = this.state;
+      useLogLifecycle();
+    }
+  }
+  class A extends Component {
+    static template = xml`<B/>`;
+    static components = { B };
+    setup() {
+      useLogLifecycle();
+    }
+  }
+  const a = await mount(A, fixture);
+  expect(fixture.innerHTML).toBe("B1");
+  expect([
+    "A:setup",
+    "A:willStart",
+    "A:willRender",
+    "B:setup",
+    "B:willStart",
+    "A:rendered",
+    "B:willRender",
+    "B:rendered",
+    "B:mounted",
+    "A:mounted",
+  ]).toBeLogged();
+
+  stateB!.value++; // force a render of B
+  await nextMicroTick(); // wait for B render to actually start
+  a.__owl__.app.destroy();
+  expect(["A:willUnmount", "B:willUnmount", "B:willDestroy", "A:willDestroy"]).toBeLogged();
+  await nextTick();
+  // check that B was not rendered after being destroyed
+  expect([]).toBeLogged();
+});
+
 //   test.skip("components with shouldUpdate=false", async () => {
 //     const state = { p: 1, cc: 10 };
 

tests/components/error_handling.test.ts

@@ -1444,6 +1444,7 @@ describe("can catch errors", () => {
       "Parent:willPatch",
       "Child:willUnmount",
       "Child:willDestroy",
+      "Parent:patched",
       "Parent:willRender",
       "Parent:rendered",
       "Parent:willPatch",
@@ -1506,12 +1507,15 @@ describe("can catch errors", () => {
     parent.state.hasChild = false;
     await nextTick();
     expect([
+      "Parent:willRender",
+      "Parent:rendered",
       "Child:willDestroy",
       "Parent:willRender",
       "Parent:rendered",
-      "Parent:willPatch",
-      "Parent:patched",
     ]).toBeLogged();
+    expect(fixture.innerHTML).toBe("1");
+    await nextTick();
+    expect(["Parent:willPatch", "Parent:patched"]).toBeLogged();
     expect(fixture.innerHTML).toBe("2");
   });
 });

tests/components/t_model.test.ts

@@ -626,6 +626,39 @@ describe("t-model directive", () => {
     expect(fixture.querySelector("select")!.value).toEqual("b");
   });
 
+  test("t-model with dynamic number values on select options in foreach", async () => {
+    class Test extends Component {
+      static template = xml`
+          <select t-model.number="state.value">
+            <t t-foreach="state.options" t-as="o" t-key="o.value">
+                <option t-att-value="o.value" t-esc="o.value"/>
+            </t>
+          </select>
+      `;
+      state: any;
+      setup() {
+        this.state = useState({
+          value: 2,
+          options: [{ value: 1 }, { value: 2 }, { value: 3 }],
+        });
+      }
+    }
+
+    const comp = await mount(Test, fixture);
+    // check that we have a value of 2 selected
+    expect(fixture.querySelector("select")!.value).toEqual("2");
+    expect(comp.state.value).toBe(2);
+
+    // emulate a click on the option=3 element
+    fixture.querySelectorAll("option")[2].selected = true;
+    fixture.querySelector("select")!.dispatchEvent(new Event("change"));
+
+    await nextTick();
+    // check that we have now selected the number 3 (and not the string)
+    expect(fixture.querySelector("select")!.value).toEqual("3");
+    expect(comp.state.value).toBe(3);
+  });
+
   test("t-model is applied before t-on-input", async () => {
     expect.assertions(3);
     class SomeComponent extends Component {

tools/devtools/src/devtools_app/devtools_window/components_tab/details_window/object_tree_element/object_tree_element.js

@@ -47,7 +47,7 @@ export class ObjectTreeElement extends Component {
 
   classFor(object) {
     // Prototype items will be dyed down to appear less important
-    if (object.path.some((item) => item?.type === "prototype")) {
+    if (object.path.some((item) => item?.type === "prototype") && !object.keepLit) {
       return "attenuate";
     }
     // Same for subscription items which are not present in the keys while the keys will be bold

tools/devtools/src/devtools_app/store/store.js

@@ -102,7 +102,7 @@ export const store = reactive({
     if (IS_FIREFOX) {
       await evalInWindow("window.$0 = $0;", this.activeFrame);
     }
-    const [apps, component] = await evalFunctionInWindow(
+    const [apps, details] = await evalFunctionInWindow(
       "getComponentsTree",
       fromOld && this.activeComponent
         ? [this.activeComponent.path, this.apps, this.activeComponent]
@@ -113,7 +113,8 @@ export const store = reactive({
     if (!fromOld && this.settings.expandByDefault) {
       this.apps.forEach((tree) => expandNodes(tree, true));
     }
-    this.activeComponent = component;
+    keepEnvLit(details);
+    this.activeComponent = details;
   },
 
   // Select a component by retrieving its details from the page based on its path
@@ -150,9 +151,11 @@ export const store = reactive({
       [component.path],
       this.activeFrame
     );
-    this.activeComponent = details;
-    if (!this.activeComponent) {
+    if (!details) {
       await this.loadComponentsTree(false);
+    } else {
+      keepEnvLit(details);
+      this.activeComponent = details;
     }
     if (this.page !== "ComponentsTab") {
       this.switchTab("ComponentsTab");
@@ -887,6 +890,31 @@ function expandNodes(node, blacklist = false) {
   }
 }
 
+// This function transforms the env part of the details such that all env keys are not
+// greyed out in the UI at their first occurence
+function keepEnvLit(details) {
+  let alreadyMet = new Set();
+  for (let i = 0; i < details.env.children.length; i++) {
+    if (i < details.env.children.length - 1) {
+      alreadyMet.add(details.env.children[i].name);
+    } else {
+      let lastElement = details.env.children[i];
+      while (lastElement.children.at(-1).name === "[[Prototype]]") {
+        for (const [index, child] of lastElement.children.entries()) {
+          if (index < lastElement.children.length - 1) {
+            if (!alreadyMet.has(child.name)) {
+              child.keepLit = true;
+              alreadyMet.add(child.name);
+            }
+          } else {
+            lastElement = child;
+          }
+        }
+      }
+    }
+  }
+}
+
 // Fold the node given in entry and all of its children
 function foldNodes(node) {
   node.toggled = false;

tools/devtools/src/page_scripts/owl_devtools_global_hook.js

@@ -117,6 +117,15 @@
             length += element.length;
             result.push(element);
           }
+          for (const key of Object.getOwnPropertySymbols(obj)) {
+            if (length > 25) {
+              result.push("...");
+              break;
+            }
+            const element = key.toString() + ": " + this.serializeItem(obj[key]);
+            length += element.length;
+            result.push(element);
+          }
           return "{" + result.join(", ") + "}";
         },
         map(obj) {
@@ -739,6 +748,9 @@
           child.contentType = "object";
           child.content = this.serializer.serializeItem(Object.getPrototypeOf(parentObj), true);
           child.hasChildren = true;
+          if (!oldTree && type === "env") {
+            child.toggled = true;
+          }
           break;
         case "set entries":
         case "map entries":
@@ -815,7 +827,8 @@
               break;
             case obj instanceof Object:
               child.contentType = "object";
-              child.hasChildren = Object.keys(obj).length > 0;
+              child.hasChildren =
+                Object.keys(obj).length || Object.getOwnPropertySymbols(obj).length;
               break;
             default:
               child.contentType = typeof obj;
@@ -880,7 +893,7 @@
       const children = [];
       depth = depth + 1;
       let obj = this.getObjectProperty(path);
-      let oldBranch = this.getObjectInOldTree(oldTree, path, objType);
+      let oldBranch = oldTree && this.getObjectInOldTree(oldTree, path, objType);
       if (!obj) {
         return [];
       }
@@ -895,7 +908,7 @@
             depth,
             objType,
             path,
-            oldBranch.children[0],
+            oldBranch?.children[0],
             oldTree
           );
           children.push(mapKey);
@@ -905,7 +918,7 @@
             depth,
             objType,
             path,
-            oldBranch.children[1],
+            oldBranch?.children[1],
             oldTree
           );
           children.push(mapValue);
@@ -916,7 +929,7 @@
             depth,
             objType,
             path,
-            oldBranch.children[0],
+            oldBranch?.children[0],
             oldTree
           );
           children.push(setValue);
@@ -937,7 +950,7 @@
                 depth,
                 objType,
                 path,
-                oldBranch.children[index],
+                oldBranch?.children[index],
                 oldTree
               );
               if (child) {
@@ -952,7 +965,7 @@
                 depth,
                 objType,
                 path,
-                oldBranch.children[index],
+                oldBranch?.children[index],
                 oldTree
               );
               if (child) {
@@ -969,7 +982,7 @@
             depth,
             objType,
             path,
-            oldBranch.children[index],
+            oldBranch?.children[index],
             oldTree
           );
           if (entries) {
@@ -983,7 +996,7 @@
               depth,
               objType,
               path,
-              oldBranch.children[index],
+              oldBranch?.children[index],
               oldTree
             );
             if (child) {
@@ -1018,7 +1031,7 @@
               depth,
               objType,
               path,
-              oldBranch.children[index],
+              oldBranch?.children[index],
               oldTree
             );
             if (child) children.push(child);
@@ -1051,14 +1064,14 @@
         });
         proto = Object.getPrototypeOf(proto);
       }
-      if (!(obj.constructor.name === "Object")) {
+      if (obj.__proto__) {
         prototype = this.serializeObjectChild(
           obj,
           { type: "prototype", childIndex: children.length },
           depth,
           objType,
           path,
-          oldBranch.children.at(-1),
+          oldBranch?.children.at(-1),
           oldTree
         );
         children.push(prototype);
@@ -1433,8 +1446,11 @@
           return;
         }
       }
-      const key = path.pop().value;
+      const item = path.pop();
       const obj = this.getObjectProperty(path);
+      const key = item.hasOwnProperty("symbolIndex")
+        ? Object.getOwnPropertySymbols(obj)[item.symbolIndex]
+        : item.value;
       if (!obj) {
         return;
       }
@@ -1503,8 +1519,9 @@
           }
         }
       }
-      // If nothing was found, return the first app's root component path
-      return ["0", "root"];
+      // If nothing was found, return the path of the first root component found in the apps
+      const appIndex = [...this.apps].findIndex((app) => app.root);
+      return [appIndex.toString(), "root"];
     }
     // Returns the tree of components of the inspected page in a parsed format
     // Use inspectedPath to specify the path of the selected component