fix(router): update url and state only when needed

hybridsjs · Sep 27, 2021 · ddded3b · ddded3b
1 parent ee6ff70
commit ddded3b
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 34 deletions.
diff --git a/src/router.js b/src/router.js
@@ -329,24 +329,25 @@ function setupView(hybrids, routerOptions, parent, nestedParent) {
           _,
           connect,
           host =>
-            browserUrl.url(
-              browserUrl.paramsKeys.reduce((acc, key) => {
-                acc[key] = host[key];
-                return acc;
-              }, {}),
-              true,
-            ),
-          (host, url) => {
+            browserUrl.paramsKeys.reduce((acc, key) => {
+              acc[key] = String(mapUrlParam(host[key]));
+              return acc;
+            }, {}),
+          (host, params, lastParams) => {
+            if (!lastParams) return;
+
             const state = window.history.state;
             let entry = state[0];
             while (entry.nested) entry = entry.nested;
 
-            entry.params = browserUrl.paramsKeys.reduce((acc, key) => {
-              acc[key] = String(host[key] || "");
-              return acc;
-            }, {});
-
-            window.history.replaceState(state, "", url);
+            window.history.replaceState(
+              [
+                config.getEntry({ ...entry.params, ...params }),
+                ...state.slice(1),
+              ],
+              "",
+              config.url(params, true),
+            );
           },
         ),
       );

diff --git a/src/store.js b/src/store.js
@@ -1436,9 +1436,7 @@ function store(Model, options = {}) {
     options.draft = draft;
   }
 
-  const writable = !options.id && config.enumerable;
-
-  if (writable) {
+  if (!options.id && config.enumerable) {
     return {
       get(host, value) {
         const valueConfig = definitions.get(value);
@@ -1460,7 +1458,7 @@ function store(Model, options = {}) {
     get: (host, value) => {
       let id = (options.id && options.id(host)) || (value && value.id);
 
-      if (!id && options.draft) {
+      if (!id && !value && options.draft) {
         const draftModel = options.draft.create({});
         syncCache(options.draft, draftModel.id, draftModel, false);
         id = draftModel.id;
@@ -1476,6 +1474,7 @@ function store(Model, options = {}) {
 
       return nextValue;
     },
+    set: options.draft && !config.enumerable ? (_, v) => v : undefined,
   };
 }
 

diff --git a/test/spec/router.js b/test/spec/router.js
@@ -595,38 +595,45 @@ describe("router:", () => {
                   "#@test-router-nested-view-one",
                 );
 
-                host.querySelector("#RootView").click();
+                host.children[0].views[0].param = true;
 
                 return resolveTimeout(() => {
-                  expect(hybrids(host.children[0])).toBe(RootView);
-                  expect(window.history.state.length).toBe(1);
+                  expect(host.children[0].views[0].param).toBe(true);
+                  expect(window.location.search).toBe("?param=1");
 
-                  host.querySelector("#Dialog").click();
+                  host.querySelector("#RootView").click();
 
                   return resolveTimeout(() => {
                     expect(hybrids(host.children[0])).toBe(RootView);
-                    expect(hybrids(host.children[1])).toBe(Dialog);
-                    expect(window.history.state.length).toBe(2);
+                    expect(window.history.state.length).toBe(1);
 
-                    host.querySelector("#DialogCurrent").click();
+                    host.querySelector("#Dialog").click();
 
                     return resolveTimeout(() => {
                       expect(hybrids(host.children[0])).toBe(RootView);
                       expect(hybrids(host.children[1])).toBe(Dialog);
                       expect(window.history.state.length).toBe(2);
 
-                      const keyEventEsc = new KeyboardEvent("keydown", {
-                        key: "Escape",
-                      });
-                      const keyEventEnter = new KeyboardEvent("keydown", {
-                        key: "Enter",
-                      });
-                      host.children[1].dispatchEvent(keyEventEnter);
-                      host.children[1].dispatchEvent(keyEventEsc);
+                      host.querySelector("#DialogCurrent").click();
 
                       return resolveTimeout(() => {
                         expect(hybrids(host.children[0])).toBe(RootView);
-                        expect(window.history.state.length).toBe(1);
+                        expect(hybrids(host.children[1])).toBe(Dialog);
+                        expect(window.history.state.length).toBe(2);
+
+                        const keyEventEsc = new KeyboardEvent("keydown", {
+                          key: "Escape",
+                        });
+                        const keyEventEnter = new KeyboardEvent("keydown", {
+                          key: "Enter",
+                        });
+                        host.children[1].dispatchEvent(keyEventEnter);
+                        host.children[1].dispatchEvent(keyEventEsc);
+
+                        return resolveTimeout(() => {
+                          expect(hybrids(host.children[0])).toBe(RootView);
+                          expect(window.history.state.length).toBe(1);
+                        });
                       });
                     });
                   });

diff --git a/test/spec/store.js b/test/spec/store.js
@@ -1426,6 +1426,12 @@ describe("store:", () => {
         });
         el = document.createElement("test-store-factory-singleton-draft");
         expect(el.draft).toEqual({ value: "test" });
+
+        return store.set(el.draft, { value: "other" }).then(() => {
+          expect(el.draft).toEqual({ value: "other" });
+          el.draft = undefined;
+          expect(el.draft).toEqual({ value: "test" });
+        });
       });
     });