[IMP] component: add support for t-on on compnents

CHANGELOG.md

@@ -44,7 +44,6 @@ All changes are documented here in no particular order.
 - breaking: style/class on components are now regular props ([details](#10-styleclass-on-components-are-now-regular-props))
 - new: components can use the `.bind` suffix to bind function props ([doc](doc/reference/props.md#binding-function-props))
 - breaking: `t-on` does not accept expressions, only functions ([details](#30-t-on-does-not-accept-expressions-only-functions))
-- breaking: `t-on` does not work on components any more ([details](#12-t-on-does-not-work-on-components-any-more))
 - new: an error is thrown if an handler defined in a `t-on-` directive is not a function (failed silently previously in some cases)
 - breaking: `t-component` no longer accepts strings ([details](#17-t-component-no-longer-accepts-strings))
 - new: the `this` variable in template expressions is now bound to the component
@@ -313,41 +312,6 @@ Documentation:
 - [Mounting a component](doc/reference/app.md#mount-helper)
 
 
-### 12. `t-on` does not work on components any more
-
-In owl 1, it was possible to bind an event listener on a component tag in a
-template:
-
-```xml
-<SomeComponent t-on-some-event="doSomething"/>
-```
-
-This does not work any more.
-
-Rationale: with the support of fragments, there is no longer a canonical html
-element that we can refer. So, this makes it difficult to implement correctly
-and efficiently. Also, we noticed in practice that the event system was an issue
-in some cases, when components need to communicate before they are mounted. In
-those cases, the better solution is to directly use a callback.  Also, another
-conceptual issue with this is that it kind of breaks the component encapsulation.
-The child component kind of leak its own implementation to the outside world.
-
-Migration: a quick fix that may work in some cases is to simply bind the event
-handler on a parent htmlelement. A better way to do it, if possible, is to change
-the component API to accept explicitely a callback as props. 
-
-```xml
-<SomeComponent onSomeEvent="doSomething"/>
-<!-- or alternatively: -->
-<SomeComponent onSomeEvent.bind="doSomething"/>
-```
-
-Note that one of the example above uses the `.bind` suffix, to bind the function
-prop to the component.  Most of the time, binding the function is necessary, and
-using the `.bind` suffix is very helpful in that case.
-
-Documentation: [Binding function props](doc/reference/props.md#binding-function-props)
-
 ### 13. Portal does no longer transfer DOM events
 
 In Owl 1, a Portal component would listen to events emitted on its portalled

doc/reference/event_handling.md

@@ -5,6 +5,7 @@
 - [Event Handling](#event-handling)
 - [Modifiers](#modifiers)
 - [Synthetic Events](#synthetic-events)
+- [On Components](#on-components)
 
 ## Event Handling
 
@@ -100,3 +101,19 @@ To enable it, one can just use the `.synthetic` suffix:
     </t>
 </div>
 ```
+
+## On Components
+
+The `t-on` directive also works on a child component:
+
+```xml
+<div>
+    in some template
+    <Child t-on-click="dosomething"/>
+</div>
+```
+
+This will catch all click events on any html element contained in the `Child`
+sub component. Note that if the child component is reduced to one (or more) text
+nodes, then clicking on it will not call the handler, since the event will be
+dispatched by the browser on the parent element (a `div` in this case).

src/app/template_helpers.ts

@@ -1,4 +1,4 @@
-import { BDom, multi, text, toggler } from "../blockdom";
+import { BDom, multi, text, toggler, createCatcher } from "../blockdom";
 import { validateProps } from "../component/props_validation";
 import { Markup } from "../utils";
 import { html } from "../blockdom/index";
@@ -199,4 +199,5 @@ export const helpers = {
   LazyValue,
   safeOutput,
   bind,
+  createCatcher,
 };

src/blockdom/event_catcher.ts

@@ -0,0 +1,89 @@
+import { createEventHandler } from "./events";
+import type { VNode } from "./index";
+
+type EventsSpec = { [name: string]: number };
+
+type Catcher = (child: VNode, handlers: any[]) => VNode;
+
+export function createCatcher(eventsSpec: EventsSpec): Catcher {
+  let setupFns: any[] = [];
+  let removeFns: any[] = [];
+  for (let name in eventsSpec) {
+    let index = eventsSpec[name];
+    let { setup, remove } = createEventHandler(name);
+    setupFns[index] = setup;
+    removeFns[index] = remove;
+  }
+  let n = setupFns.length;
+
+  class VCatcher {
+    child: VNode;
+    handlers: any[];
+
+    parentEl?: HTMLElement | undefined;
+    afterNode: Node | null = null;
+
+    constructor(child: VNode, handlers: any[]) {
+      this.child = child;
+      this.handlers = handlers;
+    }
+
+    mount(parent: HTMLElement, afterNode: Node | null) {
+      this.parentEl = parent;
+      this.afterNode = afterNode;
+      this.child.mount(parent, afterNode);
+      for (let i = 0; i < n; i++) {
+        let origFn = this.handlers[i][0];
+        const self = this;
+        this.handlers[i][0] = function (ev: any) {
+          const target = ev.target;
+          let currentNode: any = self.child.firstNode();
+          const afterNode = self.afterNode;
+          while (currentNode !== afterNode) {
+            if (currentNode.contains(target)) {
+              return origFn.call(this, ev);
+            }
+            currentNode = currentNode.nextSibling;
+          }
+        };
+        setupFns[i].call(parent, this.handlers[i]);
+      }
+    }
+
+    moveBefore(other: VCatcher | null, afterNode: Node | null) {
+      this.afterNode = null;
+      this.child.moveBefore(other ? other.child : null, afterNode);
+    }
+
+    patch(other: VCatcher, withBeforeRemove: boolean) {
+      if (this === other) {
+        return;
+      }
+      this.handlers = other.handlers;
+      this.child.patch(other.child, withBeforeRemove);
+    }
+
+    beforeRemove() {
+      this.child.beforeRemove();
+    }
+
+    remove() {
+      for (let i = 0; i < n; i++) {
+        removeFns[i].call(this.parentEl!);
+      }
+      this.child.remove();
+    }
+
+    firstNode(): Node | undefined {
+      return this.child.firstNode();
+    }
+
+    toString(): string {
+      return this.child.toString();
+    }
+  }
+
+  return function (child: VNode, handlers: any[]): VNode<VCatcher> {
+    return new VCatcher(child, handlers);
+  };
+}

src/blockdom/events.ts

@@ -5,6 +5,7 @@ type EventHandlerSetter = (this: HTMLElement, data: any) => void;
 interface EventHandlerCreator {
   setup: EventHandlerSetter;
   update: EventHandlerSetter;
+  remove: (this: HTMLElement) => void;
 }
 
 export function createEventHandler(rawEvent: string): EventHandlerCreator {
@@ -38,11 +39,15 @@ function createElementHandler(evName: string, capture: boolean = false): EventHa
     this.addEventListener(evName, listener, { capture });
   }
 
+  function remove(this: HTMLElement) {
+    delete (this as any)[eventKey];
+    this.removeEventListener(evName, listener, { capture });
+  }
   function update(this: HTMLElement, data: any) {
     (this as any)[eventKey] = data;
   }
 
-  return { setup, update };
+  return { setup, update, remove };
 }
 
 // Synthetic handler: a form of event delegation that allows placing only one
@@ -60,7 +65,12 @@ function createSyntheticHandler(evName: string, capture: boolean = false): Event
     _data[currentId] = data;
     (this as any)[eventKey] = _data;
   }
-  return { setup, update: setup };
+
+  function remove(this: HTMLElement) {
+    delete (this as any)[eventKey];
+  }
+
+  return { setup, update: setup, remove };
 }
 
 function nativeToSyntheticEvent(eventKey: string, event: Event) {

src/blockdom/index.ts

@@ -6,6 +6,7 @@ export { list } from "./list";
 export { multi } from "./multi";
 export { text, comment } from "./text";
 export { html } from "./html";
+export { createCatcher } from "./event_catcher";
 
 export interface VNode<T = any> {
   mount(parent: HTMLElement, afterNode: Node | null): void;

src/compiler/code_generator.ts

@@ -217,6 +217,9 @@ export class CodeGenerator {
   translatableAttributes: string[] = TRANSLATABLE_ATTRS;
   ast: AST;
   staticCalls: { id: string; template: string }[] = [];
+  // todo: merge with staticCalls
+  // todo: add a setCodeValue function => 2 args, call addLine, use it instead of addlin
+  eventCatchers: { id: string; expr: string }[] = [];
   helpers: Set<string> = new Set();
 
   constructor(ast: AST, options: CodeGenOptions) {
@@ -265,6 +268,9 @@ export class CodeGenerator {
     for (let { id, template } of this.staticCalls) {
       mainCode.push(`const ${id} = getTemplate(${template});`);
     }
+    for (let { id, expr } of this.eventCatchers) {
+      mainCode.push(`const ${id} = ${expr};`);
+    }
 
     // define all blocks
     if (this.blocks.length) {
@@ -1178,6 +1184,23 @@ export class CodeGenerator {
     if (ast.isDynamic) {
       blockExpr = `toggler(${expr}, ${blockExpr})`;
     }
+
+    // event handling
+    if (ast.on) {
+      this.helpers.add("createCatcher");
+      let name = this.generateId("catcher");
+      let spec: any = {};
+      let handlers: any[] = [];
+      for (let ev in ast.on) {
+        let handlerId = this.generateId("hdlr");
+        let idx = handlers.push(handlerId) - 1;
+        spec[ev] = idx;
+        const handler = this.generateHandlerCode(ev, ast.on[ev]);
+        this.addLine(`let ${handlerId} = ${handler};`);
+      }
+      blockExpr = `${name}(${blockExpr}, [${handlers.join(",")}])`;
+      this.eventCatchers.push({ id: name, expr: `createCatcher(${JSON.stringify(spec)})` });
+    }
     block = this.createBlock(block, "multi", ctx);
     this.insertBlock(blockExpr, block, ctx);
   }

src/compiler/parser.ts

@@ -119,6 +119,7 @@ export interface ASTComponent {
   name: string;
   isDynamic: boolean;
   dynamicProps: string | null;
+  on: null | { [key: string]: string };
   props: { [name: string]: string };
   slots: { [name: string]: { content: AST; attrs?: { [key: string]: string }; scope?: string } };
 }
@@ -650,7 +651,6 @@ function parseTSetNode(node: Element, ctx: ParsingContext): AST | null {
 
 // Error messages when trying to use an unsupported directive on a component
 const directiveErrorMap = new Map([
-  ["t-on", "t-on is no longer supported on components. Consider passing a callback in props."],
   [
     "t-ref",
     "t-ref is no longer supported on components. Consider exposing only the public part of the component's API through a callback prop.",
@@ -684,13 +684,19 @@ function parseComponent(node: Element, ctx: ParsingContext): AST | null {
 
   const defaultSlotScope = node.getAttribute("t-slot-scope");
   node.removeAttribute("t-slot-scope");
+  let on: ASTComponent["on"] = null;
 
   const props: ASTComponent["props"] = {};
   for (let name of node.getAttributeNames()) {
     const value = node.getAttribute(name)!;
     if (name.startsWith("t-")) {
-      const message = directiveErrorMap.get(name.split("-").slice(0, 2).join("-"));
-      throw new Error(message || `unsupported directive on Component: ${name}`);
+      if (name.startsWith("t-on-")) {
+        on = on || {};
+        on[name.slice(5)] = value;
+      } else {
+        const message = directiveErrorMap.get(name.split("-").slice(0, 2).join("-"));
+        throw new Error(message || `unsupported directive on Component: ${name}`);
+      }
     } else {
       props[name] = value;
     }
@@ -755,7 +761,7 @@ function parseComponent(node: Element, ctx: ParsingContext): AST | null {
       }
     }
   }
-  return { type: ASTType.TComponent, name, isDynamic, dynamicProps, props, slots };
+  return { type: ASTType.TComponent, name, isDynamic, dynamicProps, props, slots, on };
 }
 
 // -----------------------------------------------------------------------------

tests/blockdom/event_catcher.test.ts

@@ -0,0 +1,54 @@
+import { config, createBlock, createCatcher, mount } from "../../src/blockdom";
+import { makeTestFixture } from "./helpers";
+import { mainEventHandler } from "../../src/component/handler";
+
+//------------------------------------------------------------------------------
+// Setup and helpers
+//------------------------------------------------------------------------------
+
+let fixture: HTMLElement;
+config.mainEventHandler = mainEventHandler;
+
+beforeEach(() => {
+  fixture = makeTestFixture();
+});
+
+afterEach(() => {
+  fixture.remove();
+});
+
+test("simple event catcher", async () => {
+  const catcher = createCatcher({ click: 0 });
+  const block = createBlock("<div></div>");
+  let n = 0;
+  let ctx = {};
+  let handler = [() => n++, ctx];
+  const tree = catcher(block(), [handler]);
+
+  mount(tree, fixture);
+  expect(fixture.innerHTML).toBe("<div></div>");
+
+  expect(fixture.firstChild).toBeInstanceOf(HTMLDivElement);
+  expect(n).toBe(0);
+  (fixture.firstChild as HTMLDivElement).click();
+  expect(n).toBe(1);
+});
+
+test("do not catch events outside of itself", async () => {
+  const catcher = createCatcher({ click: 0 });
+  const childBlock = createBlock("<div></div>");
+  const parentBlock = createBlock("<button><block-child-0/></button>");
+  let n = 0;
+  let ctx = {};
+  let handler = [() => n++, ctx];
+  const tree = parentBlock([], [catcher(childBlock(), [handler])]);
+
+  mount(tree, fixture);
+  expect(fixture.innerHTML).toBe("<button><div></div></button>");
+
+  expect(n).toBe(0);
+  fixture.querySelector("div")!.click();
+  expect(n).toBe(1);
+  fixture.querySelector("button")!.click();
+  expect(n).toBe(1);
+});