[FIX] component: call patch between willpatch and patched

closes #91

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`

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<VNode> {
     this.__owl__.renderProps = this.props;
@@ -387,11 +388,12 @@ export class Component<
     force: boolean = false,
     patchQueue: any[] = []
   ): Promise<VNode> {
-    if (this.__owl__.isMounted) {
-      patchQueue.push(this);
-    }
     this.__owl__.renderId++;
     const promises: Promise<void>[] = [];
+    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;
     }

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 = `<div>A<t t-widget="B"/><t t-widget="C"/></div>`;
+      widgets = { B, C };
+      name = "A";
+    }
+    class B extends TestWidget {
+      inlineTemplate = `<div>B</div>`;
+      name = "B";
+      constructor(parent, props) {
+        super(parent, props);
+        steps.push("B:constructor");
+      }
+    }
+    class C extends TestWidget {
+      inlineTemplate = `
+        <div>C<t t-widget="D"/>
+            <t t-if="state.flag" t-widget="E"/>
+            <t t-else="!state.flag" t-widget="F"/>
+        </div>`;
+      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 = `<div>D</div>`;
+      name = "D";
+      constructor(parent, props) {
+        super(parent, props);
+        steps.push("D:constructor");
+      }
+    }
+    class E extends TestWidget {
+      inlineTemplate = `<div>E</div>`;
+      name = "E";
+      constructor(parent, props) {
+        super(parent, props);
+        steps.push("E:constructor");
+      }
+    }
+
+    class F extends TestWidget {
+      inlineTemplate = `<div>F</div>`;
+      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(
+      `<div>A<div>B</div><div>C<div>D</div><div>E</div></div></div>`
+    );
+    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", () => {