[FIX] store: change default hash function to better see changes

src/store.ts

@@ -1,6 +1,5 @@
-import { EventBus } from "./event_bus";
-import { shallowEqual } from "./utils";
 import { Component } from "./component";
+import { EventBus } from "./event_bus";
 
 //------------------------------------------------------------------------------
 // Store Definition
@@ -255,23 +254,40 @@ export function makeObserver(): Observer {
 //------------------------------------------------------------------------------
 
 function revNumber<T extends Object>(o: T): number {
-  if (!("__owl__" in o)) {
+  if (o !== null && typeof o === "object" && (<any>o).__owl__) {
-    return 0;
+    return (<any>o).__owl__.rev;
   }
-  return (<any>o).__owl__.rev;
+  return 0;
 }
 
 function deepRevNumber<T extends Object>(o: T): number {
-  if (!("__owl__" in o)) {
+  if (o !== null && typeof o === "object" && (<any>o).__owl__) {
-    return 0;
+    return (<any>o).__owl__.deepRev;
   }
-  return (<any>o).__owl__.deepRev;
+  return 0;
 }
 
 export function connect(mapStateToProps, options: any = {}) {
   let hashFunction = options.hashFunction || null;
-  let deep = "deep" in options ? options.deep : true;
+
-  let defaultRevFunction = deep ? deepRevNumber : revNumber;
+  if (!hashFunction) {
+    let deep = "deep" in options ? options.deep : true;
+    let defaultRevFunction = deep ? deepRevNumber : revNumber;
+    hashFunction = function({ storeProps, currentStoreProps }) {
+      if ("__owl__" in storeProps) {
+        return defaultRevFunction(storeProps);
+      }
+      let hash = 0;
+      let currentProps = currentStoreProps;
+      for (let key in storeProps) {
+        const val = storeProps[key];
+        hash +=
+          defaultRevFunction(storeProps[key]) ||
+          (val !== currentProps[key] ? 1 : 0);
+      }
+      return hash;
+    };
+  }
 
   return function(Comp) {
     return class extends Comp {
@@ -283,55 +299,29 @@ export function connect(mapStateToProps, options: any = {}) {
         super(parent, mergedProps);
         this.__owl__.ownProps = ownProps;
         this.__owl__.currentStoreProps = storeProps;
-        if (!hashFunction) {
+        this.__owl__.storeHash = hashFunction({
-          if ("__owl__" in storeProps) {
+          state: env.store.state,
-            hashFunction = s => defaultRevFunction(s.storeProps);
+          storeProps: storeProps,
-          } else {
+          currentStoreProps: storeProps,
-            let areKeyObservable = false;
+          revNumber,
-            for (let key in storeProps) {
+          deepRevNumber
-              areKeyObservable =
+        });
-                areKeyObservable || (storeProps[key] && typeof (storeProps[key]) === "object" && "__owl__" in storeProps[key]);
-            }
-            if (areKeyObservable) {
-              hashFunction = function({ storeProps }) {
-                return Object.values(storeProps).reduce(
-                  (sum: number, val: any) => sum + defaultRevFunction(val),
-                  0
-                );
-              };
-            }
-          }
-        }
-        if (hashFunction) {
-          this.__owl__.storeHash = hashFunction({
-            state: env.store.state,
-            storeProps: storeProps,
-            revNumber,
-            deepRevNumber
-          });
-        }
       }
       mounted() {
         this.env.store.on("update", this, () => {
           const ownProps = this.__owl__.ownProps;
           const storeProps = mapStateToProps(this.env.store.state, ownProps);
           let didChange = false;
-          if (hashFunction) {
+          const storeHash = hashFunction({
-            const storeHash = hashFunction({
+            state: this.env.store.state,
-              state: this.env.store.state,
+            storeProps: storeProps,
-              storeProps: storeProps,
+            currentStoreProps: this.__owl__.currentStoreProps,
-              revNumber,
+            revNumber,
-              deepRevNumber
+            deepRevNumber
-            });
+          });
-            if (storeHash !== this.__owl__.storeHash) {
+          if (storeHash !== this.__owl__.storeHash) {
-              didChange = true;
+            didChange = true;
-              this.__owl__.storeHash = storeHash;
+            this.__owl__.storeHash = storeHash;
-            }
-          } else {
-            didChange = !shallowEqual(
-              storeProps,
-              this.__owl__.currentStoreProps
-            );
           }
           if (didChange) {
             this.__owl__.currentStoreProps = storeProps;

src/utils.ts

@@ -113,19 +113,6 @@ 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;
-}
-
 export function patch(C: any, patchName: string, patch: any) {
   const proto = C.prototype;
   if (!proto.__patches) {

tests/store.test.ts

@@ -737,6 +737,40 @@ describe("connecting a component to store", () => {
     expect(fixture.innerHTML).toBe("<div><span>kwak</span></div>");
   });
 
+  test("connected component is updated when store is changed", async () => {
+    class App extends Component<any, any, any> {
+      inlineTemplate = `
+          <div>
+              <span t-foreach="props.beers" t-as="beer"><t t-esc="beer.name"/></span>
+          </div>`;
+    }
+
+    const mutations = {
+      addBeer({ state }, name) {
+        state.beers.push({ name });
+      }
+    };
+
+    const state = { beers: [{ name: "jupiler" }] };
+    const store = new Store({ state, mutations });
+    (<any>env).store = store;
+
+    function mapStateToProps(state) {
+      return { beers: state.beers, otherKey: 1 };
+    }
+    const ConnectedApp = connect(mapStateToProps)(App);
+    const app = new ConnectedApp(env);
+
+    await app.mount(fixture);
+    expect(fixture.innerHTML).toBe("<div><span>jupiler</span></div>");
+
+    store.commit("addBeer", "kwak");
+    await nextTick();
+    expect(fixture.innerHTML).toBe(
+      "<div><span>jupiler</span><span>kwak</span></div>"
+    );
+  });
+
   test("connected component with undefined, null and string props", async () => {
     class Beer extends Component<any, any, any> {
       inlineTemplate = `<div>
@@ -749,7 +783,7 @@ describe("connecting a component to store", () => {
       return {
         selected: state.beers[props.id],
         consumed: state.beers[state.consumedID] || null,
-        taster: state.taster,
+        taster: state.taster
       };
     })(Beer);
 
@@ -778,16 +812,24 @@ describe("connecting a component to store", () => {
     const app = new App(env);
 
     await app.mount(fixture);
-    expect(fixture.innerHTML).toBe("<div><div><span>taster:aaron</span></div></div>");
+    expect(fixture.innerHTML).toBe(
+      "<div><div><span>taster:aaron</span></div></div>"
+    );
 
     await app.updateState({ beerId: 1 });
-    expect(fixture.innerHTML).toBe("<div><div><span>taster:aaron</span><span>selected:jupiler</span></div></div>");
+    expect(fixture.innerHTML).toBe(
+      "<div><div><span>taster:aaron</span><span>selected:jupiler</span></div></div>"
+    );
 
-    store.commit('consume', 1);
+    store.commit("consume", 1);
     await nextTick();
-    expect(fixture.innerHTML).toBe("<div><div><span>taster:aaron</span><span>selected:jupiler</span><span>consumed:jupiler</span></div></div>");
+    expect(fixture.innerHTML).toBe(
+      "<div><div><span>taster:aaron</span><span>selected:jupiler</span><span>consumed:jupiler</span></div></div>"
+    );
 
     await app.updateState({ beerId: 0 });
-    expect(fixture.innerHTML).toBe("<div><div><span>taster:aaron</span><span>consumed:jupiler</span></div></div>");
+    expect(fixture.innerHTML).toBe(
+      "<div><div><span>taster:aaron</span><span>consumed:jupiler</span></div></div>"
+    );
   });
 });

tests/utils.test.ts

@@ -5,7 +5,6 @@ import {
   memoize,
   debounce,
   findInTree,
-  shallowEqual,
   patch,
   unpatch
 } from "../src/utils";
@@ -95,17 +94,6 @@ describe("findInTree", () => {
   });
 });
 
-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);
-  });
-});
-
 describe("patch/unpatch", () => {
   test("can monkey patch a class", () => {
     class Test {