[FIX] component: reorganize fiber update algorithm

Closes #473
Closes #484
Closes #489
Closes #492
Closes #512

Co-authored-by: Aaron Bohy <aab@odoo.com>

src/component/component.ts

@@ -295,13 +295,12 @@ export class Component<T extends Env, Props extends {}> {
       return Promise.resolve();
     }
     if (!(target instanceof HTMLElement || target instanceof DocumentFragment)) {
-      let message = `Component '${
-        this.constructor.name
-      }' cannot be mounted: the target is not a valid DOM node.`;
+      let message = `Component '${this.constructor.name}' cannot be mounted: the target is not a valid DOM node.`;
       message += `\nMaybe the DOM is not ready yet? (in that case, you can use owl.utils.whenReady)`;
       throw new Error(message);
     }
     const fiber = new Fiber(null, this, false, target);
+    fiber.shouldPatch = false;
     if (!__owl__.vnode) {
       this.__prepareAndRender(fiber);
     } else {
@@ -451,13 +450,7 @@ export class Component<T extends Env, Props extends {}> {
 
   __callMounted() {
     const __owl__ = this.__owl__;
-    const children = __owl__.children;
-    for (let id in children) {
-      const comp = children[id];
-      if (!comp.__owl__.isMounted && this.el!.contains(comp.el)) {
-        comp.__callMounted();
-      }
-    }
+
     __owl__.isMounted = true;
     __owl__.currentFiber = null;
     this.mounted();
@@ -473,6 +466,10 @@ export class Component<T extends Env, Props extends {}> {
     }
     this.willUnmount();
     __owl__.isMounted = false;
+    if (this.__owl__.currentFiber) {
+      this.__owl__.currentFiber.isCompleted = true;
+      this.__owl__.currentFiber.root.counter = 0;
+    }
     const children = __owl__.children;
     for (let id in children) {
       const comp = children[id];
@@ -524,7 +521,7 @@ export class Component<T extends Env, Props extends {}> {
    * Main patching method. We call the virtual dom patch method here to convert
    * a virtual dom vnode into some actual dom.
    */
-  __patch(vnode) {
+  __patch(vnode: VNode) {
     const __owl__ = this.__owl__;
     const target = __owl__.vnode || document.createElement(vnode.sel!);
     __owl__.vnode = patch(target, vnode);
@@ -629,26 +626,6 @@ export class Component<T extends Env, Props extends {}> {
     }
   }
 
-  /**
-   * Only called by qweb t-component directive
-   */
-  __mount(fiber: Fiber, elm: HTMLElement): VNode {
-    if (fiber !== this.__owl__.currentFiber) {
-      fiber = this.__owl__.currentFiber!; // TODO: check if we can remove fiber arg
-    }
-    const vnode = fiber.vnode!;
-    const __owl__ = this.__owl__;
-    if (__owl__.classObj) {
-      (<any>vnode).data.class = Object.assign((<any>vnode).data.class || {}, __owl__.classObj);
-    }
-    __owl__.vnode = patch(elm, vnode);
-    __owl__.currentFiber = null;
-    if (__owl__.parent!.__owl__.isMounted && !__owl__.isMounted) {
-      this.__callMounted();
-    }
-    return __owl__.vnode;
-  }
-
   /**
    * Only called by qweb t-component directive (when t-keepalive is set)
    */

src/component/directive.ts

@@ -239,10 +239,6 @@ QWeb.addDirective({
       ctx.addLine(`const ${refKey} = ${ctx.interpolate(ref)};`);
       refExpr = `context.__owl__.refs[${refKey}] = w${componentID};`;
     }
-    let transitionsInsertCode = "";
-    if (transition) {
-      transitionsInsertCode = `utils.transitionInsert(vn, '${transition}');`;
-    }
     let finalizeComponentCode = `w${componentID}.destroy();`;
     if (ref) {
       finalizeComponentCode += `delete context.__owl__.refs[${refKey}];`;
@@ -251,6 +247,7 @@ QWeb.addDirective({
       finalizeComponentCode = `let finalize = () => {
           ${finalizeComponentCode}
         };
+        delete w${componentID}.__owl__.transitionInserted;
         utils.transitionRemove(vn, '${transition}', finalize);`;
     }
 
@@ -409,6 +406,12 @@ QWeb.addDirective({
       `if (!W${componentID}) {throw new Error('Cannot find the definition of component "' + componentKey${componentID} + '"')}`
     );
     ctx.addLine(`w${componentID} = new W${componentID}(parent, props${componentID});`);
+    if (transition) {
+      ctx.addLine(`const __patch${componentID} = w${componentID}.__patch;`);
+      ctx.addLine(
+        `w${componentID}.__patch = fiber => {__patch${componentID}.call(w${componentID}, fiber); if(!w${componentID}.__owl__.transitionInserted){w${componentID}.__owl__.transitionInserted = true;utils.transitionInsert(w${componentID}.__owl__.vnode, '${transition}');}};`
+      );
+    }
     ctx.addLine(`parent.__owl__.cmap[${templateKey}] = w${componentID}.__owl__.id;`);
 
     if (hasSlots) {
@@ -438,8 +441,9 @@ QWeb.addDirective({
 
     ctx.addLine(`let def${defID} = w${componentID}.__prepare(extra.fiber, ${scopeVars});`);
     // hack: specify empty remove hook to prevent the node from being removed from the DOM
+    const insertHook = refExpr ? `insert(vn) {${refExpr}},` : "";
     ctx.addLine(
-      `let pvnode = h('dummy', {key: ${templateKey}, hook: {insert(vn) { let nvn=w${componentID}.__mount(fiber, pvnode.elm);pvnode.elm=nvn.elm;${refExpr}${transitionsInsertCode}},remove() {},destroy(vn) {${finalizeComponentCode}}}});`
+      `let pvnode = h('dummy', {key: ${templateKey}, hook: {${insertHook}remove() {},destroy(vn) {${finalizeComponentCode}}}});`
     );
     ctx.addLine(`const fiber = w${componentID}.__owl__.currentFiber;`);
     ctx.addLine(

src/component/fiber.ts

@@ -120,6 +120,7 @@ export class Fiber {
    */
   _remapFiber(oldFiber: Fiber) {
     oldFiber.cancel();
+    this.shouldPatch = oldFiber.shouldPatch;
     if (oldFiber === oldFiber.root) {
       oldFiber.counter++;
     }
@@ -178,54 +179,62 @@ export class Fiber {
    * are ready, and the scheduler decides to process it.
    */
   complete() {
-    const component = this.component;
-    if (this.target) {
-      component.__patch(this.vnode);
-      this.target.appendChild(component.el!);
-      if (document.body.contains(this.target)) {
-        component.__callMounted();
-      }
-    } else if (component.__owl__.isMounted && this === this.root) {
-      this.patchComponents();
-    }
+    let component = this.component;
     this.isCompleted = true;
-  }
+    if (!this.target && !component.__owl__.isMounted) {
+      return;
+    }
 
-  /**
-   * Compute and apply the patch queue of the fiber.
-   *   1) Call 'willPatch' on the component of each patch
-   *   2) Call '__patch' on the component of each patch
-   *   3) Call 'patched' on the component of each patch, in reverse order
-   */
-  patchComponents() {
+    // build patchQueue
     const patchQueue: Fiber[] = [];
     const doWork: (Fiber) => Fiber | null = function(f) {
-      if (f.shouldPatch) {
-        patchQueue.push(f);
-        return f.child;
-      }
+      patchQueue.push(f);
+      return f.child;
     };
     this._walk(doWork);
-    let component: Component<any, any> = this.component;
     const patchLen = patchQueue.length;
-    for (let i = 0; i < patchLen; i++) {
-      component = patchQueue[i].component;
-      if (component.__owl__.willPatchCB) {
-        component.__owl__.willPatchCB();
-      }
-      component.willPatch();
-    }
+
+    // call willPatch hook on each fiber of patchQueue
     for (let i = 0; i < patchLen; i++) {
+      const fiber = patchQueue[i];
+      if (fiber.shouldPatch) {
+        component = fiber.component;
+        if (component.__owl__.willPatchCB) {
+          component.__owl__.willPatchCB();
+        }
+        component.willPatch();
+      }
+    }
+
+    // call __patch on each fiber of (reversed) patchQueue
+    for (let i = patchLen - 1; i >= 0; i--) {
       const fiber = patchQueue[i];
       component = fiber.component;
-      component.__patch(fiber.vnode);
+      component.__patch(fiber.vnode!);
+      if (!fiber.shouldPatch && (!fiber.target || i !== 0)) {
+        component.__owl__.pvnode!.elm = component.__owl__.vnode!.elm;
+      }
       component.__owl__.currentFiber = null;
     }
+
+    // insert into the DOM (mount case)
+    let inDOM = false;
+    if (this.target) {
+      this.target.appendChild(this.component.el!);
+      inDOM = document.body.contains(this.target);
+    }
+
+    // call patched/mounted hook on each fiber of (reversed) patchQueue
     for (let i = patchLen - 1; i >= 0; i--) {
-      component = patchQueue[i].component;
-      component.patched();
-      if (component.__owl__.patchedCB) {
-        component.__owl__.patchedCB();
+      const fiber = patchQueue[i];
+      component = fiber.component;
+      if (fiber.shouldPatch && !this.target) {
+        component.patched();
+        if (component.__owl__.patchedCB) {
+          component.__owl__.patchedCB();
+        }
+      } else if (this.target ? inDOM : true) {
+        component.__callMounted();
       }
     }
   }

src/component/scheduler.ts

@@ -26,6 +26,9 @@ export class Scheduler {
   }
 
   addFiber(fiber): Promise<void> {
+    // if the fiber was remapped into a larger rendering fiber, it may not be a
+    // root fiber.  But we only want to register root fibers
+    fiber = fiber.root;
     return new Promise((resolve, reject) => {
       if (fiber.error) {
         return reject(fiber.error);