From 94ed6ebe606aaa545aad64855a14b92c4b3f0d6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Wed, 17 Apr 2019 10:07:31 +0200 Subject: [PATCH] [IMP] component: remove updateState method --- src/component.ts | 27 ++--------- tests/component.test.ts | 101 +++++++++++++++++++++++----------------- tests/store.test.ts | 18 ++++--- 3 files changed, 72 insertions(+), 74 deletions(-) diff --git a/src/component.ts b/src/component.ts index 11d1cc40..0a0418c8 100644 --- a/src/component.ts +++ b/src/component.ts @@ -230,12 +230,12 @@ export class Component< } this._patch(vnode); target.appendChild(this.el!); + this._observeState(); if (document.body.contains(target)) { this._visitSubTree(w => { if (!w.__owl__.isMounted && this.el!.contains(w.el)) { w.__owl__.isMounted = true; - this._observeState(); w.mounted(); return true; } @@ -326,25 +326,6 @@ export class Component< return shouldUpdate ? this._updateProps(nextProps) : Promise.resolve(); } - /** - * This is the safest update method for widget: its job is to update the state - * and rerender (if widget is mounted). - * - * Notes: - * - it checks if we do not add extra keys to the state. - * - it is ok to call updateState before the widget is started. In that - * case, it will simply update the state and will not rerender - */ - async updateState(nextState: Partial) { - if (Object.keys(nextState).length === 0) { - return; - } - Object.assign(this.state, nextState); - if (this.__owl__.isStarted) { - await this.render(); - } - } - //-------------------------------------------------------------------------- // Private //-------------------------------------------------------------------------- @@ -418,10 +399,10 @@ export class Component< if (this.__owl__.isMounted) { return; } + this._observeState(); if (this.__owl__.parent) { if (this.__owl__.parent!.__owl__.isMounted) { this.__owl__.isMounted = true; - this._observeState(); this.mounted(); const children = this.__owl__.children; for (let id in children) { @@ -444,9 +425,7 @@ export class Component< _observeState() { if (Object.keys(this.state).length) { this.__owl__.observer.observe(this.state); - this.__owl__.observer.notifyCB = () => { - this.render(); - }; + this.__owl__.observer.notifyCB = this.render.bind(this); } } } diff --git a/tests/component.test.ts b/tests/component.test.ts index 4174d46b..aabb2d80 100644 --- a/tests/component.test.ts +++ b/tests/component.test.ts @@ -45,14 +45,13 @@ function children(w: Widget): Widget[] { // Test widgets class Counter extends Widget { - // class Counter extends Widget { template = "counter"; state = { counter: 0 }; inc() { - this.updateState({ counter: this.state.counter + 1 }); + this.state.counter++; } } @@ -94,7 +93,7 @@ describe("basic widget properties", () => { expect(target.innerHTML).toBe("
0
"); const button = (counter.el).getElementsByTagName("button")[0]; await button.click(); - await nextMicroTick(); + await nextTick(); expect(target.innerHTML).toBe("
1
"); }); @@ -109,11 +108,12 @@ describe("basic widget properties", () => { ); }); - test("updateState before first render does not trigger a render", async () => { + test("changing state before first render does not trigger a render", async () => { let renderCalls = 0; class TestW extends Widget { + state = { drinks: 1 }; async willStart() { - this.updateState({}); + this.state.drinks++; } async _render() { renderCalls++; @@ -125,15 +125,6 @@ describe("basic widget properties", () => { expect(renderCalls).toBe(1); }); - test("updateState does not allow adding extra keys", async () => { - const widget = new Widget(env); - try { - await widget.updateState({ extra: 1 }); - } catch (e) { - expect(e.message).toMatch("Invalid key:"); - } - }); - test("keeps a reference to env", async () => { const widget = new Widget(env); expect(widget.env).toBe(env); @@ -317,7 +308,8 @@ describe("lifecycle hooks", () => { const widget = new ParentWidget(env); await widget.mount(fixture); expect(steps).toEqual(["init", "willstart", "mounted"]); - await widget.updateState({ ok: false }); + widget.state.ok = false; + await nextTick(); expect(steps).toEqual(["init", "willstart", "mounted", "willunmount"]); }); @@ -329,7 +321,7 @@ describe("lifecycle hooks", () => { childUnmounted = true; } increment() { - this.updateState({ n: this.state.n + 1 }); + this.state += 1; } } @@ -343,10 +335,10 @@ describe("lifecycle hooks", () => { `; state = { n: 0, flag: true }; increment() { - this.updateState({ n: this.state.n + 1 }); + this.state.n += 1; } toggleSubWidget() { - this.updateState({ flag: !this.state.flag }); + this.state.flag = !this.state.flag; } } @@ -433,7 +425,7 @@ describe("lifecycle hooks", () => { expect(fixture.innerHTML).toBe("2"); }); - test("patched hook is called after updateState", async () => { + test("patched hook is called after updating State", async () => { let n = 0; class TestWidget extends Widget { @@ -447,10 +439,12 @@ describe("lifecycle hooks", () => { await widget.mount(fixture); expect(n).toBe(0); - await widget.updateState({}); // empty update, should do nothing + widget.state.a = 1; // empty update, should do nothing + await nextTick(); expect(n).toBe(0); - await widget.updateState({ a: 3 }); + widget.state.a = 3; + await nextTick(); expect(n).toBe(1); }); @@ -533,10 +527,11 @@ describe("lifecycle hooks", () => { await widget.mount(fixture); expect(created).toBe(false); expect(mounted).toBe(false); - await widget.updateState({ flag: true }); + + widget.state.flag = true; + await nextTick(); expect(mounted).toBe(true); expect(created).toBe(true); - await widget.updateState({ flag: false }); }); test("willPatch/patched hook", async () => { @@ -674,7 +669,7 @@ describe("composition", () => { ); const button = fixture.getElementsByTagName("button")[0]; await button.click(); - await nextMicroTick(); + await nextTick(); expect(fixture.innerHTML).toBe( "
1
" ); @@ -709,7 +704,7 @@ describe("composition", () => { await widget.mount(fixture); const button = fixture.getElementsByTagName("button")[0]; await button.click(); - await nextMicroTick(); + await nextTick(); expect(fixture.innerHTML).toBe( "
1
" ); @@ -729,13 +724,16 @@ describe("composition", () => { await widget.mount(fixture); const button = fixture.getElementsByTagName("button")[0]; await button.click(); - await nextMicroTick(); + await nextTick(); expect(fixture.innerHTML).toBe( "
1
" ); - await widget.updateState({ ok: false }); + widget.state.ok = false; + await nextTick(); expect(fixture.innerHTML).toBe("
"); - await widget.updateState({ ok: true }); + + widget.state.ok = true; + await nextTick(); expect(fixture.innerHTML).toBe( "
0
" ); @@ -751,16 +749,20 @@ describe("composition", () => { await widget.mount(fixture); const button = fixture.getElementsByTagName("button")[0]; await button.click(); - await nextMicroTick(); + await nextTick(); expect(fixture.innerHTML).toBe( "
1
" ); const counter = children(widget)[0]; expect(counter.__owl__.isMounted).toBe(true); - await widget.updateState({ ok: false }); + + widget.state.ok = false; + await nextTick(); expect(fixture.innerHTML).toBe("
"); expect(counter.__owl__.isMounted).toBe(false); - await widget.updateState({ ok: true }); + + widget.state.ok = true; + await nextTick(); expect(counter.__owl__.isMounted).toBe(true); expect(fixture.innerHTML).toBe( "
1
" @@ -780,9 +782,12 @@ describe("composition", () => { await widget.mount(fixture); const input = fixture.getElementsByTagName("input")[0]; input.value = "test"; - await widget.updateState({ ok: false }); + widget.state.ok = false; + await nextTick(); expect(fixture.innerHTML).toBe("
"); - await widget.updateState({ ok: true }); + + widget.state.ok = true; + await nextTick(); expect(fixture.innerHTML).toBe("
"); const input2 = fixture.getElementsByTagName("input")[0]; expect(input).toBe(input2); @@ -847,7 +852,8 @@ describe("composition", () => { } const parent = new Parent(env); await parent.mount(fixture); - await parent.updateState({ numbers: [1, 3] }); + parent.state.numbers = [1, 3]; + await nextTick(); expect(normalize(fixture.innerHTML)).toBe( normalize(`
@@ -878,7 +884,9 @@ describe("composition", () => { const parent = new Parent(env); await parent.mount(fixture); const child = children(parent)[0]; - await parent.updateState({ flag: true }); + + parent.state.flag = true; + await nextTick(); expect(children(parent)[0]).toBe(child); expect(child.__owl__.isDestroyed).toBe(false); expect(normalize(fixture.innerHTML)).toBe( @@ -1013,10 +1021,12 @@ describe("other directives with t-widget", () => { expect(fixture.innerHTML).toBe("
hey
"); - await widget.updateState({ flag: false }); + widget.state.flag = false; + await nextTick(); expect(fixture.innerHTML).toBe("
"); - await widget.updateState({ flag: true }); + widget.state.flag = true; + await nextTick(); expect(fixture.innerHTML).toBe("
hey
"); }); @@ -1039,7 +1049,8 @@ describe("other directives with t-widget", () => { expect(normalize(fixture.innerHTML)).toBe("
somediv
"); - await widget.updateState({ flag: false }); + widget.state.flag = false; + await nextTick(); expect(normalize(fixture.innerHTML)).toBe("
hey
"); }); @@ -1062,7 +1073,8 @@ describe("other directives with t-widget", () => { expect(normalize(fixture.innerHTML)).toBe("
somediv
"); - await widget.updateState({ flag: false }); + widget.state.flag = false; + await nextTick(); expect(normalize(fixture.innerHTML)).toBe("
hey
"); }); }); @@ -1098,7 +1110,8 @@ describe("random stuff/miscellaneous", () => { const widget = new Parent(env); await widget.mount(fixture); expect(fixture.innerHTML).toBe("
abc
"); - await widget.updateState({ flag: true }); + widget.state.flag = true; + await nextTick(); expect(fixture.innerHTML).toBe("
abcdef
"); }); @@ -1225,10 +1238,10 @@ describe("async rendering", () => { const parent = new Parent(env); await parent.mount(fixture); expect(fixture.innerHTML.replace(/\r?\n|\r|\s+/g, "")).toBe("
"); - parent.updateState({ flagA: true }); + parent.state.flagA = true; await nextTick(); expect(fixture.innerHTML.replace(/\r?\n|\r|\s+/g, "")).toBe("
"); - parent.updateState({ flagB: true }); + parent.state.flagB = true; await nextTick(); expect(fixture.innerHTML.replace(/\r?\n|\r|\s+/g, "")).toBe("
"); defB.resolve(); @@ -1271,12 +1284,12 @@ describe("async rendering", () => { expect(fixture.innerHTML.replace(/\r?\n|\r|\s+/g, "")).toBe( "
a1
" ); - parent.updateState({ valA: 2 }); + parent.state.valA = 2; await nextTick(); expect(fixture.innerHTML.replace(/\r?\n|\r|\s+/g, "")).toBe( "
a1
" ); - parent.updateState({ flagB: true }); + parent.state.flagB = true; await nextTick(); expect(fixture.innerHTML.replace(/\r?\n|\r|\s+/g, "")).toBe( "
a1
" diff --git a/tests/store.test.ts b/tests/store.test.ts index 0a3b7bcf..9f102ec5 100644 --- a/tests/store.test.ts +++ b/tests/store.test.ts @@ -406,7 +406,8 @@ describe("connecting a component to store", () => { await parent.mount(fixture); expect(steps).toEqual(["child:mounted"]); - await parent.updateState({ child: false }); + parent.state.child = false; + await nextTick(); expect(steps).toEqual(["child:mounted", "child:willUnmount"]); }); @@ -479,7 +480,8 @@ describe("connecting a component to store", () => { await app.mount(fixture); expect(fixture.innerHTML).toBe("
jupiler
"); - await app.updateState({ beerId: 2 }); + app.state.beerId = 2; + await nextTick(); expect(fixture.innerHTML).toBe("
kwak
"); }); @@ -562,7 +564,8 @@ describe("connecting a component to store", () => { "
taster:aaron
" ); - await app.updateState({ beerId: 1 }); + app.state.beerId = 1; + await nextTick(); expect(fixture.innerHTML).toBe( "
taster:aaronselected:jupiler
" ); @@ -573,7 +576,8 @@ describe("connecting a component to store", () => { "
taster:aaronselected:jupilerconsumed:jupiler
" ); - await app.updateState({ beerId: 0 }); + app.state.beerId = 0; + await nextTick(); expect(fixture.innerHTML).toBe( "
taster:aaronconsumed:jupiler
" ); @@ -630,7 +634,8 @@ describe("connecting a component to store", () => { "
taster:aaron
" ); - await app.updateState({ beerId: 1 }); + app.state.beerId = 1; + await nextTick(); expect(fixture.innerHTML).toBe( "
taster:aaronselected:jupiler
" ); @@ -647,7 +652,8 @@ describe("connecting a component to store", () => { "
taster:aaronselected:kwakconsumed:kwak
" ); - await app.updateState({ beerId: 0 }); + app.state.beerId = 0; + await nextTick(); expect(fixture.innerHTML).toBe( "
taster:aaronconsumed:kwak
" );