[FIX] component: call willPatch/patched hooks properly

Each component called its hooks properly individually, however the issue
was that it was done component by component.  However, in practice, we
want the willPatch hooks to be done before ANY patch is actually
performed, and the patched should be called AFTER every patch is
applied.

The trick is to propagate a list of patched components across various
function calls.

closes #64

src/component.ts

@@ -236,17 +236,33 @@ export class Component<
     }
   }
 
-  async render(force: boolean = false): Promise<void> {
+  async render(force: boolean = false, patchQueue?: any[]): Promise<void> {
     if (this.__owl__.isDestroyed) {
       return;
     }
-    const renderVDom = this._render(force);
+    const shouldCallPatchHooks: boolean = !patchQueue;
+    if (shouldCallPatchHooks) {
+      patchQueue = [];
+    }
+    const renderVDom = this._render(force, patchQueue);
     const renderId = this.__owl__.renderId;
     const vnode = await renderVDom;
     if (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();
+        }
+      }
       this._patch(vnode);
+      if (shouldCallPatchHooks) {
+        for (let i = patchQueue!.length - 1; i >= 0; i--) {
+          const c = patchQueue![i];
+          c.patched(c.__owl__.willPatchVal);
+        }
+      }
     }
   }
 
@@ -305,10 +321,11 @@ export class Component<
 
   async updateProps(
     nextProps: Props,
-    forceUpdate: boolean = false
+    forceUpdate: boolean = false,
+    patchQueue: any[],
   ): Promise<void> {
     const shouldUpdate = forceUpdate || this.shouldUpdate(nextProps);
-    return shouldUpdate ? this._updateProps(nextProps) : Promise.resolve();
+    return shouldUpdate ? this._updateProps(nextProps, patchQueue) : Promise.resolve();
   }
 
   set(target: any, key: string | number, value: any) {
@@ -319,21 +336,16 @@ export class Component<
   // Private
   //--------------------------------------------------------------------------
 
-  async _updateProps(nextProps: Props): Promise<void> {
+  async _updateProps(nextProps: Props, patchQueue: any[]): Promise<void> {
     await this.willUpdateProps(nextProps);
     this.props = nextProps;
-    await this.render();
+    await this.render(false, patchQueue);
   }
 
   _patch(vnode) {
     this.__owl__.renderPromise = null;
     if (this.__owl__.vnode) {
-      const isMounted = this.__owl__.isMounted;
-      const snapshot = isMounted && this.willPatch();
       this.__owl__.vnode = patch(this.__owl__.vnode, vnode);
-      if (isMounted) {
-        this.patched(snapshot);
-      }
     } else {
       this.__owl__.vnode = patch(document.createElement(vnode.sel!), vnode);
     }
@@ -358,7 +370,10 @@ export class Component<
     return this.__owl__.renderPromise;
   }
 
-  async _render(force: boolean = false): Promise<VNode> {
+  async _render(force: boolean = false, patchQueue: any[] = []): Promise<VNode> {
+    if (this.__owl__.isMounted) {
+      patchQueue.push(this)
+    }
     this.__owl__.renderId++;
     const promises: Promise<void>[] = [];
     const template = this.inlineTemplate || this.template;
@@ -368,7 +383,8 @@ export class Component<
     let vnode = this.env.qweb.render(template, this, {
       promises,
       handlers: this.__owl__.boundHandlers,
-      forceUpdate: force
+      forceUpdate: force,
+      patchQueue,
     });
     if (this.__owl__.observer) {
       this.__owl__.observer.allowMutations = true;

src/qweb.ts

@@ -1060,7 +1060,7 @@ const widgetDirective: Directive = {
     ctx.addIf(`w${widgetID} && w${widgetID}.__owl__.renderPromise`);
     ctx.addIf(`w${widgetID}.__owl__.isStarted`);
     ctx.addLine(
-      `def${defID} = w${widgetID}.updateProps(props${widgetID}, extra.forceUpdate);`
+      `def${defID} = w${widgetID}.updateProps(props${widgetID}, extra.forceUpdate, extra.patchQueue);`
     );
     ctx.addElse();
     ctx.addLine(`isNew${widgetID} = true`);
@@ -1076,7 +1076,7 @@ const widgetDirective: Directive = {
     ctx.addIf(`!def${defID}`);
     ctx.addIf(`w${widgetID}`);
     ctx.addLine(
-      `def${defID} = w${widgetID}.updateProps(props${widgetID}, extra.forceUpdate);`
+      `def${defID} = w${widgetID}.updateProps(props${widgetID}, extra.forceUpdate, extra.patchQueue);`
     );
     ctx.addElse();
     ctx.addLine(

tests/__snapshots__/component.test.ts.snap

@@ -30,7 +30,7 @@ exports[`composition sub widgets with some state rendered in a loop 1`] = `
         let isNew7 = !w7;
         if (w7 && w7.__owl__.renderPromise) {
             if (w7.__owl__.isStarted) {
-                def6 = w7.updateProps(props7, extra.forceUpdate);
+                def6 = w7.updateProps(props7, extra.forceUpdate, extra.patchQueue);
             } else {
                 isNew7 = true
                 if (props7 === w7.__owl__.renderProps) {
@@ -43,7 +43,7 @@ exports[`composition sub widgets with some state rendered in a loop 1`] = `
         }
         if (!def6) {
             if (w7) {
-                def6 = w7.updateProps(props7, extra.forceUpdate);
+                def6 = w7.updateProps(props7, extra.forceUpdate, extra.patchQueue);
             } else {
                 w7 = new context.widgets['ChildWidget'](owner, props7);
                 context.__owl__.cmap[key8] = w7.__owl__.id;
@@ -78,7 +78,7 @@ exports[`random stuff/miscellaneous snapshotting compiled code 1`] = `
     let isNew4 = !w4;
     if (w4 && w4.__owl__.renderPromise) {
         if (w4.__owl__.isStarted) {
-            def3 = w4.updateProps(props4, extra.forceUpdate);
+            def3 = w4.updateProps(props4, extra.forceUpdate, extra.patchQueue);
         } else {
             isNew4 = true
             if (props4 === w4.__owl__.renderProps) {
@@ -91,7 +91,7 @@ exports[`random stuff/miscellaneous snapshotting compiled code 1`] = `
     }
     if (!def3) {
         if (w4) {
-            def3 = w4.updateProps(props4, extra.forceUpdate);
+            def3 = w4.updateProps(props4, extra.forceUpdate, extra.patchQueue);
         } else {
             w4 = new context.widgets['child'](owner, props4);
             context.__owl__.cmap[key5] = w4.__owl__.id;
@@ -124,7 +124,7 @@ exports[`random stuff/miscellaneous t-props should not be undefined (snapshottin
     let isNew4 = !w4;
     if (w4 && w4.__owl__.renderPromise) {
         if (w4.__owl__.isStarted) {
-            def3 = w4.updateProps(props4, extra.forceUpdate);
+            def3 = w4.updateProps(props4, extra.forceUpdate, extra.patchQueue);
         } else {
             isNew4 = true
             if (props4 === w4.__owl__.renderProps) {
@@ -137,7 +137,7 @@ exports[`random stuff/miscellaneous t-props should not be undefined (snapshottin
     }
     if (!def3) {
         if (w4) {
-            def3 = w4.updateProps(props4, extra.forceUpdate);
+            def3 = w4.updateProps(props4, extra.forceUpdate, extra.patchQueue);
         } else {
             w4 = new context.widgets['child'](owner, props4);
             context.__owl__.cmap[4] = w4.__owl__.id;

tests/component.test.ts

@@ -264,6 +264,55 @@ describe("lifecycle hooks", () => {
     ]);
   });
 
+  test("willPatch, patched hook are called on subsubwidgets, in proper order", async () => {
+    const steps: any[] = [];
+
+    class ParentWidget extends Widget {
+      inlineTemplate = `<div>><t t-widget="child" t-props="{n:state.n}"/></div>`;
+      widgets = { child: ChildWidget };
+      state = { n: 1 };
+      willPatch() {
+        steps.push("parent:willPatch");
+      }
+      patched() {
+        steps.push("parent:patched");
+      }
+    }
+    class ChildWidget extends Widget {
+      inlineTemplate = `<div><t t-widget="childchild" t-props="{n:props.n}"/></div>`;
+      widgets = { childchild: ChildChildWidget };
+      willPatch() {
+        steps.push("child:willPatch");
+      }
+      patched() {
+        steps.push("child:patched");
+      }
+    }
+    class ChildChildWidget extends Widget {
+      inlineTemplate = `<div><t t-esc="props.n"/></div>`;
+      willPatch() {
+        steps.push("childchild:willPatch");
+      }
+      patched() {
+        steps.push("childchild:patched");
+      }
+    }
+    const widget = new ParentWidget(env);
+    await widget.mount(fixture);
+    expect(steps).toEqual([]);
+    widget.state.n = 2;
+    await nextTick();
+    widget.destroy();
+    expect(steps).toEqual([
+      "parent:willPatch",
+      "child:willPatch",
+      "childchild:willPatch",
+      "childchild:patched",
+      "child:patched",
+      "parent:patched"
+    ]);
+  });
+
   test("willStart, mounted on subwidget rendered after main is mounted in some other position", async () => {
     const steps: string[] = [];
 
@@ -450,6 +499,12 @@ describe("lifecycle hooks", () => {
 
   test("willUpdateProps hook is called", async () => {
     let def = makeDeferred();
+    class Parent extends Widget {
+      inlineTemplate =
+        '<span><t t-widget="Child" t-props="{n: state.n}"/></span>';
+      state = { n: 1 };
+      widgets = { Child: HookWidget };
+    }
     class HookWidget extends Widget {
       inlineTemplate = '<span><t t-esc="props.n"/></span>';
 
@@ -458,15 +513,15 @@ describe("lifecycle hooks", () => {
         return def;
       }
     }
-    const widget = new HookWidget(env, { n: 1 });
+    const widget = new Parent(env);
     await widget.mount(fixture);
-    expect(fixture.innerHTML).toBe("<span>1</span>");
+    expect(fixture.innerHTML).toBe("<span><span>1</span></span>");
-    widget.updateProps({ n: 2 });
+    widget.state.n = 2;
     await nextTick();
-    expect(fixture.innerHTML).toBe("<span>1</span>");
+    expect(fixture.innerHTML).toBe("<span><span>1</span></span>");
     def.resolve();
     await nextTick();
-    expect(fixture.innerHTML).toBe("<span>2</span>");
+    expect(fixture.innerHTML).toBe("<span><span>2</span></span>");
   });
 
   test("patched hook is called after updating State", async () => {
@@ -495,16 +550,23 @@ describe("lifecycle hooks", () => {
   test("patched hook is called after updateProps", async () => {
     let n = 0;
 
+    class Parent extends Widget {
+      inlineTemplate = '<div><t t-widget="Child" t-props="{a:state.a}"/></div>';
+      state = { a: 1 };
+      widgets = { Child: TestWidget };
+    }
+
     class TestWidget extends Widget {
       patched() {
         n++;
       }
     }
-    const widget = new TestWidget(env, { a: 1 });
+    const widget = new Parent(env);
     await widget.mount(fixture);
     expect(n).toBe(0);
 
-    await widget.updateProps({ a: 2 });
+    widget.state.a = 2;
+    await nextTick();
     expect(n).toBe(1);
   });
 
@@ -528,20 +590,27 @@ describe("lifecycle hooks", () => {
 
   test("shouldUpdate hook prevent rerendering", async () => {
     let shouldUpdate = false;
+    class Parent extends Widget {
+      inlineTemplate = `<div><t t-widget="Child" t-props="{val:state.val}"/></div>`;
+      state = { val: 42 };
+      widgets = { Child: TestWidget };
+    }
     class TestWidget extends Widget {
       inlineTemplate = `<div><t t-esc="props.val"/></div>`;
       shouldUpdate() {
         return shouldUpdate;
       }
     }
-    const widget = new TestWidget(env, { val: 42 });
+    const widget = new Parent(env);
     await widget.mount(fixture);
-    expect(fixture.innerHTML).toBe("<div>42</div>");
+    expect(fixture.innerHTML).toBe("<div><div>42</div></div>");
-    await widget.updateProps({ val: 123 });
+    widget.state.val = 123;
-    expect(fixture.innerHTML).toBe("<div>42</div>");
+    await nextTick();
+    expect(fixture.innerHTML).toBe("<div><div>42</div></div>");
     shouldUpdate = true;
-    await widget.updateProps({ val: 666 });
+    widget.state.val = 666;
-    expect(fixture.innerHTML).toBe("<div>666</div>");
+    await nextTick();
+    expect(fixture.innerHTML).toBe("<div><div>666</div></div>");
   });
 
   test("sub widget (inside sub node): hooks are correctly called", async () => {
@@ -613,9 +682,9 @@ describe("lifecycle hooks", () => {
 
     // Not sure about this order.  If you disagree, feel free to open an issue...
     expect(steps).toEqual([
+      "parent:willPatch",
       "child:willPatch",
       "child:patched",
-      "parent:willPatch",
       "parent:patched"
     ]);
   });
@@ -1416,8 +1485,8 @@ describe("async rendering", () => {
 
     class ChildA extends Widget {
       inlineTemplate = `<span>a<t t-esc="props.val"/></span>`;
-      updateProps(props): Promise<void> {
+      updateProps(props, forceUpdate, fiber): Promise<void> {
-        return defA.then(() => super.updateProps(props));
+        return defA.then(() => super.updateProps(props, forceUpdate, fiber));
       }
     }
     class ChildB extends Widget {

tests/qweb.test.ts

@@ -1097,12 +1097,17 @@ describe("t-key", () => {
 
 describe("debugging", () => {
   test("t-debug", () => {
+    const consoleLog = console.log;
+    console.log = jest.fn()
     qweb.addTemplate(
       "test",
       `<div t-debug="1"><t t-if="true"><span t-debug="1">hey</span></t></div>`
     );
     qweb.render('test');
     expect(qweb.templates.test.toString()).toMatchSnapshot();
+
+    expect(console.log).toHaveBeenCalledTimes(1);
+    console.log = consoleLog;
   });
 
   test("t-log", () => {