From 65344dbf1ff3289e31b54b0f8c6e773d5b994c83 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Theys?= <seb@odoo.com>
Date: Wed, 27 Nov 2019 14:23:13 +0100
Subject: [PATCH] [FIX] store: fix useStore update when mixing state and props
 changes

This commit also fixes an issue where updates would not be triggered when the
new and old revNumber where the same but concerned a different object.
---
 src/store.ts              | 14 ++++++-------
 tests/store_hooks.test.ts | 43 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/src/store.ts b/src/store.ts
index 6571d358..289e99e1 100644
--- a/src/store.ts
+++ b/src/store.ts
@@ -96,14 +96,11 @@ export function useStore(selector, options: SelectorOptions = {}): any {
   if (!store.updateFunctions[componentId]) {
     store.updateFunctions[componentId] = [];
   }
-  store.updateFunctions[componentId].push(function(): boolean {
+  function selectCompareUpdate(state, props): boolean {
     const oldResult = result;
-    result = selector(store!.state, component.props);
+    result = selector(state, props);
     const newRevNumber = hashFn(result);
-    if (
-      (newRevNumber > 0 && revNumber !== newRevNumber) ||
-      (newRevNumber === 0 && !isEqual(oldResult, result))
-    ) {
+    if ((newRevNumber > 0 && revNumber !== newRevNumber) || !isEqual(oldResult, result)) {
       revNumber = newRevNumber;
       if (options.onUpdate) {
         options.onUpdate(result);
@@ -111,6 +108,9 @@ export function useStore(selector, options: SelectorOptions = {}): any {
       return true;
     }
     return false;
+  }
+  store.updateFunctions[componentId].push(function(): boolean {
+    return selectCompareUpdate(store!.state, component.props);
   });
 
   useContextWithCB(store, component, function(): Promise<void> | void {
@@ -123,7 +123,7 @@ export function useStore(selector, options: SelectorOptions = {}): any {
     }
   });
   onWillUpdateProps(props => {
-    result = selector(store.state, props);
+    selectCompareUpdate(store.state, props);
   });
 
   const __destroy = component.__destroy;
diff --git a/tests/store_hooks.test.ts b/tests/store_hooks.test.ts
index 262ca3b5..b8684289 100644
--- a/tests/store_hooks.test.ts
+++ b/tests/store_hooks.test.ts
@@ -490,6 +490,49 @@ describe("connecting a component to store", () => {
     expect(fixture.innerHTML).toBe("<div><span>jupiler</span><span>kwak</span></div>");
   });
 
+  test("connected component is updated when mixing store and props changes", async () => {
+    let counter = 0;
+
+    class Beer extends Component<any, any> {
+      static template = xml`<span><t t-esc="beer.name"/></span>`;
+      beer = useStore((state, props) => state.beers[props.id], {
+        onUpdate: result => {
+          ++counter;
+        }
+      });
+    }
+
+    class App extends Component<any, any> {
+      static template = xml`<div><Beer id="state.beerId"/></div>`;
+      static components = { Beer };
+      state = useState({ beerId: 1 });
+    }
+
+    const state = { beers: { 1: { name: "jupiler" }, 2: { name: "kwak" } } };
+    const actions = {
+      renameBeer({ state }, { id, name }) {
+        state.beers[id].name = name;
+      }
+    };
+    const store = new Store({ state, actions });
+    (<any>env).store = store;
+    const app = new App();
+
+    await app.mount(fixture);
+    expect(fixture.innerHTML).toBe("<div><span>jupiler</span></div>");
+    expect(counter).toBe(0);
+
+    app.state.beerId = 2;
+    await nextTick();
+    expect(fixture.innerHTML).toBe("<div><span>kwak</span></div>");
+    expect(counter).toBe(1);
+
+    store.dispatch("renameBeer", { id: 2, name: "orval" });
+    await nextTick();
+    expect(fixture.innerHTML).toBe("<div><span>orval</span></div>");
+    expect(counter).toBe(2);
+  });
+
   test("connected component is properly cleaned up on destroy", async () => {
     class App extends Component<any, any> {
       static template = xml`<div></div>`;