From d48cb5aae4b0139bb9a1a2c8d50d8fb270ce0504 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Fri, 22 Mar 2019 09:52:48 +0100
Subject: [PATCH] imp: work on store, refactor, improve todoapp

---
 examples/todoapp/components/TodoApp.js | 20 +++++----
 examples/todoapp/main.js               |  2 +-
 examples/todoapp/templates.xml         |  6 +--
 src/index.ts                           |  4 +-
 src/store.ts                           | 43 ++++++++++++++++---
 src/utils.ts                           | 13 ++++++
 tests/__snapshots__/store.test.ts.snap | 15 +++++++
 tests/store.test.ts                    | 57 +++++++++++++++++++++++++-
 tests/utils.test.ts                    | 14 ++++++-
 9 files changed, 150 insertions(+), 24 deletions(-)
 create mode 100644 tests/__snapshots__/store.test.ts.snap

diff --git a/examples/todoapp/components/TodoApp.js b/examples/todoapp/components/TodoApp.js
index 2d565b87..ab3aca07 100644
--- a/examples/todoapp/components/TodoApp.js
+++ b/examples/todoapp/components/TodoApp.js
@@ -1,20 +1,20 @@
 import { TodoItem } from "./TodoItem.js";
 
-const { StoreMixin, Component } = odoo.core;
+const { connect, Component } = odoo.core;
 
 const ENTER_KEY = 13;
 
-export class TodoApp extends StoreMixin(Component) {
+function mapStateToProps(state) {
+  return { todos: state.todos };
+}
+
+class TodoApp extends Component {
   template = "todoapp";
   widgets = { TodoItem };
   state = { filter: "all" };
 
-  get todos() {
-    return this.env.store.state.todos;
-  }
-
   get visibleTodos() {
-    let todos = this.todos;
+    let todos = this.props.todos;
     if (this.state.filter === "active") {
       todos = todos.filter(t => !t.completed);
     }
@@ -25,11 +25,11 @@ export class TodoApp extends StoreMixin(Component) {
   }
 
   get allChecked() {
-    return this.todos.every(todo => todo.completed);
+    return this.props.todos.every(todo => todo.completed);
   }
 
   get remaining() {
-    return this.todos.filter(todo => !todo.completed).length;
+    return this.props.todos.filter(todo => !todo.completed).length;
   }
 
   get remainingText() {
@@ -54,3 +54,5 @@ export class TodoApp extends StoreMixin(Component) {
     this.env.store.dispatch("toggleAll", !this.allChecked);
   }
 }
+
+export default connect(mapStateToProps)(TodoApp);
diff --git a/examples/todoapp/main.js b/examples/todoapp/main.js
index 0e77fe0d..342511b7 100644
--- a/examples/todoapp/main.js
+++ b/examples/todoapp/main.js
@@ -1,4 +1,4 @@
-import { TodoApp } from "./components/TodoApp.js";
+import TodoApp from "./components/TodoApp.js";
 import { makeStore } from "./store.js";
 
 async function makeEnv() {
diff --git a/examples/todoapp/templates.xml b/examples/todoapp/templates.xml
index dd444b50..a2290c86 100644
--- a/examples/todoapp/templates.xml
+++ b/examples/todoapp/templates.xml
@@ -8,7 +8,7 @@
             <input class="new-todo" autofocus="true" autocomplete="off" placeholder="What needs to be done?" t-on-keyup="addTodo"/>
         </header>
         <!-- main section -->
-        <section class="main" t-if="todos.length">
+        <section class="main" t-if="props.todos.length">
             <input class="toggle-all" id="toggle-all" type="checkbox" t-att-checked="allChecked" t-on-click="toggleAll"/>
             <label for="toggle-all"></label>
             <ul class="todo-list">
@@ -18,7 +18,7 @@
             </ul>
         </section>
         <!-- footer -->
-        <footer class="footer" t-if="todos.length">
+        <footer class="footer" t-if="props.todos.length">
             <span class="todo-count">
                 <strong>
                     <t t-esc="remaining"/>
@@ -36,7 +36,7 @@
                     <a href="#/completed" t-on-click="updateState({filter:'completed'})" t-att-class="{selected: state.filter === 'completed'}">Completed</a>
                 </li>
             </ul>
-            <button class="clear-completed" t-if="todos.length gt remaining" t-on-click="clearCompleted">
+            <button class="clear-completed" t-if="props.todos.length gt remaining" t-on-click="clearCompleted">
         Clear completed
             </button>
         </footer>
diff --git a/src/index.ts b/src/index.ts
index 11bb49b2..9df98e56 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -1,7 +1,7 @@
 import { QWeb } from "./qweb";
 import { EventBus } from "./event_bus";
 import { Component, PureComponent } from "./component";
-import { Store, StoreMixin } from "./store";
+import { Store, connect } from "./store";
 
 export const core = {
   QWeb,
@@ -9,5 +9,5 @@ export const core = {
   Component,
   PureComponent,
   Store,
-  StoreMixin
+  connect
 };
diff --git a/src/store.ts b/src/store.ts
index 2554b080..57466382 100644
--- a/src/store.ts
+++ b/src/store.ts
@@ -1,10 +1,41 @@
 import { EventBus } from "./event_bus";
+import { Component } from "./component";
+import { shallowEqual } from "./utils";
 
-export function StoreMixin(Component) {
-  return class extends Component {
-    mounted() {
-      this.env.store.on("update", this, this.render);
-    }
+export function connect(mapStateToProps) {
+  return function(Comp) {
+    return class extends Comp {
+      constructor(parent, props?: any) {
+        const env = parent instanceof Component ? parent.env : parent;
+        const storeProps = mapStateToProps(env.store.state);
+        props = Object.assign(props || {}, storeProps);
+        super(parent, props);
+        this.__widget__.currentStoreProps = storeProps;
+      }
+      mounted() {
+        this.env.store.on("update", this, () => {
+          const storeProps = mapStateToProps(this.env.store.state);
+          if (!shallowEqual(storeProps, this.__widget__.currentStoreProps)) {
+            this.__widget__.currentStoreProps = storeProps;
+            // probably not optimal, will do 2 object.assign, one here and
+            // one in updateProps.
+            const nextProps = Object.assign(
+              {},
+              this.props,
+              this.__widget__.currentStoreProps
+            );
+            this.updateProps(nextProps, false);
+          }
+        });
+      }
+      willUnmount() {
+        this.env.store.off("update", this);
+      }
+      updateProps(nextProps, forceUpdate) {
+        nextProps = Object.assign(nextProps, this.__widget__.currentStoreProps);
+        return super.updateProps(nextProps, forceUpdate);
+      }
+    };
   };
 }
 
@@ -53,7 +84,7 @@ export class Store extends EventBus {
     await Promise.resolve();
     if (this._isMutating) {
       this._isMutating = false;
-      this.trigger("update");
+      this.trigger("update", this.state);
     }
   }
 
diff --git a/src/utils.ts b/src/utils.ts
index 303096d5..12a76fb6 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -112,3 +112,16 @@ export function findInTree<T extends Tree<T>>(
   }
   return null;
 }
+
+export function shallowEqual(objA, objB) {
+  if (objA === objB) {
+    return true;
+  }
+  const keysA = Object.keys(objA);
+  for (let key of keysA) {
+    if (!(key in objB) || objA[key] !== objB[key]) {
+      return false;
+    }
+  }
+  return true;
+}
diff --git a/tests/__snapshots__/store.test.ts.snap b/tests/__snapshots__/store.test.ts.snap
new file mode 100644
index 00000000..392d8141
--- /dev/null
+++ b/tests/__snapshots__/store.test.ts.snap
@@ -0,0 +1,15 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`connecting a component to store connecting a component works 1`] = `
+"<div>
+                
+            </div>"
+`;
+
+exports[`connecting a component to store connecting a component works 2`] = `
+"<div>
+                
+                    <span>hello</span>
+                
+            </div>"
+`;
diff --git a/tests/store.test.ts b/tests/store.test.ts
index dcd19f1c..5fa3855d 100644
--- a/tests/store.test.ts
+++ b/tests/store.test.ts
@@ -1,5 +1,11 @@
-import { Store } from "../src/store";
-import { nextMicroTick } from "./helpers";
+import { Store, connect } from "../src/store";
+import { Component, Env } from "../src/component";
+import {
+  nextMicroTick,
+  makeTestWEnv,
+  makeTestFixture,
+  nextTick
+} from "./helpers";
 
 describe("basic use", () => {
   test("commit a mutation", () => {
@@ -54,3 +60,50 @@ describe("basic use", () => {
     expect(updateCounter).toBe(1);
   });
 });
+
+describe("connecting a component to store", () => {
+  let fixture: HTMLElement;
+  let env: Env;
+
+  beforeEach(() => {
+    fixture = makeTestFixture();
+    env = makeTestWEnv();
+  });
+
+  afterEach(() => {
+    fixture.remove();
+  });
+
+  class App extends Component<any, any, any> {
+    inlineTemplate = `
+            <div>
+                <t t-foreach="props.todos" t-as="todo">
+                    <t t-widget="Todo" t-props="todo"/>
+                </t>
+            </div>`;
+    widgets = { Todo };
+  }
+  class Todo extends Component<any, any, any> {
+    inlineTemplate = `<span><t t-esc="props.msg"/></span>`;
+  }
+
+  test("connecting a component works", async () => {
+    const state = { todos: [] };
+    const mutations = {
+      addTodo(state, msg) {
+        state.todos.push({ msg });
+      }
+    };
+    const TodoApp = connect(s => s)(App);
+    const store = new Store({ state, mutations });
+    (<any>env).store = store;
+    const app = new TodoApp(env);
+
+    await app.mount(fixture);
+    expect(fixture.innerHTML).toMatchSnapshot();
+
+    store.commit("addTodo", "hello");
+    await nextTick();
+    expect(fixture.innerHTML).toMatchSnapshot();
+  });
+});
diff --git a/tests/utils.test.ts b/tests/utils.test.ts
index 78071c54..103c3593 100644
--- a/tests/utils.test.ts
+++ b/tests/utils.test.ts
@@ -4,7 +4,8 @@ import {
   idGenerator,
   memoize,
   debounce,
-  findInTree
+  findInTree,
+  shallowEqual
 } from "../src/utils";
 
 describe("escape", () => {
@@ -91,3 +92,14 @@ describe("findInTree", () => {
     expect(match2).toBe(null);
   });
 });
+
+describe("shallowEqual", () => {
+  test("simple comparisons", () => {
+    const obj1 = {};
+    expect(shallowEqual(obj1, obj1)).toBe(true);
+    expect(shallowEqual({}, {})).toBe(true);
+    expect(shallowEqual({ a: 1 }, {})).toBe(false);
+    expect(shallowEqual({ a: 1 }, { a: 1 })).toBe(true);
+    expect(shallowEqual({ a: 1 }, ["a"])).toBe(false);
+  });
+});