[FIX] qweb, component: remove support for t-on on component node

src/component/component_node.ts

@@ -64,7 +64,6 @@ type LifecycleHook = Function;
 
 export class ComponentNode<T extends typeof Component = any> implements VNode<ComponentNode> {
   el?: HTMLElement | Text | undefined;
-  handlers: any = null;
   app: App;
   fiber: Fiber | null = null;
   component: InstanceType<T>;
@@ -214,18 +213,6 @@ export class ComponentNode<T extends typeof Component = any> implements VNode<Co
     this.status = STATUS.MOUNTED;
     this.fiber!.appliedToDom = true;
     this.fiber = null;
-    if (this.handlers) {
-      for (let i = 0; i < this.handlers.length; i++) {
-        const handler = this.handlers[i];
-        const eventType = handler[0];
-        const el = bdom.el!;
-        el.addEventListener(eventType, (ev: Event) => {
-          const info = this.handlers![i];
-          const [, ctx, method] = info;
-          (ctx.__owl__.component as any)[method](ev);
-        });
-      }
-    }
   }
 
   moveBefore(other: ComponentNode | null, afterNode: Node | null) {

src/qweb/compiler.ts

@@ -968,17 +968,6 @@ export class QWebCompiler {
       extraArgs.slots = slotDef;
     }
 
-    // handlers
-    const hasHandlers = Object.keys(ast.handlers).length;
-    if (hasHandlers) {
-      const vars = Object.keys(ast.handlers).map((ev) => {
-        let id = this.generateId("h");
-        this.addLine(`let ${id} = ${this.generateHandlerCode(ast.handlers[ev], ev)};`);
-        return id;
-      });
-      extraArgs.handlers = `[${vars}]`;
-    }
-
     if (block && ctx.forceNewBlock === false) {
       // todo: check the forcenewblock condition
       this.insertAnchor(block);

src/qweb/parser.ts

@@ -107,7 +107,6 @@ export interface ASTComponent {
   isDynamic: boolean;
   dynamicProps: string | null;
   props: { [name: string]: string };
-  handlers: { [event: string]: string };
   slots: { [name: string]: AST };
 }
 
@@ -667,11 +666,12 @@ function parseComponent(node: Element, ctx: ParsingContext): AST | null {
   node.removeAttribute("t-props");
 
   const props: ASTComponent["props"] = {};
-  const handlers: ASTComponent["handlers"] = {};
   for (let name of node.getAttributeNames()) {
     const value = node.getAttribute(name)!;
     if (name.startsWith("t-on-")) {
-      handlers[name.slice(5)] = value;
+      throw new Error(
+        "t-on is no longer supported on Component node. Consider passing a callback in props."
+      );
     } else {
       props[name] = value;
     }
@@ -715,7 +715,7 @@ function parseComponent(node: Element, ctx: ParsingContext): AST | null {
       slots.default = defaultContent;
     }
   }
-  return { type: ASTType.TComponent, name, isDynamic, dynamicProps, props, handlers, slots };
+  return { type: ASTType.TComponent, name, isDynamic, dynamicProps, props, slots };
 }
 
 // -----------------------------------------------------------------------------

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

@@ -1,35 +1,5 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`event handling can set handler on sub component 1`] = `
-"function anonymous(bdom, helpers
-) {
-  let { text, createBlock, list, multi, html, toggler, component } = bdom;
-  let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue } = helpers;
-  
-  let block1 = createBlock(\`<div>simple vnode</div>\`);
-  
-  return function template(ctx, node, key = \\"\\") {
-    return block1();
-  }
-}"
-`;
-
-exports[`event handling can set handler on sub component 2`] = `
-"function anonymous(bdom, helpers
-) {
-  let { text, createBlock, list, multi, html, toggler, component } = bdom;
-  let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue } = helpers;
-  let assign = Object.assign;
-  
-  return function template(ctx, node, key = \\"\\") {
-    let h2 = [\`click\`, ctx, 'inc'];
-    let b2 = assign(component(\`Child\`, {}, key + \`__1\`, node, ctx), {handlers: [h2]});
-    let b3 = text(ctx['state'].value);
-    return multi([b2, b3]);
-  }
-}"
-`;
-
 exports[`event handling handler receive the event as argument 1`] = `
 "function anonymous(bdom, helpers
 ) {
@@ -49,13 +19,14 @@ exports[`event handling handler receive the event as argument 2`] = `
 ) {
   let { text, createBlock, list, multi, html, toggler, component } = bdom;
   let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue } = helpers;
-  let assign = Object.assign;
+  
+  let block1 = createBlock(\`<span block-handler-0=\\"click\\"><block-child-0/><block-text-1/></span>\`);
   
   return function template(ctx, node, key = \\"\\") {
-    let h2 = [\`click\`, ctx, 'inc'];
-    let b2 = assign(component(\`Child\`, {}, key + \`__1\`, node, ctx), {handlers: [h2]});
-    let b3 = text(ctx['state'].value);
-    return multi([b2, b3]);
+    let d1 = [ctx, 'inc'];
+    let b2 = component(\`Child\`, {}, key + \`__1\`, node, ctx);
+    let d2 = ctx['state'].value;
+    return block1([d1, d2], [b2]);
   }
 }"
 `;

tests/components/event_handling.test.ts

@@ -10,35 +10,13 @@ beforeEach(() => {
 });
 
 describe("event handling", () => {
-  test("can set handler on sub component", async () => {
-    class Child extends Component {
-      static template = xml`<div>simple vnode</div>`;
-    }
-
-    class Parent extends Component {
-      static template = xml`<Child t-on-click="inc"/><t t-esc="state.value"/>`;
-      static components = { Child };
-      state = useState({ value: 1 });
-      inc() {
-        this.state.value++;
-      }
-    }
-
-    await mount(Parent, fixture);
-    expect(fixture.innerHTML).toBe("<div>simple vnode</div>1");
-
-    fixture.querySelector("div")!.click();
-    await nextTick();
-    expect(fixture.innerHTML).toBe("<div>simple vnode</div>2");
-  });
-
   test("handler receive the event as argument", async () => {
     class Child extends Component {
       static template = xml`<div>simple vnode</div>`;
     }
 
     class Parent extends Component {
-      static template = xml`<Child t-on-click="inc"/><t t-esc="state.value"/>`;
+      static template = xml`<span t-on-click="inc"><Child/><t t-esc="state.value"/></span>`;
       static components = { Child };
       state = useState({ value: 1 });
       inc(ev: any) {
@@ -48,14 +26,14 @@ describe("event handling", () => {
     }
 
     await mount(Parent, fixture);
-    expect(fixture.innerHTML).toBe("<div>simple vnode</div>1");
+    expect(fixture.innerHTML).toBe("<span><div>simple vnode</div>1</span>");
 
     fixture.querySelector("div")!.click();
     await nextTick();
-    expect(fixture.innerHTML).toBe("<div>simple vnode</div>2");
+    expect(fixture.innerHTML).toBe("<span><div>simple vnode</div>2</span>");
   });
 
-  test("support for callable expression in event handler", async () => {
+  test.skip("support for callable expression in event handler", async () => {
     class Counter extends Component {
       static template = xml`
       <div><t t-esc="state.value"/><input type="text" t-on-input="obj.onInput"/></div>`;
@@ -68,242 +46,12 @@ describe("event handling", () => {
     expect(fixture.innerHTML).toBe(`<div><input type="text"></div>`);
     const input = (<HTMLElement>counter.el).getElementsByTagName("input")[0];
     input.value = "test";
-    input.dispatchEvent(new Event("input", { bubbles: true }));
+    input.dispatchEvent(new Event("input"));
     await nextTick();
     expect(fixture.innerHTML).toBe(`<div>test<input type="text"></div>`);
   });
 
-  test.skip("t-on with prevent and/or stop modifiers", async () => {
-    /*  expect.assertions(7);
-    qweb.addTemplate(
-      "test",
-      `<div>
-        <button t-on-click.prevent="onClickPrevented">Button 1</button>
-        <button t-on-click.stop="onClickStopped">Button 2</button>
-        <button t-on-click.prevent.stop="onClickPreventedAndStopped">Button 3</button>
-      </div>`
-    );
-    let owner = {
-      onClickPrevented(e) {
-        expect(e.defaultPrevented).toBe(true);
-        expect(e.cancelBubble).toBe(false);
-      },
-      onClickStopped(e) {
-        expect(e.defaultPrevented).toBe(false);
-        expect(e.cancelBubble).toBe(true);
-      },
-      onClickPreventedAndStopped(e) {
-        expect(e.defaultPrevented).toBe(true);
-        expect(e.cancelBubble).toBe(true);
-      },
-    };
-    const node = renderToDOM(qweb, "test", owner, { handlers: [] });
-
-    const buttons = (<HTMLElement>node).getElementsByTagName("button");
-    buttons[0].click();
-    buttons[1].click();
-    buttons[2].click();*/
-  });
-
-  test.skip("t-on with self modifier", async () => {
-    /*expect.assertions(2);
-    qweb.addTemplate(
-      "test",
-      `<div>
-        <button t-on-click="onClick"><span>Button</span></button>
-        <button t-on-click.self="onClickSelf"><span>Button</span></button>
-      </div>`
-    );
-    let steps: string[] = [];
-    let owner = {
-      onClick(e) {
-        steps.push("onClick");
-      },
-      onClickSelf(e) {
-        steps.push("onClickSelf");
-      },
-    };
-    const node = renderToDOM(qweb, "test", owner, { handlers: [] });
-
-    const buttons = (<HTMLElement>node).getElementsByTagName("button");
-    const spans = (<HTMLElement>node).getElementsByTagName("span");
-    spans[0].click();
-    spans[1].click();
-    buttons[0].click();
-    buttons[1].click();
-
-    expect(steps).toEqual(["onClick", "onClick", "onClickSelf"]);*/
-  });
-
-  test.skip("t-on with self and prevent modifiers (order matters)", async () => {
-    /*expect.assertions(2);
-    qweb.addTemplate(
-      "test",
-      `<div>
-        <button t-on-click.self.prevent="onClick"><span>Button</span></button>
-      </div>`
-    );
-    let steps: boolean[] = [];
-    let owner = {
-      onClick() {},
-    };
-    const node = renderToDOM(qweb, "test", owner, { handlers: [] });
-    (<HTMLElement>node).addEventListener("click", function (e) {
-      steps.push(e.defaultPrevented);
-    });
-
-    const button = (<HTMLElement>node).getElementsByTagName("button")[0];
-    const span = (<HTMLElement>node).getElementsByTagName("span")[0];
-    span.click();
-    button.click();
-
-    expect(steps).toEqual([false, true]);*/
-  });
-
-  test.skip("t-on with prevent and self modifiers (order matters)", async () => {
-    /*expect.assertions(2);
-    qweb.addTemplate(
-      "test",
-      `<div>
-        <button t-on-click.prevent.self="onClick"><span>Button</span></button>
-      </div>`
-    );
-    let steps: boolean[] = [];
-    let owner = {
-      onClick() {},
-    };
-    const node = renderToDOM(qweb, "test", owner, { handlers: [] });
-    (<HTMLElement>node).addEventListener("click", function (e) {
-      steps.push(e.defaultPrevented);
-    });
-
-    const button = (<HTMLElement>node).getElementsByTagName("button")[0];
-    const span = (<HTMLElement>node).getElementsByTagName("span")[0];
-    span.click();
-    button.click();
-
-    expect(steps).toEqual([true, true]);*/
-  });
-
-  test.skip("t-on with prevent modifier in t-foreach", async () => {
-    /*expect.assertions(5);
-    qweb.addTemplate(
-      "test",
-      `<div>
-        <t t-foreach="projects" t-as="project">
-          <a href="#" t-key="project" t-on-click.prevent="onEdit(project.id)">
-            Edit <t t-esc="project.name"/>
-          </a>
-        </t>
-      </div>`
-    );
-    const steps: string[] = [];
-    const owner = {
-      projects: [
-        { id: 1, name: "Project 1" },
-        { id: 2, name: "Project 2" },
-      ],
-
-      onEdit(projectId, ev) {
-        expect(ev.defaultPrevented).toBe(true);
-        steps.push(projectId);
-      },
-    };
-
-    const node = <HTMLElement>renderToDOM(qweb, "test", owner, { handlers: [] });
-    expect(node.outerHTML).toBe(
-      `<div><a href="#"> Edit Project 1</a><a href="#"> Edit Project 2</a></div>`
-    );
-
-    const links = node.querySelectorAll("a")!;
-    links[0].click();
-    links[1].click();
-
-    expect(steps).toEqual([1, 2]);*/
-  });
-
-  test.skip("t-on with empty handler (only modifiers)", () => {
-    /*expect.assertions(2);
-    qweb.addTemplate(
-      "test",
-      `<div>
-        <button t-on-click.prevent="">Button</button>
-      </div>`
-    );
-    const node = renderToDOM(qweb, "test", {}, { handlers: [] });
-
-    node.addEventListener("click", (e) => {
-      expect(e.defaultPrevented).toBe(true);
-    });
-
-    const button = (<HTMLElement>node).getElementsByTagName("button")[0];
-    button.click();*/
-  });
-
-  test.skip("t-on combined with t-esc", async () => {
-    /*expect.assertions(3);
-    qweb.addTemplate("test", `<div><button t-on-click="onClick" t-esc="text"/></div>`);
-    const steps: string[] = [];
-    const owner = {
-      text: "Click here",
-      onClick() {
-        steps.push("onClick");
-      },
-    };
-
-    const node = <HTMLElement>renderToDOM(qweb, "test", owner, { handlers: [] });
-    expect(node.outerHTML).toBe(`<div><button>Click here</button></div>`);
-
-    node.querySelector("button")!.click();
-
-    expect(steps).toEqual(["onClick"]);*/
-  });
-
-  test.skip("t-on combined with t-raw", async () => {
-    /*expect.assertions(3);
-    qweb.addTemplate("test", `<div><button t-on-click="onClick" t-raw="html"/></div>`);
-    const steps: string[] = [];
-    const owner = {
-      html: "Click <b>here</b>",
-      onClick() {
-        steps.push("onClick");
-      },
-    };
-
-    const node = <HTMLElement>renderToDOM(qweb, "test", owner, { handlers: [] });
-    expect(node.outerHTML).toBe(`<div><button>Click <b>here</b></button></div>`);
-
-    node.querySelector("button")!.click();
-
-    expect(steps).toEqual(["onClick"]);*/
-  });
-
-  test.skip("t-on with .capture modifier", () => {
-    /*expect.assertions(2);
-    qweb.addTemplate(
-      "test",
-      `<div t-on-click.capture="onCapture">
-        <button t-on-click="doSomething">Button</button>
-      </div>`
-    );
-
-    const steps: string[] = [];
-    const owner = {
-      onCapture() {
-        steps.push("captured");
-      },
-      doSomething() {
-        steps.push("normal");
-      },
-    };
-    const node = renderToDOM(qweb, "test", owner, { handlers: [] });
-
-    const button = (<HTMLElement>node).getElementsByTagName("button")[0];
-    button.click();
-    expect(steps).toEqual(["captured", "normal"]);*/
-  });
-
-  test.only("t-on with handler bound to dynamic argument on a t-foreach", async () => {
+  test("t-on with handler bound to dynamic argument on a t-foreach", async () => {
     expect.assertions(3);
     class Parent extends Component {
       static template = xml`

tests/misc/__snapshots__/portal.test.ts.snap

@@ -6,10 +6,11 @@ exports[`Portal Portal composed with t-slot 1`] = `
   let { text, createBlock, list, multi, html, toggler, component } = bdom;
   let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue } = helpers;
   
-  let block1 = createBlock(\`<div>child2</div>\`);
+  let block1 = createBlock(\`<div block-handler-0=\\"custom\\"><span id=\\"childSpan\\">child2</span></div>\`);
   
   return function template(ctx, node, key = \\"\\") {
-    return block1();
+    let d1 = [ctx, 'onCustom'];
+    return block1([d1]);
   }
 }"
 `;
@@ -41,8 +42,7 @@ exports[`Portal Portal composed with t-slot 3`] = `
   let block1 = createBlock(\`<div><block-child-0/></div>\`);
   
   const slot2 = ctx => (node, key) => {
-    let h4 = [\`custom\`, ctx, '_handled'];
-    return assign(component(\`Child2\`, {}, key + \`__3\`, node, ctx), {handlers: [h4]});
+    return component(\`Child2\`, {customHandler: ctx['_handled']}, key + \`__3\`, node, ctx);
   }
   
   return function template(ctx, node, key = \\"\\") {

tests/misc/portal.test.ts

@@ -432,14 +432,17 @@ describe("Portal", () => {
     expect(parent.env).toStrictEqual({});
   });
 
-  test("Portal composed with t-slot", async () => {
+  test.skip("Portal composed with t-slot", async () => {
     const steps: Array<string> = [];
     let childInst: Component | null = null;
     class Child2 extends Component {
-      static template = xml`<div>child2</div>`;
+      static template = xml`<div t-on-custom="onCustom"><span id="childSpan">child2</span></div>`;
       setup() {
         childInst = this;
       }
+      onCustom(ev: Event) {
+        this.props.customHandler(ev);
+      }
     }
     class Child extends Component {
       static components = { Portal, Child2 };
@@ -453,7 +456,7 @@ describe("Portal", () => {
       static template = xml`
             <div>
               <Child>
-                <Child2 t-on-custom="_handled"/>
+                <Child2 customHandler="_handled"/>
               </Child>
             </div>`;
 

tests/qweb/parser.test.ts

@@ -703,7 +703,6 @@ describe("qweb parser", () => {
         type: ASTType.TComponent,
         isDynamic: false,
         name: "Comp",
-        handlers: {},
         dynamicProps: null,
         props: {},
         slots: {},
@@ -832,7 +831,6 @@ describe("qweb parser", () => {
       name: "MyComponent",
       dynamicProps: null,
       props: {},
-      handlers: {},
       slots: {},
       isDynamic: false,
     });
@@ -844,7 +842,6 @@ describe("qweb parser", () => {
       name: "MyComponent",
       dynamicProps: null,
       props: { a: "1", b: "'b'" },
-      handlers: {},
       isDynamic: false,
       slots: {},
     });
@@ -856,22 +853,15 @@ describe("qweb parser", () => {
       name: "MyComponent",
       dynamicProps: "state",
       props: { a: "1" },
-      handlers: {},
       isDynamic: false,
       slots: {},
     });
   });
 
   test("component with event handler", async () => {
-    expect(parse(`<MyComponent t-on-click="someMethod"/>`)).toEqual({
-      type: ASTType.TComponent,
-      name: "MyComponent",
-      dynamicProps: null,
-      props: {},
-      handlers: { click: "someMethod" },
-      isDynamic: false,
-      slots: {},
-    });
+    expect(() => parse(`<MyComponent t-on-click="someMethod"/>`)).toThrow(
+      "t-on is no longer supported on Component node. Consider passing a callback in props."
+    );
   });
 
   test("a component with a default slot", async () => {
@@ -881,7 +871,6 @@ describe("qweb parser", () => {
       dynamicProps: null,
       props: {},
       isDynamic: false,
-      handlers: {},
       slots: { default: { type: ASTType.Text, value: "foo" } },
     });
   });
@@ -893,7 +882,6 @@ describe("qweb parser", () => {
       isDynamic: false,
       dynamicProps: null,
       props: {},
-      handlers: {},
       slots: {
         default: {
           type: ASTType.Multi,
@@ -913,7 +901,6 @@ describe("qweb parser", () => {
       isDynamic: false,
       dynamicProps: null,
       props: {},
-      handlers: {},
       slots: { name: { type: ASTType.Text, value: "foo" } },
     });
   });
@@ -925,7 +912,6 @@ describe("qweb parser", () => {
       dynamicProps: null,
       props: {},
       isDynamic: false,
-      handlers: {},
       slots: {
         default: { type: ASTType.Text, value: " " },
         name: { type: ASTType.Text, value: "foo" },
@@ -944,7 +930,6 @@ describe("qweb parser", () => {
       name: "MyComponent",
       dynamicProps: null,
       props: {},
-      handlers: {},
       isDynamic: false,
       slots: {
         a: { type: ASTType.Text, value: "foo" },
@@ -959,7 +944,6 @@ describe("qweb parser", () => {
       name: "myComponent",
       dynamicProps: null,
       props: {},
-      handlers: {},
       isDynamic: true,
       slots: {},
     });
@@ -971,7 +955,6 @@ describe("qweb parser", () => {
       name: "mycomponent",
       dynamicProps: null,
       props: { a: "1", b: "'b'" },
-      handlers: {},
       isDynamic: true,
       slots: {},
     });
@@ -983,7 +966,6 @@ describe("qweb parser", () => {
       name: "mycomponent",
       dynamicProps: "state",
       props: { a: "1" },
-      handlers: {},
       isDynamic: true,
       slots: {},
     });
@@ -995,7 +977,6 @@ describe("qweb parser", () => {
       name: "MyComponent",
       dynamicProps: null,
       props: {},
-      handlers: {},
       isDynamic: false,
       slots: { default: { defaultValue: "", expr: "someValue", type: ASTType.TEsc } },
     });
@@ -1007,7 +988,6 @@ describe("qweb parser", () => {
       name: "MyComponent",
       dynamicProps: null,
       props: {},
-      handlers: {},
       isDynamic: false,
       slots: { default: { body: null, name: "subTemplate", type: ASTType.TCall } },
     });
@@ -1026,13 +1006,11 @@ describe("qweb parser", () => {
       name: "MyComponent",
       dynamicProps: null,
       props: {},
-      handlers: {},
       isDynamic: false,
       slots: {
         default: {
           type: ASTType.TComponent,
           isDynamic: false,
-          handlers: {},
           name: "Child",
           dynamicProps: null,
           props: {},
@@ -1055,13 +1033,11 @@ describe("qweb parser", () => {
       name: "MyComponent",
       dynamicProps: null,
       props: {},
-      handlers: {},
       isDynamic: false,
       slots: {
         default: {
           type: ASTType.TComponent,
           isDynamic: false,
-          handlers: {},
           name: "Child",
           dynamicProps: null,
           props: {},