From b89327c8319ab3204c2aad81f0214e8c3866d81a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Tue, 16 Apr 2019 17:49:33 +0200
Subject: [PATCH] [IMP] qweb: log a warning if a t-foreach is used without
 t-key

closes #40
---
 src/qweb.ts                           | 7 +++++++
 tests/__snapshots__/qweb.test.ts.snap | 6 +++---
 tests/qweb.test.ts                    | 6 +++---
 tests/store.test.ts                   | 6 +++---
 4 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/src/qweb.ts b/src/qweb.ts
index 37dbd875..aeb20528 100644
--- a/src/qweb.ts
+++ b/src/qweb.ts
@@ -928,6 +928,13 @@ const forEachDirective: Directive = {
     ctx.addLine(`context.${name} = _${keysID}[i];`);
     ctx.addLine(`context.${name}_value = _${valuesID}[i];`);
     const nodeCopy = <Element>node.cloneNode(true);
+    if (nodeCopy.tagName !== "t" && !nodeCopy.hasAttribute("t-key")) {
+      console.warn(
+        `Directive t-foreach should always be used with a t-key! (in template: '${
+          ctx.templateName
+        }')`
+      );
+    }
     nodeCopy.removeAttribute("t-foreach");
     qweb._compileNode(nodeCopy, ctx);
     ctx.dedent();
diff --git a/tests/__snapshots__/qweb.test.ts.snap b/tests/__snapshots__/qweb.test.ts.snap
index 6d27ef6f..acf9f953 100644
--- a/tests/__snapshots__/qweb.test.ts.snap
+++ b/tests/__snapshots__/qweb.test.ts.snap
@@ -303,7 +303,7 @@ exports[`foreach iterate on items (on a element node) 1`] = `
         context.item_index = i;
         context.item = _3[i];
         context.item_value = _4[i];
-        var c5 = [], p5 = {key:5};
+        var c5 = [], p5 = {key:context['item']};
         var vn5 = h('span', p5, c5);
         c1.push(vn5);
         var _6 = context['item'];
@@ -1121,7 +1121,7 @@ exports[`t-on can bind handlers with loop variable as argument 1`] = `
         context.action_index = i;
         context.action = _3[i];
         context.action_value = _4[i];
-        var c5 = [], p5 = {key:5};
+        var c5 = [], p5 = {key:context['action_index']};
         var vn5 = h('li', p5, c5);
         c1.push(vn5);
         var c6 = [], p6 = {key:6,on:{}};
@@ -1429,7 +1429,7 @@ exports[`t-set t-set should reuse variable if possible 1`] = `
         context.elem_index = i;
         context.elem = _4[i];
         context.elem_value = _5[i];
-        var c6 = [], p6 = {key:6};
+        var c6 = [], p6 = {key:context['elem_index']};
         var vn6 = h('div', p6, c6);
         c1.push(vn6);
         var c7 = [], p7 = {key:7};
diff --git a/tests/qweb.test.ts b/tests/qweb.test.ts
index 945eb727..5fce363f 100644
--- a/tests/qweb.test.ts
+++ b/tests/qweb.test.ts
@@ -289,7 +289,7 @@ describe("t-set", () => {
       "test",
       `<div>
           <t t-set="v" t-value="1"/>
-          <div t-foreach="list" t-as="elem">
+          <div t-foreach="list" t-as="elem" t-key="elem_index">
               <span>v<t t-esc="v"/></span>
               <t t-set="v" t-value="elem"/>
           </div>
@@ -663,7 +663,7 @@ describe("foreach", () => {
       "test",
       `
       <div>
-        <span t-foreach="[1, 2]" t-as="item"><t t-esc="item"/></span>
+        <span t-foreach="[1, 2]" t-as="item" t-key="item"><t t-esc="item"/></span>
     </div>`
     );
     const result = trim(renderToString(qweb, "test"));
@@ -922,7 +922,7 @@ describe("t-on", () => {
       "test",
       `
       <ul>
-        <li t-foreach="['someval']" t-as="action"><a t-on-click="activate(action)">link</a></li>
+        <li t-foreach="['someval']" t-as="action" t-key="action_index"><a t-on-click="activate(action)">link</a></li>
       </ul>`
     );
     const node = renderToDOM(
diff --git a/tests/store.test.ts b/tests/store.test.ts
index 61f29251..0a3b7bcf 100644
--- a/tests/store.test.ts
+++ b/tests/store.test.ts
@@ -292,7 +292,7 @@ describe("connecting a component to store", () => {
   class App extends Component<any, any, any> {
     inlineTemplate = `
             <div>
-                <t t-foreach="props.todos" t-as="todo">
+                <t t-foreach="props.todos" t-as="todo" t-key="todo">
                     <t t-widget="Todo" t-props="todo"/>
                 </t>
             </div>`;
@@ -340,7 +340,7 @@ describe("connecting a component to store", () => {
     class App extends Component<any, any, any> {
       inlineTemplate = `
             <div>
-                <span t-foreach="props.todos" t-as="todo">
+                <span t-foreach="props.todos" t-as="todo" t-key="todo">
                   <t t-esc="todo.title"/>
                 </span>
             </div>`;
@@ -487,7 +487,7 @@ describe("connecting a component to store", () => {
     class App extends Component<any, any, any> {
       inlineTemplate = `
           <div>
-              <span t-foreach="props.beers" t-as="beer"><t t-esc="beer.name"/></span>
+              <span t-foreach="props.beers" t-as="beer" t-key="beer.name"><t t-esc="beer.name"/></span>
           </div>`;
     }