From 7f33f9b68eebd68efdb9301903121f2fb7e9c207 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Wed, 8 May 2019 15:21:33 +0200 Subject: [PATCH] [FIX] component: call patch between willpatch and patched closes #91 --- doc/component.md | 7 +- src/component.ts | 49 +++++++------ tests/component.test.ts | 154 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 184 insertions(+), 26 deletions(-) diff --git a/doc/component.md b/doc/component.md index 6d6b7f0d..19c2c99a 100644 --- a/doc/component.md +++ b/doc/component.md @@ -418,15 +418,16 @@ Here is what Owl will do: 2. template `D` is rerendered - widget `F` is created: 1. hook `willStart` is called on `E` (async) - 2. template `F` is rerendered + 2. template `F` is rendered 3. `willPatch` hooks are called recursively on widgets `C`, `D` (not on `F`, because it is not mounted yet) 4. widget `C` is patched, which will cause recursively: - 1. patching of `D`, 2. `willUnmount` hook on `E`, then destruction of `E`, 3. (initial) patching of `F`, then hook `mounted` is called on `F` -5. `patched` hooks are called on `D`, `C` +5. patching of `D`, + +6. `patched` hooks are called on `D`, `C` diff --git a/src/component.ts b/src/component.ts index a8fc489d..6b0b9a1e 100644 --- a/src/component.ts +++ b/src/component.ts @@ -252,29 +252,33 @@ export class Component< if (!this.__owl__.isMounted) { return; } - const shouldCallPatchHooks: boolean = !patchQueue; - if (shouldCallPatchHooks) { + const shouldPatch: boolean = !patchQueue; + if (shouldPatch) { patchQueue = []; } const renderVDom = this._render(force, patchQueue); const renderId = this.__owl__.renderId; - const vnode = await renderVDom; + await renderVDom; - if (this.__owl__.isMounted && renderId === this.__owl__.renderId) { + if ( + shouldPatch && + this.__owl__.isMounted && + renderId === this.__owl__.renderId + ) { // we only update the vnode and the actual DOM if no other rendering // occurred between now and when the render method was initially called. - if (shouldCallPatchHooks) { - for (let i = 0; i < patchQueue!.length; i++) { - const c = patchQueue![i]; - c.__owl__.willPatchVal = c.willPatch(); - } + for (let i = 0; i < patchQueue!.length; i++) { + const patch = patchQueue![i]; + patch.push(patch[0].willPatch()); } - this._patch(vnode); - if (shouldCallPatchHooks) { - for (let i = patchQueue!.length - 1; i >= 0; i--) { - const c = patchQueue![i]; - c.patched(c.__owl__.willPatchVal); - } + for (let i = 0; i < patchQueue!.length; i++) { + const patch = patchQueue![i]; + patch[0]._patch(patch[1]); + } + + for (let i = patchQueue!.length - 1; i >= 0; i--) { + const patch = patchQueue![i]; + patch[0].patched(patch[2]); } } } @@ -355,11 +359,8 @@ export class Component< _patch(vnode) { this.__owl__.renderPromise = null; - if (this.__owl__.vnode) { - this.__owl__.vnode = patch(this.__owl__.vnode, vnode); - } else { - this.__owl__.vnode = patch(document.createElement(vnode.sel!), vnode); - } + const target = this.__owl__.vnode || document.createElement(vnode.sel!); + this.__owl__.vnode = patch(target, vnode); } _prepare(): Promise { this.__owl__.renderProps = this.props; @@ -387,11 +388,12 @@ export class Component< force: boolean = false, patchQueue: any[] = [] ): Promise { - if (this.__owl__.isMounted) { - patchQueue.push(this); - } this.__owl__.renderId++; const promises: Promise[] = []; + const patch: any[] = [this]; + if (this.__owl__.isMounted) { + patchQueue.push(patch); + } if (this.__owl__.observer) { this.__owl__.observer.allowMutations = false; } @@ -401,6 +403,7 @@ export class Component< forceUpdate: force, patchQueue }); + patch.push(vnode); if (this.__owl__.observer) { this.__owl__.observer.allowMutations = true; } diff --git a/tests/component.test.ts b/tests/component.test.ts index 246356b3..c7540c46 100644 --- a/tests/component.test.ts +++ b/tests/component.test.ts @@ -1491,6 +1491,160 @@ describe("random stuff/miscellaneous", () => { await widget.mount(fixture); expect(env.qweb.templates.parent.fn.toString()).toMatchSnapshot(); }); + + test("component semantics", async () => { + let steps: string[] = []; + let c: C; + + class TestWidget extends Widget { + name: string = "test"; + async willStart() { + steps.push(`${this.name}:willStart`); + } + _render(f, p) { + steps.push(`${this.name}:render`); + return super._render(f, p); + } + _patch(vnode) { + steps.push(`${this.name}:_patch`); + super._patch(vnode); + } + _mount(vnode, elm) { + steps.push(`${this.name}:_patch(from _mount)`); + return super._mount(vnode, elm); + } + mounted() { + steps.push(`${this.name}:mounted`); + } + async willUpdateProps() { + steps.push(`${this.name}:willUpdateProps`); + } + willPatch() { + steps.push(`${this.name}:willPatch`); + } + patched() { + steps.push(`${this.name}:patched`); + } + willUnmount() { + steps.push(`${this.name}:willUnmount`); + } + destroy() { + super.destroy(); + steps.push(`${this.name}:destroy`); + } + } + class A extends TestWidget { + inlineTemplate = `
A
`; + widgets = { B, C }; + name = "A"; + } + class B extends TestWidget { + inlineTemplate = `
B
`; + name = "B"; + constructor(parent, props) { + super(parent, props); + steps.push("B:constructor"); + } + } + class C extends TestWidget { + inlineTemplate = ` +
C + + +
`; + widgets = { D, E, F }; + name = "C"; + state = {flag: true}; + + constructor(parent, props) { + super(parent, props); + c = this; + steps.push("C:constructor"); + } + } + + class D extends TestWidget { + inlineTemplate = `
D
`; + name = "D"; + constructor(parent, props) { + super(parent, props); + steps.push("D:constructor"); + } + } + class E extends TestWidget { + inlineTemplate = `
E
`; + name = "E"; + constructor(parent, props) { + super(parent, props); + steps.push("E:constructor"); + } + } + + class F extends TestWidget { + inlineTemplate = `
F
`; + name = "F"; + constructor(parent, props) { + super(parent, props); + steps.push("F:constructor"); + } + } + + const a = new A(env); + await a.mount(fixture); + expect(fixture.innerHTML).toBe( + `
A
B
C
D
E
` + ); + expect(steps).toEqual([ + "A:willStart", + "A:render", + "B:constructor", + "B:willStart", + "C:constructor", + "C:willStart", + "B:render", + "C:render", + "D:constructor", + "D:willStart", + "E:constructor", + "E:willStart", + "D:render", + "E:render", + "A:_patch", + "B:_patch(from _mount)", + "C:_patch(from _mount)", + "D:_patch(from _mount)", + "E:_patch(from _mount)", + "B:mounted", + "D:mounted", + "E:mounted", + "C:mounted", + "A:mounted", + ]); + + // update + steps = []; + c!.state.flag = false; + await nextTick(); + expect(steps).toEqual([ + "C:render", + "D:willUpdateProps", + "F:constructor", + "F:willStart", + "D:render", + "F:render", + "C:willPatch", + "D:willPatch", + "C:_patch", + "E:willUnmount", + "E:destroy", + "E:destroy", // maybe should look into this + "F:_patch(from _mount)", + "F:mounted", + "D:_patch", + "D:patched", + "C:patched" + ]); + }); }); describe("async rendering", () => {