[REF] component: large cleanup of concurrency branch

We remove here old comments, add some tests and documentation, and in
general, make sure the state of the code is in a good shape

part of #330

src/component/component.ts

@@ -1,10 +1,10 @@
 import { Observer } from "../core/observer";
 import { CompiledTemplate, QWeb } from "../qweb/index";
 import { h, patch, VNode } from "../vdom/index";
-import { Fiber } from "./fiber";
-import { scheduler } from "./scheduler";
 import "./directive";
+import { Fiber } from "./fiber";
 import "./props_validation";
+import { Scheduler } from "./scheduler";
 
 /**
  * Owl Component System
@@ -20,6 +20,8 @@ import "./props_validation";
 //------------------------------------------------------------------------------
 // Types/helpers
 //------------------------------------------------------------------------------
+const raf = window.requestAnimationFrame.bind(window);
+export const scheduler = new Scheduler(raf);
 
 /**
  * An Env (environment) is an object that will be (mostly) shared between all
@@ -62,7 +64,7 @@ interface Internal<T extends Env, Props> {
 
   boundHandlers: { [key: number]: any };
   observer: Observer | null;
-  render: CompiledTemplate;
+  renderFn: CompiledTemplate;
   mountedCB: Function | null;
   willUnmountCB: Function | null;
   willPatchCB: Function | null;
@@ -179,7 +181,7 @@ export class Component<T extends Env, Props extends {}> {
       willStartCB: null,
       willUpdatePropsCB: null,
       observer: null,
-      render: qweb.render.bind(qweb, this.__getTemplate(qweb)),
+      renderFn: qweb.render.bind(qweb, this.__getTemplate(qweb)),
       classObj: null,
       refs: null
     };
@@ -280,41 +282,31 @@ export class Component<T extends Env, Props extends {}> {
     if (__owl__.isMounted) {
       return Promise.resolve();
     }
-    const fiber = new Fiber(null, this, this.props, undefined, undefined, false);
+    if (__owl__.vnode && !renderBeforeRemount) {
-    if (!__owl__.vnode) {
-      this.__prepareAndRender(fiber);
-      return new Promise(resolve => {
-        scheduler.addFiber(fiber, () => {
-          if (!__owl__.isDestroyed) {
-            this.__patch(fiber.vnode);
-            target.appendChild(this.el!);
-            if (document.body.contains(target)) {
-              this.__callMounted();
-            }
-          }
-          resolve();
-        });
-      });
-    } else if (renderBeforeRemount) {
-      this.__render(fiber);
-      return new Promise(resolve => {
-        scheduler.addFiber(fiber, () => {
-          if (!__owl__.isDestroyed) {
-            this.__patch(fiber.vnode);
-            target.appendChild(this.el!);
-            if (document.body.contains(target)) {
-              this.__callMounted();
-            }
-          }
-          resolve();
-        });
-      });
-    } else {
       target.appendChild(this.el!);
       if (document.body.contains(target)) {
         this.__callMounted();
       }
+      return;
     }
+    const fiber = new Fiber(null, this, this.props, undefined, undefined, false);
+    if (!__owl__.vnode) {
+      this.__prepareAndRender(fiber);
+    } else {
+      this.__render(fiber);
+    }
+    return new Promise(resolve => {
+      scheduler.addFiber(fiber, () => {
+        if (!__owl__.isDestroyed) {
+          this.__patch(fiber.vnode);
+          target.appendChild(this.el!);
+          if (document.body.contains(target)) {
+            this.__callMounted();
+          }
+        }
+        resolve();
+      });
+    });
   }
 
   /**
@@ -350,7 +342,7 @@ export class Component<T extends Env, Props extends {}> {
     return new Promise(resolve => {
       scheduler.addFiber(fiber.root, () => {
         if (__owl__.isMounted && fiber === fiber.root) {
-          fiber.__applyPatchQueue();
+          fiber.patchComponents();
         }
         resolve();
       });
@@ -588,7 +580,7 @@ export class Component<T extends Env, Props extends {}> {
     }
     let vnode;
     try {
-      vnode = __owl__.render!(this, {
+      vnode = __owl__.renderFn!(this, {
         handlers: __owl__.boundHandlers,
         fiber: fiber
       });
@@ -674,7 +666,7 @@ Fiber.prototype.handleError = function(error) {
  * If there are no such component, we destroy everything. This is better than
  * being in a corrupted state.
  */
-export function errorHandler(error: Error, fiber: Fiber) {
+function errorHandler(error: Error, fiber: Fiber) {
   let canCatch = false;
   let component = fiber.component;
   let qweb = component.env.qweb;

src/component/directive.ts

@@ -240,7 +240,6 @@ QWeb.addDirective({
       // want to evaluate it only once)
       ctx.addLine(`let key${keyID} = 'key' + ${key};`);
     }
-    ctx.addLine(`let def${defID};`);
 
     let locationExpr = `\`__${ctx.generateID()}__`;
     for (let i = 0; i < ctx.loopNumber - 1; i++) {
@@ -376,7 +375,69 @@ QWeb.addDirective({
     ctx.addLine(`w${componentID} = false;`);
     ctx.closeIf();
 
-    ctx.addIf(`!w${componentID}`);
+    let registerCode = "";
+    if (shouldProxy) {
+      registerCode = `utils.defineProxy(vn${ctx.rootNode}, pvnode);`;
+    }
+
+    // SLOTS
+    const varDefs: string[] = [];
+    const hasSlots = node.childNodes.length;
+    if (hasSlots) {
+      ctx.rootContext.shouldTrackScope = true;
+      for (let v of Object.values(ctx.variables)) {
+        if (v["id"]) {
+          varDefs.push(v["id"]);
+        }
+      }
+    }
+
+    let scopeVars;
+    if (hasSlots) {
+      let scope = ctx.scopeVars.length ? `Object.assign({}, scope)` : `{}`;
+      let vars = varDefs.length ? `{${varDefs.join(",")}}` : "undefined";
+      scopeVars = `${scope}, ${vars}`;
+    } else {
+      scopeVars = "undefined, undefined";
+    }
+
+    ctx.addIf(`w${componentID}`);
+
+    // need to update component
+    let patchQueueCode = keepAlive ? `fiber${componentID}` : "extra.fiber";
+    if (keepAlive) {
+      // if we have t-keepalive="1", the component could be unmounted, but then
+      // we __updateProps is called.  This is ok, but we do not want to call
+      // the willPatch/patched hooks of the component in this case, so we
+      // disable the patch queue
+      patchQueueCode = `w${componentID}.__owl__.isMounted ? extra.fiber : fiber${componentID}`;
+    }
+    if (QWeb.dev) {
+      ctx.addLine(`utils.validateProps(w${componentID}.constructor, props${componentID})`);
+    }
+    let styleCode = "";
+    if (tattStyle) {
+      styleCode = `.then(()=>{if (w${componentID}.__owl__.isDestroyed) {return};w${componentID}.el.style=${tattStyle};});`;
+    }
+    ctx.addLine(
+      `w${componentID}.__updateProps(props${componentID}, ${patchQueueCode}${scopeVars &&
+        ", " + scopeVars}, sibling)${styleCode};`
+    );
+    ctx.addLine(`let pvnode = w${componentID}.__owl__.pvnode;`);
+    let keepAliveCode = "";
+    if (keepAlive) {
+      keepAliveCode = `pvnode.data.hook.insert = vn => {vn.elm.parentNode.replaceChild(w${componentID}.el,vn.elm);vn.elm=w${componentID}.el;w${componentID}.__remount();};`;
+      ctx.addLine(keepAliveCode);
+    }
+    if (registerCode) {
+      ctx.addLine(registerCode);
+    }
+    if (ctx.parentNode) {
+      ctx.addLine(`c${ctx.parentNode}.push(pvnode);`);
+    }
+
+    ctx.addElse();
+
     // new component
     let dynamicFallback = "";
     if (!value.match(INTERP_REGEXP)) {
@@ -395,17 +456,7 @@ QWeb.addDirective({
     ctx.addLine(`w${componentID} = new W${componentID}(parent, props${componentID});`);
     ctx.addLine(`parent.__owl__.cmap[${templateId}] = w${componentID}.__owl__.id;`);
 
-    // SLOTS
-    const varDefs: string[] = [];
-    const hasSlots = node.childNodes.length;
     if (hasSlots) {
-      ctx.rootContext.shouldTrackScope = true;
-      for (let v of Object.values(ctx.variables)) {
-        if (v["id"]) {
-          varDefs.push(v["id"]);
-        }
-      }
-
       const clone = <Element>node.cloneNode(true);
       const slotNodes = clone.querySelectorAll("[t-set]");
       const slotId = QWeb.nextSlotId++;
@@ -430,20 +481,8 @@ QWeb.addDirective({
       }
     }
 
-    let scopeVars;
+    ctx.addLine(`let def${defID} = w${componentID}.__prepare(extra.fiber, ${scopeVars}, sibling);`);
-    if (hasSlots) {
-      let scope = ctx.scopeVars.length ? `Object.assign({}, scope)` : `{}`;
-      let vars = varDefs.length ? `{${varDefs.join(",")}}` : "undefined";
-      scopeVars = `${scope}, ${vars}`;
-    } else {
-      scopeVars = "undefined, undefined";
-    }
-    ctx.addLine(`def${defID} = w${componentID}.__prepare(extra.fiber, ${scopeVars}, sibling);`);
     // hack: specify empty remove hook to prevent the node from being removed from the DOM
-    let registerCode = "";
-    if (shouldProxy) {
-      registerCode = `utils.defineProxy(vn${ctx.rootNode}, pvnode);`;
-    }
     ctx.addLine(
       `let pvnode = h('dummy', {key: ${templateId}, hook: {insert(vn) { let nvn=w${componentID}.__mount(fiber, pvnode.elm);pvnode.elm=nvn.elm;${refExpr}${transitionsInsertCode}},remove() {},destroy(vn) {${finalizeComponentCode}}}});`
     );
@@ -459,47 +498,13 @@ QWeb.addDirective({
     }
     ctx.addLine(`w${componentID}.__owl__.pvnode = pvnode;`);
 
-    ctx.addElse();
-    // need to update component
-    let patchQueueCode = keepAlive ? `fiber${componentID}` : "extra.fiber";
-    if (keepAlive) {
-      // if we have t-keepalive="1", the component could be unmounted, but then
-      // we __updateProps is called.  This is ok, but we do not want to call
-      // the willPatch/patched hooks of the component in this case, so we
-      // disable the patch queue
-      patchQueueCode = `w${componentID}.__owl__.isMounted ? extra.fiber : fiber${componentID}`;
-    }
-    if (QWeb.dev) {
-      ctx.addLine(`utils.validateProps(w${componentID}.constructor, props${componentID})`);
-    }
-    ctx.addLine(
-      `def${defID} = def${defID} || w${componentID}.__updateProps(props${componentID}, ${patchQueueCode}${scopeVars &&
-        ", " + scopeVars}, sibling);`
-    );
-    ctx.addLine(`let pvnode = w${componentID}.__owl__.pvnode;`);
-    let keepAliveCode = "";
-    if (keepAlive) {
-      keepAliveCode = `pvnode.data.hook.insert = vn => {vn.elm.parentNode.replaceChild(w${componentID}.el,vn.elm);vn.elm=w${componentID}.el;w${componentID}.__remount();};`;
-      ctx.addLine(keepAliveCode);
-    }
-    if (registerCode) {
-      ctx.addLine(registerCode);
-    }
-    if (ctx.parentNode) {
-      ctx.addLine(`c${ctx.parentNode}.push(pvnode);`);
-    }
-    if (tattStyle) {
-      ctx.addLine(
-        `def${defID} = def${defID}.then(()=>{if (w${componentID}.__owl__.isDestroyed) {return};w${componentID}.el.style=${tattStyle};});`
-      );
-    }
     ctx.closeIf();
 
     if (classObj) {
       ctx.addLine(`w${componentID}.__owl__.classObj=${classObj};`);
     }
 
-    ctx.addLine(`sibling = w${componentID}.__owl__.currentFiber;`);
+    ctx.addLine(`sibling = w${componentID}.__owl__.currentFiber || sibling;`);
 
     return true;
   }

src/component/fiber.ts

@@ -1,17 +1,46 @@
 import { VNode } from "../vdom/index";
 import { Component } from "./component";
+
 /**
+ * Owl Fiber Class
+ *
  * Fibers are small abstractions designed to contain all the internal state
- * associated to a "rendering work unit", relative to a specific component.
+ * associated with a "rendering work unit", relative to a specific component.
  *
  * A rendering will cause the creation of a fiber for each impacted components.
+ *
+ * Fibers capture all that necessary information, which is critical to owl
+ * asynchronous rendering pipeline. Fibers can be cancelled, can be in different
+ * states and in general determine the state of the rendering.
  */
+
 export class Fiber {
+  // The force attribute determines if a rendering should bypass the `shouldUpdate`
+  // method potentially implemented by a component. It is usually set to false.
   force: boolean;
+
+  // isCancelled means that the rendering corresponding to this fiber and its
+  // children is cancelled. No extra work should be done.
   isCancelled: boolean = false;
+
+  // the fibers corresponding to component updates (updateProps) need to call
+  // the willPatch and patched hooks from the corresponding component. However,
+  // fibers corresponding to a new component do not need to do that. So, the
+  // shouldPatch hook is the boolean that we check whenever we need to apply
+  // a patch.
   shouldPatch: boolean = true;
+
+  // isRendered is the last state of a fiber. If true, this means that it has
+  // been rendered and is inert (so, it should not be taken into account when
+  // counting the number of active fibers).
   isRendered: boolean = false;
 
+  // the counter number is a critical information. It is only necessary for a
+  // root fiber.  For that fiber, this number counts the number of active sub
+  // fibers.  When that number reaches 0, the fiber can be applied by the
+  // scheduler.
+  counter: number = 0;
+
   scope: any;
   vars: any;
   props: any;
@@ -24,8 +53,6 @@ export class Fiber {
   sibling: Fiber | null = null;
   parent: Fiber | null = null;
 
-  counter: number = 0;
-
   constructor(parent: Fiber | null, component: Component<any, any>, props, scope, vars, force) {
     this.force = force;
     this.scope = scope;
@@ -38,7 +65,7 @@ export class Fiber {
 
     let oldFiber = component.__owl__.currentFiber;
     if (oldFiber && !oldFiber.isCancelled) {
-      this.__remapFiber(oldFiber);
+      this._remapFiber(oldFiber);
     }
 
     this.root.counter++;
@@ -46,7 +73,13 @@ export class Fiber {
     component.__owl__.currentFiber = this;
   }
 
-  __remapFiber(oldFiber: Fiber) {
+  /**
+   * In some cases, a rendering initiated at some component can detect that it
+   * should be part of a larger rendering initiated somewhere up the component
+   * tree.  In that case, it needs to cancel the previous rendering and
+   * remap itself as a part of the current parent rendering.
+   */
+  _remapFiber(oldFiber: Fiber) {
     oldFiber.cancel();
     if (oldFiber === oldFiber.root) {
       oldFiber.root.counter++;
@@ -75,7 +108,7 @@ export class Fiber {
    * This function has been taken from
    * https://medium.com/react-in-depth/the-how-and-why-on-reacts-usage-of-linked-list-in-fiber-67f1014d0eb7
    */
-  __walk(doWork: (f: Fiber) => Fiber | null) {
+  _walk(doWork: (f: Fiber) => Fiber | null) {
     let root = this;
     let current: Fiber = this;
     while (true) {
@@ -103,7 +136,7 @@ export class Fiber {
    *   2) Call '__patch' on the component of each patch
    *   3) Call 'patched' on the component of each patch, in reverse order
    */
-  __applyPatchQueue() {
+  patchComponents() {
     const patchQueue: Fiber[] = [];
     const doWork: (Fiber) => Fiber | null = function(f) {
       if (f.shouldPatch) {
@@ -111,7 +144,7 @@ export class Fiber {
       }
       return f.child;
     };
-    this.__walk(doWork);
+    this._walk(doWork);
     let component: Component<any, any> = this.component;
     this.shouldPatch = false;
     const patchLen = patchQueue.length;
@@ -149,8 +182,11 @@ export class Fiber {
     this.shouldPatch = true;
   }
 
+  /**
+   * Cancel a fiber and all its children.
+   */
   cancel() {
-    this.__walk(f => {
+    this._walk(f => {
       if (!f.isRendered) {
         f.root.counter--;
       }

src/component/scheduler.ts

@@ -1,14 +1,29 @@
 import { Fiber } from "./fiber";
 
-// scheduler
+/**
+ * Owl Scheduler Class
+ *
+ * The scheduler is the part of Owl that will effectively apply a rendering
+ * whenever a fiber is ready.
+ *
+ * Briefly, it can be used to register root fibers.  Whenever there is an
+ * active root fiber, it will poll continuously each animation frame (so, about
+ * once every 16ms) and whenever a root fiber is ready, it will apply it.
+ */
+
 interface Task {
   fiber: Fiber;
   callback: () => void;
 }
 
-export const scheduler = {
+export class Scheduler {
-  tasks: [] as Task[],
+  tasks: Task[] = [];
-  isRunning: false,
+  isRunning: boolean = false;
+  requestAnimationFrame: typeof window.requestAnimationFrame;
+
+  constructor(requestAnimationFrame) {
+    this.requestAnimationFrame = requestAnimationFrame;
+  }
 
   addFiber(fiber, callback) {
     this.tasks.push({ fiber, callback });
@@ -16,7 +31,12 @@ export const scheduler = {
       return;
     }
     this.scheduleTasks();
-  },
+  }
+
+  /**
+   * Process all current tasks. This only applies to the fibers that are ready.
+   * Other tasks are left unchanged.
+   */
   flush() {
     let tasks = this.tasks;
     this.tasks = [];
@@ -31,20 +51,17 @@ export const scheduler = {
       return true;
     });
     this.tasks = tasks.concat(this.tasks);
-  },
+  }
-  processTasks() {
-    this.flush();
-    if (this.tasks.length > 0) {
-      this.scheduleTasks();
-    } else {
-      this.isRunning = false;
-    }
-  },
 
   scheduleTasks() {
     this.isRunning = true;
-    this.requestAnimationFrame(() => this.processTasks());
+    this.requestAnimationFrame(() => {
-  },
+      this.flush();
-
+      if (this.tasks.length > 0) {
-  requestAnimationFrame: requestAnimationFrame.bind(window)
+        this.scheduleTasks();
-};
+      } else {
+        this.isRunning = false;
+      }
+    });
+  }
+}

src/context.ts

@@ -1,5 +1,4 @@
-import { Component } from "./component/component";
+import { Component, scheduler } from "./component/component";
-import { scheduler } from "./component/scheduler";
 import { EventBus } from "./core/event_bus";
 import { Observer } from "./core/observer";
 import { onWillUnmount } from "./hooks";
@@ -70,7 +69,7 @@ export function useContext(ctx: Context): any {
   return useContextWithCB(ctx, component, component.render.bind(component));
 }
 
-export function useContextWithCB(ctx: Context, component, method): any {
+export function useContextWithCB(ctx: Context, component: Component<any, any>, method): any {
   const __owl__ = component.__owl__;
   const id = __owl__.id;
   const mapping = ctx.mapping;
@@ -78,8 +77,8 @@ export function useContextWithCB(ctx: Context, component, method): any {
     return ctx.state;
   }
   mapping[id] = 0;
-  const renderFn = __owl__.render;
+  const renderFn = __owl__.renderFn;
-  __owl__.render = function(comp, params) {
+  __owl__.renderFn = function(comp, params) {
     mapping[id] = ctx.id;
     return renderFn(comp, params);
   };

src/qweb/base_directives.ts

@@ -133,7 +133,7 @@ QWeb.addDirective({
   priority: 20,
   atNodeEncounter({ node, ctx }): boolean {
     let cond = ctx.getValue(node.getAttribute("t-if")!);
-    ctx.addIf(typeof cond === 'string' ? ctx.formatExpression(cond) : cond.id);
+    ctx.addIf(typeof cond === "string" ? ctx.formatExpression(cond) : cond.id);
     return false;
   },
   finalize({ ctx }) {
@@ -146,7 +146,7 @@ QWeb.addDirective({
   priority: 30,
   atNodeEncounter({ node, ctx }): boolean {
     let cond = ctx.getValue(node.getAttribute("t-elif")!);
-    ctx.addLine(`else if (${typeof cond === 'string' ? ctx.formatExpression(cond) : cond.id}) {`);
+    ctx.addLine(`else if (${typeof cond === "string" ? ctx.formatExpression(cond) : cond.id}) {`);
     ctx.indent();
     return false;
   },

src/store.ts

@@ -104,11 +104,11 @@ export function useStore(selector, options: SelectorOptions = {}): any {
       return true;
     }
     return false;
-  })
+  });
 
   useContextWithCB(store, component, function(): Promise<void> | void {
     let shouldRender = false;
-    updateFunctions.forEach(function (updateFn) {
+    updateFunctions.forEach(function(updateFn) {
       shouldRender = updateFn() || shouldRender;
     });
     if (shouldRender) {

tests/__snapshots__/animations.test.ts.snap

@@ -12,7 +12,6 @@ exports[`animations t-transition combined with component 1`] = `
     let c1 = [], p1 = {key:1};
     var vn1 = h('div', p1, c1);
     //COMPONENT
-    let def2;
     let templateId3 = \`__4__\`;
     let w3 = templateId3 in parent.__owl__.cmap ? parent.__owl__.children[parent.__owl__.cmap[templateId3]] : false;
     let props3 = {};
@@ -20,13 +19,17 @@ exports[`animations t-transition combined with component 1`] = `
         w3.destroy();
         w3 = false;
     }
-    if (!w3) {
+    if (w3) {
+        w3.__updateProps(props3, extra.fiber, undefined, undefined, sibling);
+        let pvnode = w3.__owl__.pvnode;
+        c1.push(pvnode);
+    } else {
         let componentKey3 = \`Child\`;
         let W3 = context.constructor.components[componentKey3] || QWeb.components[componentKey3]|| context['Child'];
         if (!W3) {throw new Error('Cannot find the definition of component \\"' + componentKey3 + '\\"')}
         w3 = new W3(parent, props3);
         parent.__owl__.cmap[templateId3] = w3.__owl__.id;
-        def2 = w3.__prepare(extra.fiber, undefined, undefined, sibling);
+        let def2 = w3.__prepare(extra.fiber, undefined, undefined, sibling);
         let pvnode = h('dummy', {key: templateId3, hook: {insert(vn) { let nvn=w3.__mount(fiber, pvnode.elm);pvnode.elm=nvn.elm;utils.transitionInsert(vn, 'chimay');},remove() {},destroy(vn) {let finalize = () => {
           w3.destroy();
         };
@@ -35,12 +38,8 @@ exports[`animations t-transition combined with component 1`] = `
         def2.then(function () {if (w3.__owl__.isDestroyed) {return;} const vnode = fiber.vnode; pvnode.sel = vnode.sel; });
         c1.push(pvnode);
         w3.__owl__.pvnode = pvnode;
-    } else {
-        def2 = def2 || w3.__updateProps(props3, extra.fiber, undefined, undefined, sibling);
-        let pvnode = w3.__owl__.pvnode;
-        c1.push(pvnode);
     }
-    sibling = w3.__owl__.currentFiber;
+    sibling = w3.__owl__.currentFiber || sibling;
     return vn1;
 }"
 `;
@@ -58,7 +57,6 @@ exports[`animations t-transition combined with t-component and t-if 1`] = `
     var vn1 = h('div', p1, c1);
     if (context['state'].display) {
         //COMPONENT
-        let def2;
         let templateId3 = \`__4__\`;
         let w3 = templateId3 in parent.__owl__.cmap ? parent.__owl__.children[parent.__owl__.cmap[templateId3]] : false;
         let props3 = {};
@@ -66,13 +64,17 @@ exports[`animations t-transition combined with t-component and t-if 1`] = `
             w3.destroy();
             w3 = false;
         }
-        if (!w3) {
+        if (w3) {
+            w3.__updateProps(props3, extra.fiber, undefined, undefined, sibling);
+            let pvnode = w3.__owl__.pvnode;
+            c1.push(pvnode);
+        } else {
             let componentKey3 = \`Child\`;
             let W3 = context.constructor.components[componentKey3] || QWeb.components[componentKey3]|| context['Child'];
             if (!W3) {throw new Error('Cannot find the definition of component \\"' + componentKey3 + '\\"')}
             w3 = new W3(parent, props3);
             parent.__owl__.cmap[templateId3] = w3.__owl__.id;
-            def2 = w3.__prepare(extra.fiber, undefined, undefined, sibling);
+            let def2 = w3.__prepare(extra.fiber, undefined, undefined, sibling);
             let pvnode = h('dummy', {key: templateId3, hook: {insert(vn) { let nvn=w3.__mount(fiber, pvnode.elm);pvnode.elm=nvn.elm;utils.transitionInsert(vn, 'chimay');},remove() {},destroy(vn) {let finalize = () => {
           w3.destroy();
         };
@@ -81,12 +83,8 @@ exports[`animations t-transition combined with t-component and t-if 1`] = `
             def2.then(function () {if (w3.__owl__.isDestroyed) {return;} const vnode = fiber.vnode; pvnode.sel = vnode.sel; });
             c1.push(pvnode);
             w3.__owl__.pvnode = pvnode;
-        } else {
-            def2 = def2 || w3.__updateProps(props3, extra.fiber, undefined, undefined, sibling);
-            let pvnode = w3.__owl__.pvnode;
-            c1.push(pvnode);
         }
-        sibling = w3.__owl__.currentFiber;
+        sibling = w3.__owl__.currentFiber || sibling;
     }
     return vn1;
 }"

tests/component/__snapshots__/props_validation.test.ts.snap

@@ -12,7 +12,6 @@ exports[`props validation props are validated in dev mode (code snapshot) 1`] =
     let c1 = [], p1 = {key:1};
     var vn1 = h('div', p1, c1);
     //COMPONENT
-    let def2;
     let templateId3 = \`__4__\`;
     let w3 = templateId3 in parent.__owl__.cmap ? parent.__owl__.children[parent.__owl__.cmap[templateId3]] : false;
     let props3 = {message:1};
@@ -20,25 +19,25 @@ exports[`props validation props are validated in dev mode (code snapshot) 1`] =
         w3.destroy();
         w3 = false;
     }
-    if (!w3) {
+    if (w3) {
+        utils.validateProps(w3.constructor, props3)
+        w3.__updateProps(props3, extra.fiber, undefined, undefined, sibling);
+        let pvnode = w3.__owl__.pvnode;
+        c1.push(pvnode);
+    } else {
         let componentKey3 = \`Child\`;
         let W3 = context.constructor.components[componentKey3] || QWeb.components[componentKey3]|| context['Child'];
         if (!W3) {throw new Error('Cannot find the definition of component \\"' + componentKey3 + '\\"')}
         w3 = new W3(parent, props3);
         parent.__owl__.cmap[templateId3] = w3.__owl__.id;
-        def2 = w3.__prepare(extra.fiber, undefined, undefined, sibling);
+        let def2 = w3.__prepare(extra.fiber, undefined, undefined, sibling);
         let pvnode = h('dummy', {key: templateId3, hook: {insert(vn) { let nvn=w3.__mount(fiber, pvnode.elm);pvnode.elm=nvn.elm;},remove() {},destroy(vn) {w3.destroy();}}});
         const fiber = w3.__owl__.currentFiber;
         def2.then(function () {if (w3.__owl__.isDestroyed) {return;} const vnode = fiber.vnode; pvnode.sel = vnode.sel; });
         c1.push(pvnode);
         w3.__owl__.pvnode = pvnode;
-    } else {
-        utils.validateProps(w3.constructor, props3)
-        def2 = def2 || w3.__updateProps(props3, extra.fiber, undefined, undefined, sibling);
-        let pvnode = w3.__owl__.pvnode;
-        c1.push(pvnode);
     }
-    sibling = w3.__owl__.currentFiber;
+    sibling = w3.__owl__.currentFiber || sibling;
     return vn1;
 }"
 `;

tests/component/component.test.ts

@@ -193,6 +193,23 @@ describe("basic widget properties", () => {
     expect(steps).toEqual(["__render", "mounted"]);
   });
 
+  test("render method wait until rendering is done", async () => {
+    class TestW extends Component<any, any> {
+      static template = xml`<div><t t-esc="state.drinks"/></div>`;
+      state = { drinks: 1 };
+    }
+    const widget = new TestW(env);
+    await widget.mount(fixture);
+    expect(fixture.innerHTML).toBe("<div>1</div>");
+
+    widget.state.drinks = 2;
+
+    const renderPromise = widget.render();
+    expect(fixture.innerHTML).toBe("<div>1</div>");
+    await renderPromise;
+    expect(fixture.innerHTML).toBe("<div>2</div>");
+  });
+
   test("keeps a reference to env", async () => {
     const widget = new Widget(env);
     expect(widget.env).toBe(env);
@@ -1553,6 +1570,44 @@ describe("composition", () => {
     expect(fixture.innerHTML).toBe("<div><div>world</div></div>");
     expect(env.qweb.templates.ParentWidget.fn.toString()).toMatchSnapshot();
   });
+
+  test("sub components, loops, and shouldUpdate", async () => {
+    class ChildWidget extends Component<any, any> {
+      static template = xml`<span><t t-esc="props.val"/></span>`;
+      shouldUpdate(nextProps) {
+        if (nextProps.val === 12) {
+          return false;
+        }
+        return true;
+      }
+    }
+
+    class Parent extends Component<any, any> {
+      static template = xml`
+          <div>
+            <t t-foreach="state.records" t-as="record">
+              <ChildWidget t-key="record.id" val="record.val"/>
+            </t>
+          </div>`;
+      state = useState({
+        records: [{ id: 1, val: 1 }, { id: 2, val: 2 }, { id: 3, val: 3 }]
+      });
+      static components = { ChildWidget };
+    }
+    const parent = new Parent(env);
+    await parent.mount(fixture);
+    expect(normalize(fixture.innerHTML)).toBe(
+      "<div><span>1</span><span>2</span><span>3</span></div>"
+    );
+
+    parent.state.records[0].val = 11;
+    parent.state.records[1].val = 12;
+    parent.state.records[2].val = 13;
+    await nextTick();
+    expect(normalize(fixture.innerHTML)).toBe(
+      "<div><span>11</span><span>2</span><span>13</span></div>"
+    );
+  });
 });
 
 describe("props evaluation ", () => {
@@ -2707,7 +2762,7 @@ describe("async rendering", () => {
         return defs[index++];
       }
       patched() {
-        steps.push('patched');
+        steps.push("patched");
       }
     }
 
@@ -2730,7 +2785,45 @@ describe("async rendering", () => {
     defs[1].resolve();
     await nextTick();
     expect(fixture.innerHTML).toBe("<div><span>3</span></div>");
-    expect(steps).toEqual(['patched']);
+    expect(steps).toEqual(["patched"]);
+  });
+
+  test("update a sub-component twice in the same frame, 2", async () => {
+    const steps: string[] = [];
+    class ChildA extends Component<any, any> {
+      static template = xml`<span><t t-esc="val()"/></span>`;
+      patched() {
+        steps.push("patched");
+      }
+      val() {
+        steps.push("render");
+        return this.props.val;
+      }
+    }
+
+    class Parent extends Component<any, any> {
+      static template = xml`<div><ChildA val="state.valA"/></div>`;
+      static components = { ChildA };
+      state = useState({ valA: 1 });
+    }
+    const parent = new Parent(env);
+    await parent.mount(fixture);
+    expect(fixture.innerHTML).toBe("<div><span>1</span></div>");
+    parent.state.valA = 2;
+    await nextMicroTick();
+    expect(steps).toEqual(["render"]);
+    await nextMicroTick();
+    expect(steps).toEqual(["render", "render"]);
+    expect(fixture.innerHTML).toBe("<div><span>1</span></div>");
+    parent.state.valA = 3;
+    await nextMicroTick();
+    expect(steps).toEqual(["render", "render"]);
+    await nextMicroTick();
+    expect(steps).toEqual(["render", "render", "render"]);
+    expect(fixture.innerHTML).toBe("<div><span>1</span></div>");
+    await nextTick();
+    expect(fixture.innerHTML).toBe("<div><span>3</span></div>");
+    expect(steps).toEqual(["render", "render", "render", "patched"]);
   });
 
   test("components in a node in a t-foreach ", async () => {

tests/component/props_validation.test.ts

@@ -363,13 +363,12 @@ describe("default props", () => {
     }
 
     class App extends Widget {
-        static template = xml`<div><TestWidget/></div>`;
+      static template = xml`<div><TestWidget/></div>`;
-        static components = { TestWidget };
+      static components = { TestWidget };
     }
 
     const w = new App(env, {});
     await w.mount(fixture);
-    expect(fixture.innerHTML).toBe('<div><span>heyhey</span></div>')
+    expect(fixture.innerHTML).toBe("<div><span>heyhey</span></div>");
   });
-
 });

tests/helpers.ts

@@ -1,19 +1,15 @@
-import { Env } from "../src/component/component";
+import { Env, scheduler } from "../src/component/component";
 import { EvalContext, QWeb } from "../src/qweb/qweb";
 import { patch } from "../src/vdom";
 import "../src/qweb/base_directives";
 import "../src/qweb/extensions";
 import "../src/component/directive";
 
-// modifies scheduler to make it easier to test components
+// modifies scheduler to make it faster to test components
-// let current;
+scheduler.requestAnimationFrame = function(callback: FrameRequestCallback) {
-// scheduler.requestAnimationFrame = function(callback: FrameRequestCallback) {
+  setTimeout(callback, 1);
-//   if (current) {
+  return 1;
-//     throw new Error("should not schedule 2 callbacks!");
+};
-//   }
-//   current = callback;
-//   return 1;
-// };
 
 // Some static cleanup
 let nextSlotId;
@@ -22,7 +18,6 @@ let nextId;
 let TEMPLATES;
 
 beforeEach(() => {
-  // current = null;
   nextSlotId = QWeb.nextSlotId;
   slots = Object.assign({}, QWeb.slots);
   nextId = QWeb.nextId;
@@ -30,7 +25,6 @@ beforeEach(() => {
 });
 
 afterEach(() => {
-  // current = null;
   QWeb.nextSlotId = nextSlotId;
   QWeb.slots = slots;
   QWeb.nextId = nextId;
@@ -41,22 +35,10 @@ afterEach(() => {
 export function nextMicroTick(): Promise<void> {
   return Promise.resolve();
 }
-// export async function nextTick(): Promise<void> {
-//   await Promise.resolve();
-//   let max = 1000;
-//   while (current && max > 0) {
-//     const cb = current;
-//     console.warn('next');
-//     current = null;
-//     cb();
-//     max--;
-//     await Promise.resolve();
-//   }
-// }
 
 export async function nextTick(): Promise<void> {
-  return new Promise(function (resolve) {
+  return new Promise(function(resolve) {
-    setTimeout(() => requestAnimationFrame(() => resolve()));
+    setTimeout(() => scheduler.requestAnimationFrame(() => resolve()));
   });
 }
 

tests/router/__snapshots__/route_component.test.ts.snap

@@ -13,7 +13,6 @@ exports[`RouteComponent can render simple cases 1`] = `
     if (context['routeComponent']) {
         //COMPONENT
         let key3 = 'key' + context['env'].router.currentRouteName;
-        let def1;
         let templateId2 = \`__4__\` + key3;
         let w2 = templateId2 in parent.__owl__.cmap ? parent.__owl__.children[parent.__owl__.cmap[templateId2]] : false;
         let vn5 = {};
@@ -23,24 +22,24 @@ exports[`RouteComponent can render simple cases 1`] = `
             w2.destroy();
             w2 = false;
         }
-        if (!w2) {
+        if (w2) {
+            w2.__updateProps(props2, extra.fiber, undefined, undefined, sibling);
+            let pvnode = w2.__owl__.pvnode;
+            utils.defineProxy(vn5, pvnode);
+        } else {
             let componentKey2 = \`routeComponent\`;
             let W2 = context.constructor.components[componentKey2] || QWeb.components[componentKey2]|| context['routeComponent'];
             if (!W2) {throw new Error('Cannot find the definition of component \\"' + componentKey2 + '\\"')}
             w2 = new W2(parent, props2);
             parent.__owl__.cmap[templateId2] = w2.__owl__.id;
-            def1 = w2.__prepare(extra.fiber, undefined, undefined, sibling);
+            let def1 = w2.__prepare(extra.fiber, undefined, undefined, sibling);
             let pvnode = h('dummy', {key: templateId2, hook: {insert(vn) { let nvn=w2.__mount(fiber, pvnode.elm);pvnode.elm=nvn.elm;},remove() {},destroy(vn) {w2.destroy();}}});
             const fiber = w2.__owl__.currentFiber;
             def1.then(function () {if (w2.__owl__.isDestroyed) {return;} const vnode = fiber.vnode; pvnode.sel = vnode.sel; });
             utils.defineProxy(vn5, pvnode);
             w2.__owl__.pvnode = pvnode;
-        } else {
-            def1 = def1 || w2.__updateProps(props2, extra.fiber, undefined, undefined, sibling);
-            let pvnode = w2.__owl__.pvnode;
-            utils.defineProxy(vn5, pvnode);
         }
-        sibling = w2.__owl__.currentFiber;
+        sibling = w2.__owl__.currentFiber || sibling;
     }
     return result;
 }"