[REL] v2.0.1

# v2.0.1 - fix: runtime: correctly throw an error for duplicate object keys - fix: parser: give t-set-slot="default" priority over the content - fix: blockdom: correctly reorder children in heterogeneous t-foreach - fix: portal: correctly move portal content when target is after it - fix: blockdom: fix event_catcher traceback when a parent component has an empty child
[FIX] blockdom: fix event_catcher traceback
2025-10-06 19:59:41 +07:00 · 2022-10-21 10:00:06 +02:00 · 2022-10-21 09:50:10 +02:00 · 2022-10-20 09:24:08 +02:00 · 2022-10-19 13:13:49 +02:00
13 changed files with 104 additions and 30 deletions
@@ -1,6 +1,6 @@
 {
  "name": "@odoo/owl",
-  "version": "2.0.0",
+  "version": "2.0.1",
  "description": "Odoo Web Library (OWL)",
  "main": "dist/owl.cjs.js",
  "module": "dist/owl.es.js",
@@ -517,8 +517,9 @@ function createBlockClass(template: HTMLElement, ctx: BlockCtx): BlockClass {
      return this.el!;
    }

-    moveBeforeDOMNode(node: Node | null) {
-      nodeInsertBefore.call(this.parentEl, this.el!, node);
+    moveBeforeDOMNode(node: Node | null, parent = this.parentEl) {
+      this.parentEl = parent;
+      nodeInsertBefore.call(parent, this.el!, node);
    }

    moveBeforeVNode(other: Block | null, afterNode: Node | null) {
@@ -46,7 +46,7 @@ export function createCatcher(eventsSpec: EventsSpec): Catcher {
          const target = ev.target;
          let currentNode: any = self.child.firstNode();
          const afterNode = self.afterNode;
-          while (currentNode !== afterNode) {
+          while (currentNode && currentNode !== afterNode) {
            if (currentNode.contains(target)) {
              return origFn.call(this, ev);
            }
@@ -56,9 +56,10 @@ export function createCatcher(eventsSpec: EventsSpec): Catcher {
      }
    }

-    moveBeforeDOMNode(node: Node | null) {
-      this.child.moveBeforeDOMNode(node);
-      this.parentEl!.insertBefore(this.afterNode!, node);
+    moveBeforeDOMNode(node: Node | null, parent = this.parentEl) {
+      this.parentEl = parent;
+      this.child.moveBeforeDOMNode(node, parent);
+      parent!.insertBefore(this.afterNode!, node);
    }

    moveBeforeVNode(other: VCatcher | null, afterNode: Node | null) {
@@ -29,8 +29,8 @@ class VHtml {
    }
  }

-  moveBeforeDOMNode(node: Node | null) {
-    const parent = this.parentEl;
+  moveBeforeDOMNode(node: Node | null, parent = this.parentEl) {
+    this.parentEl = parent;
    for (let elem of this.content) {
      nodeInsertBefore.call(parent, elem, node);
    }
@@ -10,7 +10,7 @@ export { createCatcher } from "./event_catcher";

 export interface VNode<T = any> {
  mount(parent: HTMLElement, afterNode: Node | null): void;
-  moveBeforeDOMNode(node: Node | null): void;
+  moveBeforeDOMNode(node: Node | null, parent?: HTMLElement): void;
  moveBeforeVNode(other: T | null, afterNode: Node | null): void;
  patch(other: T, withBeforeRemove: boolean): void;
  beforeRemove(): void;
@@ -38,12 +38,13 @@ class VList {
    this.parentEl = parent;
  }

-  moveBeforeDOMNode(node: Node | null) {
+  moveBeforeDOMNode(node: Node | null, parent = this.parentEl) {
+    this.parentEl = parent;
    const children = this.children;
    for (let i = 0, l = children.length; i < l; i++) {
-      children[i].moveBeforeDOMNode(node);
+      children[i].moveBeforeDOMNode(node, parent);
    }
-    this.parentEl!.insertBefore(this.anchor!, node);
+    parent!.insertBefore(this.anchor!, node);
  }

  moveBeforeVNode(other: VList | null, afterNode: Node | null) {
@@ -38,14 +38,14 @@ export class VMulti {
    this.parentEl = parent;
  }

-  moveBeforeDOMNode(node: Node | null) {
+  moveBeforeDOMNode(node: Node | null, parent = this.parentEl) {
+    this.parentEl = parent;
    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) {
-        child.moveBeforeDOMNode(node);
+        child.moveBeforeDOMNode(node, parent);
      } else {
        const anchor = anchors![i];
        nodeInsertBefore.call(parent, anchor, node);
@@ -23,8 +23,9 @@ abstract class VSimpleNode {
    this.el = node;
  }

-  moveBeforeDOMNode(node: Node | null) {
-    nodeInsertBefore.call(this.parentEl, this.el!, node);
+  moveBeforeDOMNode(node: Node | null, parent = this.parentEl) {
+    this.parentEl = parent;
+    nodeInsertBefore.call(parent, this.el!, node);
  }

  moveBeforeVNode(other: VText | null, afterNode: Node | null) {
@@ -20,8 +20,8 @@ class VToggler {
    this.child.mount(parent, afterNode);
  }

-  moveBeforeDOMNode(node: Node | null) {
-    this.child.moveBeforeDOMNode(node);
+  moveBeforeDOMNode(node: Node | null, parent?: HTMLElement) {
+    this.child.moveBeforeDOMNode(node, parent);
  }

  moveBeforeVNode(other: VToggler | null, afterNode: Node | null) {
@@ -306,8 +306,8 @@ export class ComponentNode<P extends Props = any, E = any> implements VNode<Comp
    this.fiber = null;
  }

-  moveBeforeDOMNode(node: Node | null): void {
-    this.bdom!.moveBeforeDOMNode(node);
+  moveBeforeDOMNode(node: Node | null, parent?: HTMLElement): void {
+    this.bdom!.moveBeforeDOMNode(node, parent);
  }

  moveBeforeVNode(other: ComponentNode<P, E> | null, afterNode: Node | null) {
@@ -21,16 +21,20 @@ class VPortal extends VText implements Partial<VNode<VPortal>> {
    this.target = document.querySelector(this.selector) as any;
    if (this.target) {
      this.content!.mount(this.target!, null);
+    } else {
+      this.content!.mount(parent, anchor);
    }
  }

  beforeRemove() {
-    // this.target not being null means content is mounted
-    if (this.target) {
-      this.content!.beforeRemove();
+    this.content!.beforeRemove();
+  }
+  remove() {
+    if (this.content) {
+      super.remove();
      this.content!.remove();
+      this.content = null;
    }
-    this.content = null;
  }

  patch(other: VPortal) {
@@ -69,9 +73,9 @@ export class Portal extends Component {
    onMounted(() => {
      const portal: VPortal = node.bdom;
      if (!portal.target) {
-        portal.target = document.querySelector(this.props.target);
-        if (portal.target) {
-          portal.content!.mount(portal.target, null);
+        const target: HTMLElement = document.querySelector(this.props.target);
+        if (target) {
+          portal.content!.moveBeforeDOMNode(target.firstChild, target);
        } else {
          throw new OwlError("invalid portal target");
        }
@@ -80,7 +84,7 @@ export class Portal extends Component {

    onWillUnmount(() => {
      const portal: VPortal = node.bdom;
-      portal.beforeRemove();
+      portal.remove();
    });
  }
 }
@@ -456,3 +456,45 @@ exports[`t-on t-on on t-slots 2`] = `
  }
 }"
 `;
+
+exports[`t-on t-on when first component child is an empty component 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  let { createCatcher } = helpers;
+  const comp1 = app.createComponent(\`Child\`, true, false, false, false);
+  const catcher1 = createCatcher({\\"click\\":0});
+  
+  let block1 = createBlock(\`<div block-handler-0=\\"click\\"><block-child-0/></div>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let hdlr1 = [ctx['push'], ctx];
+    const hdlr2 = [()=>{}, ctx];
+    const b2 = catcher1(comp1({list: ctx['list']}, key + \`__1\`, node, this, null), [hdlr2]);
+    return block1([hdlr1], [b2]);
+  }
+}"
+`;
+
+exports[`t-on t-on when first component child is an empty component 2`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  let { prepareList, withKey } = helpers;
+  
+  let block2 = createBlock(\`<span><block-text-0/></span>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    ctx = Object.create(ctx);
+    const [k_block1, v_block1, l_block1, c_block1] = prepareList(ctx['props'].list);;
+    for (let i1 = 0; i1 < l_block1; i1++) {
+      ctx[\`c\`] = v_block1[i1];
+      ctx[\`c_index\`] = i1;
+      const key1 = ctx['c_index'];
+      let txt1 = ctx['c'];
+      c_block1[i1] = withKey(block2([txt1]), key1);
+    }
+    return list(c_block1);
+  }
+}"
+`;
@@ -41,6 +41,30 @@ describe("t-on", () => {
    expect(steps).toEqual(["click"]);
  });

+  test("t-on when first component child is an empty component", async () => {
+    class Child extends Component {
+      static template = xml`
+        <span t-foreach="props.list" t-as="c" t-key="c_index" t-esc="c"/>
+      `;
+    }
+    class Parent extends Component {
+      static template = xml`
+        <div t-on-click="push"><Child list="list" t-on-click="() => {}"/></div>
+      `;
+      static components = { Child };
+      list = useState([] as string[]);
+      push() {
+        this.list.push("foo");
+      }
+    }
+    const parent = await mount(Parent, fixture);
+    const el = elem(parent);
+    expect(el.innerHTML).toBe("");
+    el.click();
+    await nextTick();
+    expect(el.innerHTML).toBe("<span>foo</span>");
+  });
+
  test("t-on expression in t-foreach", async () => {
    class Comp extends Component {
      static template = xml`
Author	SHA1	Message	Date
Bruno Boi	9fe8e93980	[REL] v2.0.1 # v2.0.1 - fix: runtime: correctly throw an error for duplicate object keys - fix: parser: give t-set-slot="default" priority over the content - fix: blockdom: correctly reorder children in heterogeneous t-foreach - fix: portal: correctly move portal content when target is after it - fix: blockdom: fix event_catcher traceback when a parent component has an empty child	2022-10-21 10:00:06 +02:00
Bruno Boi	bd199971bd	[FIX] blockdom: fix event_catcher traceback When a parent component has an empty child with a t-on and an event is triggered inside the parent, blockdom checks if there is an event_catcher to call. Doing so, an empty child would cause an error. This commit fixes that.	2022-10-21 09:50:10 +02:00
Samuel Degueldre	6f23b18cab	[FIX] portal: correctly move portal content when target is after it Previously, when trying to mount a portal into a target that would be mounted in the same render as the portal itself, the portal content could not be mounted correctly. In a previous attempt to fix it, a mistake was made while writing the test causing us to incorrectly believe in now worked, when in fact, it would just move the portal content to the end of its parent but without changing it. This commit fixes the issue by making the `moveBeforeDOMNode` method of VNodes accept a second optional parameter which is the parent in which the element should be moved, defaulting to the current parent. This results in identical behaviour when a parent is not specified, but when it is, the VNode is "reparented" to the passed parent, which is what is now done by the Portal	2022-10-20 09:24:08 +02:00
Samuel Degueldre	2c244aa31a	[FIX] blockdom: correctly reorder children in heterogeneous t-foreach Currently, the `moveBefore` method on VNodes assumes that the `other` VNode it receives is of the same type, and that the entire VNode tree below that other VNode has the exact same structure. While this is correct in most cases, it breaks down when there is a VToggler somewhere in the VNode tree, as the structure below a VToggler can be very different from the structure below another VToggler that was created from the same compiled code. For example, two iterations of a t-foreach that contains a <t t-component="..."/> may spawn different components, and different components obviously have different structures. One way to fix this is to remove the assumption that the structure of the `this` block tree in moveBefore is the same as the structure of the `other` block tree, and instead, always give the concrete DOM node before which we want to move the current VNode instead of giving it a VNode and an afterNode as a fallback. One problem with this solution is that it degrades performance in the "standard" case, where a t-foreach contains no VToggler anywhere in its block tree, as retrieving the first concrete DOM node requires calling firstNode() which recursively traverses the entire tree. To avoid this performance penalty in the standard case, we opt to only go down this route whenever we encounter a VToggler when calling `moveBefore`. This requires that we maintain two separate methods, one to move a VNode before another VNode of assumed similar structure, which is basically the current implementation of `moveBefore` for all VNode types except VToggler, and one implementation that moves a VNode before a concrete DOM node. This method needs to be implemented for all VNode types, as all VNode types can be descendants of a VToggler. This method will only be called from one place: the `moveBeforeVNode` method of the toggler, which is the point where we realize that the assumption of identical structure breaks down. Co-authored-by: Bruno Boi <boi@odoo.com>	2022-10-19 13:13:49 +02:00