[FIX] router: properly react on hashchange

Before this commit, hashchanges were not taken into account by the
router, if used in history mode.

Also, it's stupid, but i ran prettier on the codebase
This commit is contained in:
Géry Debongnie
2019-09-01 11:45:13 +02:00
parent 40d8090232
commit f9861040c8
10 changed files with 73 additions and 55 deletions
+5 -5
View File
@@ -456,11 +456,11 @@ QWeb.addDirective({
// need to update component // need to update component
let patchQueueCode = async ? `patchQueue${componentID}` : "extra.patchQueue"; let patchQueueCode = async ? `patchQueue${componentID}` : "extra.patchQueue";
if (keepAlive) { if (keepAlive) {
// if we have t-keepalive="1", the component could be unmounted, but then // if we have t-keepalive="1", the component could be unmounted, but then
// we __updateProps is called. This is ok, but we do not want to call // we __updateProps is called. This is ok, but we do not want to call
// the willPatch/patched hooks of the component in this case, so we // the willPatch/patched hooks of the component in this case, so we
// disable the patch queue // disable the patch queue
patchQueueCode = `w${componentID}.__owl__.isMounted ? ${patchQueueCode} : []`; patchQueueCode = `w${componentID}.__owl__.isMounted ? ${patchQueueCode} : []`;
} }
if (QWeb.dev) { if (QWeb.dev) {
ctx.addLine(`utils.validateProps(w${componentID}.constructor, props${componentID})`); ctx.addLine(`utils.validateProps(w${componentID}.constructor, props${componentID})`);
-1
View File
@@ -25,7 +25,6 @@ export class Observer {
notifyCB() {} notifyCB() {}
async notifyChange() { async notifyChange() {
this.dirty = true; this.dirty = true;
await Promise.resolve(); await Promise.resolve();
if (this.dirty) { if (this.dirty) {
+8 -3
View File
@@ -85,9 +85,6 @@ export class Router {
this.routeIds.push(partialRoute.name); this.routeIds.push(partialRoute.name);
} }
(this as any)._listener = () => this.matchAndApplyRules(this.currentPath());
window.addEventListener("popstate", (this as any)._listener);
// setup link and directive // setup link and directive
env.qweb.addTemplate(LINK_TEMPLATE_NAME, LINK_TEMPLATE); env.qweb.addTemplate(LINK_TEMPLATE_NAME, LINK_TEMPLATE);
QWeb.addDirective(makeDirective(<RouterEnv>env)); QWeb.addDirective(makeDirective(<RouterEnv>env));
@@ -98,6 +95,11 @@ export class Router {
//-------------------------------------------------------------------------- //--------------------------------------------------------------------------
async start() { async start() {
(this as any)._listener = () => this._navigate(this.currentPath());
window.addEventListener("popstate", (this as any)._listener);
if (this.mode === "hash") {
window.addEventListener("hashchange", (this as any)._listener);
}
const result = await this.matchAndApplyRules(this.currentPath()); const result = await this.matchAndApplyRules(this.currentPath());
if (result.type === "match") { if (result.type === "match") {
this.currentRoute = result.route; this.currentRoute = result.route;
@@ -111,6 +113,9 @@ export class Router {
async navigate(to: Destination): Promise<boolean> { async navigate(to: Destination): Promise<boolean> {
const path = this.destToPath(to); const path = this.destToPath(to);
return this._navigate(path);
}
async _navigate(path: string): Promise<boolean> {
const initialName = this.currentRouteName; const initialName = this.currentRouteName;
const initialParams = this.currentParams; const initialParams = this.currentParams;
const result = await this.matchAndApplyRules(path); const result = await this.matchAndApplyRules(path);
+8 -8
View File
@@ -42,7 +42,7 @@ export class Store extends EventBus {
debug: boolean; debug: boolean;
env: any; env: any;
observer: Observer; observer: Observer;
getters: { [name: string]: (payload?) => any }; getters: { [name: string]: (payload?) => any };
constructor(config: StoreConfig, options: StoreOption = {}) { constructor(config: StoreConfig, options: StoreOption = {}) {
super(); super();
@@ -54,13 +54,13 @@ export class Store extends EventBus {
this.state = this.observer.observe(config.state || {}); this.state = this.observer.observe(config.state || {});
this.getters = {}; this.getters = {};
if (config.getters) { if (config.getters) {
const firstArg = { const firstArg = {
state: this.state, state: this.state,
getters: this.getters, getters: this.getters
}; };
for (let g in config.getters) { for (let g in config.getters) {
this.getters[g] = config.getters[g].bind(this, firstArg); this.getters[g] = config.getters[g].bind(this, firstArg);
} }
} }
} }
+1 -1
View File
@@ -526,7 +526,7 @@ const htmlDomApi = {
parentNode, parentNode,
nextSibling, nextSibling,
tagName, tagName,
setTextContent, setTextContent
} as DOMAPI; } as DOMAPI;
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------
+5 -6
View File
@@ -280,7 +280,7 @@ describe("props validation", () => {
test("props: extra props cause an error, part 2", async () => { test("props: extra props cause an error, part 2", async () => {
class TestWidget extends Widget { class TestWidget extends Widget {
static props = {message: true}; static props = { message: true };
} }
expect(() => { expect(() => {
@@ -294,23 +294,22 @@ describe("props validation", () => {
} }
expect(() => { expect(() => {
QWeb.utils.validateProps(TestWidget, { message: 1}); QWeb.utils.validateProps(TestWidget, { message: 1 });
}).not.toThrow(); }).not.toThrow();
}); });
test("optional prop do not cause an error if value is undefined", async () => { test("optional prop do not cause an error if value is undefined", async () => {
class TestWidget extends Widget { class TestWidget extends Widget {
static props = {message: {type: String, optional: true}}; static props = { message: { type: String, optional: true } };
} }
expect(() => { expect(() => {
QWeb.utils.validateProps(TestWidget, { message: undefined}); QWeb.utils.validateProps(TestWidget, { message: undefined });
}).not.toThrow(); }).not.toThrow();
expect(() => { expect(() => {
QWeb.utils.validateProps(TestWidget, { message: null}); QWeb.utils.validateProps(TestWidget, { message: null });
}).toThrow(); }).toThrow();
}); });
}); });
describe("default props", () => { describe("default props", () => {
+1 -1
View File
@@ -65,7 +65,7 @@ describe("observer", () => {
expect(observer.revNumber(obj)).toBe(1); expect(observer.revNumber(obj)).toBe(1);
expect(observer.deepRevNumber(obj)).toBe(1); expect(observer.deepRevNumber(obj)).toBe(1);
expect(observer.rev).toBe(1); expect(observer.rev).toBe(1);
expect(typeof obj.date.getFullYear()).toBe('number'); expect(typeof obj.date.getFullYear()).toBe("number");
expect(obj.date).toBe(date); expect(obj.date).toBe(date);
obj.date = new Date(); obj.date = new Date();
+16 -1
View File
@@ -1,5 +1,5 @@
import { Destination, RouterEnv, Route } from "../../src/router/Router"; import { Destination, RouterEnv, Route } from "../../src/router/Router";
import { makeTestEnv } from "../helpers"; import { makeTestEnv, nextTick } from "../helpers";
import { TestRouter } from "./TestRouter"; import { TestRouter } from "./TestRouter";
let env: RouterEnv; let env: RouterEnv;
@@ -37,6 +37,21 @@ describe("router miscellaneous", () => {
expect(window.location.pathname).toBe("/users/5"); expect(window.location.pathname).toBe("/users/5");
expect(env.qweb.forceUpdate).toHaveBeenCalledTimes(2); expect(env.qweb.forceUpdate).toHaveBeenCalledTimes(2);
}); });
test("changing url to same route but with different params should trigger update (hash mode)", async () => {
env.qweb.forceUpdate = jest.fn();
router = new TestRouter(env, [{ name: "users", path: "/users/{{id}}" }], { mode: "hash" });
await router.start();
await router.navigate({ to: "users", params: { id: 3 } });
expect(window.location.hash).toBe("#/users/3");
expect(env.qweb.forceUpdate).toHaveBeenCalledTimes(1);
window.location.hash = "/users/5";
window.dispatchEvent(new Event("hashchange"));
await nextTick();
expect(window.location.hash).toBe("#/users/5");
expect(env.qweb.forceUpdate).toHaveBeenCalledTimes(2);
});
}); });
describe("routeToPath", () => { describe("routeToPath", () => {
+5 -5
View File
@@ -616,8 +616,8 @@ describe("connecting a component to store", () => {
return { flag: s.flag, someId: s.someId }; return { flag: s.flag, someId: s.someId };
} }
async render(force) { async render(force) {
await def; await def;
return super.render(force); return super.render(force);
} }
} }
@@ -628,7 +628,7 @@ describe("connecting a component to store", () => {
} }
} }
const state = { someId: 1, flag: true, messages: {1: "abc"}}; const state = { someId: 1, flag: true, messages: { 1: "abc" } };
const actions = { const actions = {
setFlagToFalse({ state }) { setFlagToFalse({ state }) {
state.flag = false; state.flag = false;
@@ -1040,7 +1040,7 @@ describe("connected components and default values", () => {
super.off(eventType, owner); super.off(eventType, owner);
} }
} }
const store = new TestStore({ state: {val: 1} }); const store = new TestStore({ state: { val: 1 } });
(<any>env).store = store; (<any>env).store = store;
const parent = new Parent(env); const parent = new Parent(env);
@@ -1088,7 +1088,7 @@ describe("connected components and default values", () => {
await app.mount(fixture); await app.mount(fixture);
expect(fixture.innerHTML).toBe("<div>0</div>"); expect(fixture.innerHTML).toBe("<div>0</div>");
const res = app.dispatch('inc'); const res = app.dispatch("inc");
expect(res).toBe(1); expect(res).toBe(1);
await nextTick(); await nextTick();
expect(fixture.innerHTML).toBe("<div>1</div>"); expect(fixture.innerHTML).toBe("<div>1</div>");
+24 -24
View File
@@ -249,32 +249,32 @@ describe("advanced state properties", () => {
expect(store.state.a).toEqual([1, 2, 3, 53]); expect(store.state.a).toEqual([1, 2, 3, 53]);
}); });
test("can use object assign in store", async () => { test("can use object assign in store", async () => {
const actions = { const actions = {
dosomething({ state }) { dosomething({ state }) {
Object.assign(state.westmalle, { a: 3, b: 4 }); Object.assign(state.westmalle, { a: 3, b: 4 });
} }
}; };
const store = new Store({ const store = new Store({
state: { westmalle: { a: 1, b: 2 } }, state: { westmalle: { a: 1, b: 2 } },
actions actions
});
store.dispatch("dosomething");
expect(store.state.westmalle).toEqual({ a: 3, b: 4 });
}); });
store.dispatch("dosomething");
expect(store.state.westmalle).toEqual({ a: 3, b: 4 });
});
test("aku reactive store state 1", async () => { test("aku reactive store state 1", async () => {
const actions = { const actions = {
inc({ state }) { inc({ state }) {
state.counter++; state.counter++;
} }
}; };
const state = { counter: 0 }; const state = { counter: 0 };
const store = new Store({ state, actions }); const store = new Store({ state, actions });
expect(store.state.counter).toBe(0); expect(store.state.counter).toBe(0);
store.dispatch("inc", {}); store.dispatch("inc", {});
expect(store.state.counter).toBe(1); expect(store.state.counter).toBe(1);
}); });
}); });
describe("updates triggered by the store", () => { describe("updates triggered by the store", () => {