[FIX] components: make sure t-ref work with t-if/t-else

src/app/template_helpers.ts

@@ -141,6 +141,24 @@ function bind(ctx: any, fn: Function): Function {
   return boundFn;
 }
 
+type RefMap = { [key: string]: HTMLElement | null };
+type RefSetter = (el: HTMLElement | null) => void;
+
+function multiRefSetter(refs: RefMap, name: string): RefSetter {
+  let count = 0;
+  return (el) => {
+    if (el) {
+      count++;
+      if (count > 1) {
+        throw new Error("Cannot have 2 elements with same ref name at the same time");
+      }
+    }
+    if (count === 0 || el) {
+      refs[name] = el;
+    }
+  };
+}
+
 export const UTILS = {
   withDefault,
   zero: Symbol("zero"),
@@ -150,6 +168,7 @@ export const UTILS = {
   withKey,
   prepareList,
   setContextValue,
+  multiRefSetter,
   shallowEqual,
   toNumber,
   validateProps,

src/compiler/code_generator.ts

@@ -148,6 +148,8 @@ class CodeTarget {
   hasRoot = false;
   hasCache = false;
   hasRef: boolean = false;
+  // maps ref name to [id, expr]
+  refInfo: { [name: string]: [string, string] } = {};
   shouldProtectScope: boolean = false;
 
   constructor(name: string) {
@@ -168,6 +170,10 @@ class CodeTarget {
     result.push(`function ${this.name}(ctx, node, key = "") {`);
     if (this.hasRef) {
       result.push(`  const refs = ctx.__owl__.refs;`);
+      for (let name in this.refInfo) {
+        const [id, expr] = this.refInfo[name];
+        result.push(`  const ${id} = ${expr};`);
+      }
     }
     if (this.shouldProtectScope) {
       result.push(`  ctx = Object.create(ctx);`);
@@ -556,8 +562,20 @@ export class CodeGenerator {
         const idx = block!.insertData(`(el) => refs[\`${str}\`] = el`, "ref");
         attrs["block-ref"] = String(idx);
       } else {
-        const idx = block!.insertData(`(el) => refs[\`${ast.ref}\`] = el`);
-        attrs["block-ref"] = String(idx);
+        let name = ast.ref;
+        if (name in this.target.refInfo) {
+          // ref has already been defined
+          this.helpers.add("multiRefSetter");
+          const info = this.target.refInfo[name];
+          const index = block!.data.push(info[0]) - 1;
+          attrs["block-ref"] = String(index);
+          info[1] = `multiRefSetter(refs, \`${name}\`)`;
+        } else {
+          let id = this.generateId("ref");
+          this.target.refInfo[name] = [id, `(el) => refs[\`${name}\`] = el`];
+          const index = block!.data.push(id) - 1;
+          attrs["block-ref"] = String(index);
+        }
       }
     }
 

src/hooks.ts

@@ -12,9 +12,10 @@ import { onMounted, onPatched, onWillUnmount } from "./component/lifecycle_hooks
  */
 export function useRef<T extends HTMLElement = HTMLElement>(name: string): { el: T | null } {
   const node = getCurrent()!;
+  const refs = node.refs;
   return {
     get el(): T | null {
-      return node.refs[name] || null;
+      return refs[name] || null;
     },
   };
 }

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

@@ -215,6 +215,8 @@ exports[`misc other complex template 1`] = `
   
   return function template(ctx, node, key = \\"\\") {
     const refs = ctx.__owl__.refs;
+    const ref1 = (el) => refs[\`search_input\`] = el;
+    const ref2 = (el) => refs[\`settings_menu\`] = el;
     let b2,b4,b14,b17,b22,b23,b24,b25;
     let attr1 = \`/runbot/\${ctx['project'].slug}\`;
     let txt1 = ctx['project'].name;
@@ -275,9 +277,7 @@ exports[`misc other complex template 1`] = `
     let attr8 = ctx['search'].value;
     let hdlr4 = [ctx['updateFilter'], ctx];
     let hdlr5 = [ctx['updateFilter'], ctx];
-    let d1 = (el) => refs[\`search_input\`] = el;
     let hdlr6 = [ctx['clearSearch'], ctx];
-    let d2 = (el) => refs[\`settings_menu\`] = el;
     if (ctx['triggers']) {
       ctx = Object.create(ctx);
       const [k_block18, v_block18, l_block18, c_block18] = prepareList(ctx['triggers']);
@@ -320,7 +320,7 @@ exports[`misc other complex template 1`] = `
       let b27 = component(\`BundlesList\`, {bundles: ctx['bundles'].dev,search: ctx['search']}, key + \`__3\`, node, ctx);
       b25 = block25([], [b26, b27]);
     }
-    return block1([attr1, txt1, hdlr2, hdlr3, attr8, hdlr4, hdlr5, d1, hdlr6, d2], [b2, b4, b14, b17, b22, b23, b24, b25]);
+    return block1([attr1, txt1, hdlr2, hdlr3, attr8, hdlr4, hdlr5, ref1, hdlr6, ref2], [b2, b4, b14, b17, b22, b23, b24, b25]);
   }
 }"
 `;

tests/compiler/__snapshots__/t_ref.test.ts.snap

@@ -25,8 +25,8 @@ exports[`t-ref can get a ref on a node 1`] = `
   
   return function template(ctx, node, key = \\"\\") {
     const refs = ctx.__owl__.refs;
-    let d1 = (el) => refs[\`myspan\`] = el;
-    return block1([d1]);
+    const ref1 = (el) => refs[\`myspan\`] = el;
+    return block1([ref1]);
   }
 }"
 `;
@@ -56,8 +56,8 @@ exports[`t-ref ref in a t-call 2`] = `
   
   return function template(ctx, node, key = \\"\\") {
     const refs = ctx.__owl__.refs;
-    let d1 = (el) => refs[\`name\`] = el;
-    return block1([d1]);
+    const ref1 = (el) => refs[\`name\`] = el;
+    return block1([ref1]);
   }
 }"
 `;
@@ -72,10 +72,10 @@ exports[`t-ref ref in a t-if 1`] = `
   
   return function template(ctx, node, key = \\"\\") {
     const refs = ctx.__owl__.refs;
+    const ref1 = (el) => refs[\`name\`] = el;
     let b2;
     if (ctx['condition']) {
-      let d1 = (el) => refs[\`name\`] = el;
-      b2 = block2([d1]);
+      b2 = block2([ref1]);
     }
     return block1([], [b2]);
   }
@@ -120,13 +120,13 @@ exports[`t-ref two refs, one in a t-if 1`] = `
   
   return function template(ctx, node, key = \\"\\") {
     const refs = ctx.__owl__.refs;
+    const ref1 = (el) => refs[\`name\`] = el;
+    const ref2 = (el) => refs[\`p\`] = el;
     let b2;
     if (ctx['condition']) {
-      let d1 = (el) => refs[\`name\`] = el;
-      b2 = block2([d1]);
+      b2 = block2([ref1]);
     }
-    let d2 = (el) => refs[\`p\`] = el;
-    return block1([d2], [b2]);
+    return block1([ref2], [b2]);
   }
 }"
 `;

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

@@ -10,13 +10,13 @@ exports[`hooks autofocus hook input in a t-if 1`] = `
   
   return function template(ctx, node, key = \\"\\") {
     const refs = ctx.__owl__.refs;
+    const ref1 = (el) => refs[\`input1\`] = el;
+    const ref2 = (el) => refs[\`input2\`] = el;
     let b2;
-    let d1 = (el) => refs[\`input1\`] = el;
     if (ctx['state'].flag) {
-      let d2 = (el) => refs[\`input2\`] = el;
-      b2 = block2([d2]);
+      b2 = block2([ref2]);
     }
-    return block1([d1], [b2]);
+    return block1([ref1], [b2]);
   }
 }"
 `;
@@ -30,9 +30,9 @@ exports[`hooks autofocus hook simple input 1`] = `
   
   return function template(ctx, node, key = \\"\\") {
     const refs = ctx.__owl__.refs;
-    let d1 = (el) => refs[\`input1\`] = el;
-    let d2 = (el) => refs[\`input2\`] = el;
-    return block1([d1, d2]);
+    const ref1 = (el) => refs[\`input1\`] = el;
+    const ref2 = (el) => refs[\`input2\`] = el;
+    return block1([ref1, ref2]);
   }
 }"
 `;
@@ -206,10 +206,10 @@ exports[`hooks useEffect hook effect can depend on stuff in dom 1`] = `
   
   return function template(ctx, node, key = \\"\\") {
     const refs = ctx.__owl__.refs;
+    const ref1 = (el) => refs[\`div\`] = el;
     let b2;
     if (ctx['state'].value) {
-      let d1 = (el) => refs[\`div\`] = el;
-      b2 = block2([d1]);
+      b2 = block2([ref1]);
     }
     return multi([b2]);
   }
@@ -281,9 +281,9 @@ exports[`hooks useRef hook: basic use 1`] = `
   
   return function template(ctx, node, key = \\"\\") {
     const refs = ctx.__owl__.refs;
-    let d1 = (el) => refs[\`button\`] = el;
+    const ref1 = (el) => refs[\`button\`] = el;
     let txt1 = ctx['value'];
-    return block1([d1, txt1]);
+    return block1([ref1, txt1]);
   }
 }"
 `;

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

@@ -9,8 +9,31 @@ exports[`refs basic use 1`] = `
   
   return function template(ctx, node, key = \\"\\") {
     const refs = ctx.__owl__.refs;
-    let d1 = (el) => refs[\`div\`] = el;
-    return block1([d1]);
+    const ref1 = (el) => refs[\`div\`] = el;
+    return block1([ref1]);
+  }
+}"
+`;
+
+exports[`refs can use 2 refs with same name in a t-if/t-else situation 1`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  let { multiRefSetter } = helpers;
+  
+  let block2 = createBlock(\`<div block-ref=\\"0\\"/>\`);
+  let block3 = createBlock(\`<span block-ref=\\"0\\"/>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    const refs = ctx.__owl__.refs;
+    const ref1 = multiRefSetter(refs, \`coucou\`);
+    let b2,b3;
+    if (ctx['state'].value) {
+      b2 = block2([ref1]);
+    } else {
+      b3 = block3([ref1]);
+    }
+    return multi([b2, b3]);
   }
 }"
 `;
@@ -26,9 +49,9 @@ exports[`refs refs are properly bound in slots 1`] = `
   
   function slot1(ctx, node, key = \\"\\") {
     const refs = ctx.__owl__.refs;
+    const ref1 = (el) => refs[\`myButton\`] = el;
     let hdlr1 = [ctx['doSomething'], ctx];
-    let d1 = (el) => refs[\`myButton\`] = el;
-    return block2([hdlr1, d1]);
+    return block2([hdlr1, ref1]);
   }
   
   return function template(ctx, node, key = \\"\\") {
@@ -54,3 +77,22 @@ exports[`refs refs are properly bound in slots 2`] = `
   }
 }"
 `;
+
+exports[`refs throws if there are 2 same refs at the same time 1`] = `
+"function anonymous(bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
+  let { multiRefSetter } = helpers;
+  
+  let block2 = createBlock(\`<div block-ref=\\"0\\"/>\`);
+  let block3 = createBlock(\`<span block-ref=\\"0\\"/>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    const refs = ctx.__owl__.refs;
+    const ref1 = multiRefSetter(refs, \`coucou\`);
+    let b2 = block2([ref1]);
+    let b3 = block3([ref1]);
+    return multi([b2, b3]);
+  }
+}"
+`;

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

@@ -128,8 +128,8 @@ exports[`slots can render node with t-ref and Component in same slot 1`] = `
   
   function slot1(ctx, node, key = \\"\\") {
     const refs = ctx.__owl__.refs;
-    let d1 = (el) => refs[\`div\`] = el;
-    let b2 = block2([d1]);
+    const ref1 = (el) => refs[\`div\`] = el;
+    let b2 = block2([ref1]);
     let b3 = component(\`Child\`, {}, key + \`__1\`, node, ctx);
     return multi([b2, b3]);
   }

tests/components/refs.test.ts

@@ -53,4 +53,51 @@ describe("refs", () => {
       '<div><span class="counter">1</span><span><button>do something</button></span></div>'
     );
   });
+
+  test("can use 2 refs with same name in a t-if/t-else situation", async () => {
+    class Test extends Component {
+      static template = xml`
+        <t t-if="state.value">
+          <div t-ref="coucou"/>
+        </t>
+        <t t-else="">
+          <span t-ref="coucou"/>
+        </t>`;
+
+      state = useState({ value: true });
+      ref = useRef("coucou");
+    }
+    const test = await mount(Test, fixture);
+    expect(fixture.innerHTML).toBe("<div></div>");
+    expect(test.ref.el!.tagName).toBe("DIV");
+
+    test.state.value = false;
+    await nextTick();
+    expect(fixture.innerHTML).toBe("<span></span>");
+    expect(test.ref.el!.tagName).toBe("SPAN");
+
+    test.state.value = true;
+    await nextTick();
+    expect(fixture.innerHTML).toBe("<div></div>");
+    expect(test.ref.el!.tagName).toBe("DIV");
+  });
+
+  test("throws if there are 2 same refs at the same time", async () => {
+    const consoleWarn = console.warn;
+    console.warn = jest.fn();
+    class Test extends Component {
+      static template = xml`
+        <div t-ref="coucou"/>
+        <span t-ref="coucou"/>`;
+
+      state = useState({ value: true });
+      ref = useRef("coucou");
+    }
+
+    await expect(async () => {
+      await mount(Test, fixture);
+    }).rejects.toThrowError("Cannot have 2 elements with same ref name at the same time");
+    expect(console.warn).toBeCalledTimes(1);
+    console.warn = consoleWarn;
+  });
 });