[IMP] qweb/components: remove t-ref on components

Refs to component expose a lot of implementation details that should be
private to parents. Parent to child communication should go through
props.

src/qweb/compiler.ts

@@ -261,8 +261,7 @@ export class QWebCompiler {
     return block;
   }
 
-  insertBlock(expression: string, block: BlockDescription, ctx: Context): string | null {
+  insertBlock(expression: string, block: BlockDescription, ctx: Context): void {
-    let id: string | null = null;
     let blockExpr = block.generateExpr(expression);
     const tKeyExpr = ctx.tKeyExpr;
     if (block.parentVar) {
@@ -271,7 +270,7 @@ export class QWebCompiler {
         keyArg = `${tKeyExpr} + ${keyArg}`;
       }
       this.addLine(`${block.parentVar}[${ctx.index}] = withKey(${blockExpr}, ${keyArg});`);
-      return id;
+      return;
     }
 
     if (tKeyExpr) {
@@ -283,7 +282,6 @@ export class QWebCompiler {
     } else {
       this.addLine(`let ${block.varName} = ${blockExpr};`);
     }
-    return id;
   }
 
   generateCode(): string {

src/qweb/parser.ts

@@ -302,11 +302,8 @@ function parseDOMNode(node: Element, ctx: ParsingContext): AST | null {
   if (tagName === "pre") {
     ctx = { inPreTag: true };
   }
-  let ref = null;
+  const ref = node.getAttribute("t-ref");
-  if (node.hasAttribute("t-ref")) {
+  node.removeAttribute("t-ref");
-    ref = node.getAttribute("t-ref");
-    node.removeAttribute("t-ref");
-  }
 
   for (let child of node.childNodes) {
     const ast = parseNode(child, ctx);

src/refs.ts

@@ -2,35 +2,17 @@
 // useRef
 // -----------------------------------------------------------------------------
 
-import type { Component } from "./component/component";
 import { getCurrent } from "./component/component_node";
 
 /**
  * The purpose of this hook is to allow components to get a reference to a sub
  * html node or component.
  */
-interface Ref<C extends Component = Component> {
+export function useRef<T extends HTMLElement = HTMLElement>(name: string): { el: T | null } {
-  el: HTMLElement | null;
-  comp: C | null;
-}
-
-export function useRef<C extends Component = Component>(name: string): Ref<C> {
   const node = getCurrent()!;
   return {
-    get el(): HTMLElement | null {
+    get el(): T | null {
-      const val = node.refs[name];
+      return node.refs[name] || null;
-      return val || null;
-      // if (val instanceof HTMLElement) {
-      //   return val;
-      // } else if (val instanceof Component) {
-      //   return val.el;
-      // }
-      // return null;
-    },
-    get comp(): C | null {
-      return null;
-      // const val = node.refs && node.refs[name];
-      // return val instanceof Component ? (val as C) : null;
     },
   };
 }

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

@@ -1,5 +1,21 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`refs basic use 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, toNumber } = helpers;
+  
+  let block1 = createBlock(\`<div block-ref=\\"0\\"/>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    const refs = ctx.__owl__.refs;
+    let d1 = (el) => refs[\`div\`] = el;
+    return block1([d1]);
+  }
+}"
+`;
+
 exports[`refs refs are properly bound in slots 1`] = `
 "function anonymous(bdom, helpers
 ) {

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

@@ -490,3 +490,30 @@ exports[`style and class handling t-att-class is properly added/removed on widge
   }
 }"
 `;
+
+exports[`style and class handling t-att-class is properly added/removed on widget root el (v3) 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, toNumber } = helpers;
+  
+  let block1 = createBlock(\`<span class=\\"c\\" block-attribute-0=\\"class\\"/>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let d1 = {d:ctx['state'].d,...ctx['props'].class};
+    return block1([d1]);
+  }
+}"
+`;
+
+exports[`style and class handling t-att-class is properly added/removed on widget root el (v3) 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, toNumber } = helpers;
+  
+  return function template(ctx, node, key = \\"\\") {
+    return component(\`Child\`, {class: {a:true,b:ctx['state'].b}}, key + \`__1\`, node, ctx);
+  }
+}"
+`;

tests/components/hooks.test.ts

@@ -34,7 +34,7 @@ describe("hooks", () => {
       }
       increment() {
         this.value++;
-        (this.button.el as HTMLButtonElement).innerHTML = String(this.value);
+        this.button.el!.innerHTML = String(this.value);
       }
     }
     const mounted = mount(Counter, fixture);

tests/components/refs.test.ts

@@ -10,6 +10,15 @@ beforeEach(() => {
 });
 
 describe("refs", () => {
+  test("basic use", async () => {
+    class Test extends Component {
+      static template = xml`<div t-ref="div"/>`;
+      button = useRef("div");
+    }
+    const test = await mount(Test, fixture);
+    expect(test.button.el).toBe(fixture.firstChild);
+  });
+
   test("refs are properly bound in slots", async () => {
     class Dialog extends Component {
       static template = xml`<span><t t-slot="footer"/></span>`;
@@ -44,104 +53,4 @@ describe("refs", () => {
       '<div><span class="counter">1</span><span><button>do something</button></span></div>'
     );
   });
-  // TODO: rename
-  test.skip("t-refs on widget are components", async () => {
-    class Child extends Component {
-      static template = xml`<div>b</div>`;
-    }
-    let parent: Parent;
-    class Parent extends Component {
-      static template = xml`<div class="outer-div">Hello<Child t-ref="mywidgetb" /></div>`;
-      static components = { Child };
-      ref = useRef<Child>("mywidgetb");
-      setup() {
-        parent = this;
-      }
-    }
-
-    const mounted = mount(Parent, fixture);
-    expect(parent!.ref.comp).toBe(null);
-    expect(parent!.ref.el).toBe(null);
-    await mounted;
-    expect(parent!.ref.comp).toBeInstanceOf(Child);
-    expect(parent!.ref.el).toEqual(fixture.querySelector(".outer-div > div"));
-  });
-
-  test.skip("t-refs are bound at proper timing", async () => {
-    expect.assertions(4);
-    class Child extends Component {
-      static template = xml`<div>widget</div>`;
-    }
-
-    class Parent extends Component {
-      static template = xml`
-        <div>
-          <Child t-foreach="state.list" t-as="elem" t-ref="child" t-key="elem"/>
-        </div>
-      `;
-      static components = { Child };
-      state = useState({ list: <any>[] });
-      child = useRef("child");
-      willPatch() {
-        expect(this.child.comp).toBeNull();
-      }
-      patched() {
-        expect(this.child.comp).not.toBeNull();
-      }
-    }
-
-    const parent = await mount(Parent, fixture);
-    parent.state.list.push(1);
-    await nextTick();
-  });
-
-  test.skip("t-refs are bound at proper timing (2)", async () => {
-    expect.assertions(10);
-    class Child extends Component {
-      static template = xml`<div>widget</div>`;
-    }
-    class Parent extends Component {
-      static template = xml`
-        <div>
-          <Child t-if="state.child1" t-ref="child1"/>
-          <Child t-if="state.child2" t-ref="child2"/>
-        </div>`;
-      static components = { Child };
-      state = useState({ child1: true, child2: false });
-      child1 = useRef("child1");
-      child2 = useRef("child2");
-      count = 0;
-      mounted() {
-        expect(this.child1.comp).toBeDefined();
-        expect(this.child2.comp).toBeNull();
-      }
-      willPatch() {
-        if (this.count === 0) {
-          expect(this.child1.comp).toBeDefined();
-          expect(this.child2.comp).toBeNull();
-        }
-        if (this.count === 1) {
-          expect(this.child1.comp).toBeDefined();
-          expect(this.child2.comp).toBeDefined();
-        }
-      }
-      patched() {
-        if (this.count === 0) {
-          expect(this.child1.comp).toBeDefined();
-          expect(this.child2.comp).toBeDefined();
-        }
-        if (this.count === 1) {
-          expect(this.child1.comp).toBeNull();
-          expect(this.child2.comp).toBeDefined();
-        }
-        this.count++;
-      }
-    }
-
-    const parent = await mount(Parent, fixture);
-    parent.state.child2 = true;
-    await nextTick();
-    parent.state.child1 = false;
-    await nextTick();
-  });
 });

tests/components/style_class.test.ts

@@ -1,4 +1,4 @@
-import { Component, mount, useState, useRef, xml } from "../../src";
+import { Component, mount, useState, xml } from "../../src";
 import { makeTestFixture, nextTick, snapshotEverything } from "../helpers";
 
 snapshotEverything();
@@ -197,7 +197,7 @@ describe("style and class handling", () => {
 
   // TODO: adapt name (t-att-class no longer makes sense on Components)
   test("t-att-class is properly added/removed on widget root el (v2)", async () => {
-    let child: any = null;
+    let child: Child;
     class Child extends Component {
       static template = xml`<span class="c" t-att-class="{ d: state.d, ...props.class }"/>`;
       state = useState({ d: true });
@@ -223,7 +223,7 @@ describe("style and class handling", () => {
     await nextTick();
     expect(span.className).toBe("c d");
 
-    child.state.d = false;
+    child!.state.d = false;
     await nextTick();
     expect(span.className).toBe("c");
 
@@ -231,23 +231,25 @@ describe("style and class handling", () => {
     await nextTick();
     expect(span.className).toBe("c b");
 
-    child.state.d = true;
+    child!.state.d = true;
     await nextTick();
     expect(span.className).toBe("c b d");
   });
 
   // TODO: adapt name (t-att-class no longer makes sense on Components)
-  // TODO: this test depends on ref to components
+  test("t-att-class is properly added/removed on widget root el (v3)", async () => {
-  test.skip("t-att-class is properly added/removed on widget root el (v3)", async () => {
+    let child: Child;
     class Child extends Component {
-      static template = `<span t-name="Child" class="c" t-att-class="state.d ? 'd' : ''"/>`;
+      static template = xml`<span class="c" t-att-class="{ d: state.d, ...props.class }"/>`;
       state = useState({ d: true });
+      setup() {
+        child = this;
+      }
     }
     class Parent extends Component {
-      static template = xml`<Child class="a" t-att-class="state.b ? 'b' : ''" t-ref="child"/>`;
+      static template = xml`<Child class="{ a: true, b: state.b }"/>`;
       static components = { Child };
       state = useState({ b: true });
-      child = useRef("child");
     }
     const widget = await mount(Parent, fixture);
 
@@ -258,7 +260,7 @@ describe("style and class handling", () => {
     await nextTick();
     expect(span.className).toBe("c d a");
 
-    (widget.child.comp as Child).state.d = false;
+    child!.state.d = false;
     await nextTick();
     expect(span.className).toBe("c a");
 
@@ -266,7 +268,7 @@ describe("style and class handling", () => {
     await nextTick();
     expect(span.className).toBe("c a b");
 
-    (widget.child.comp as Child).state.d = true;
+    child!.state.d = true;
     await nextTick();
     expect(span.className).toBe("c a b d");
   });