PROTOTYPE: batch renders in animation frame instead of microtick

doc/reference/reactivity.md

@@ -117,3 +117,10 @@ rawState.value = 3; // will NOT be picked up by the reactivity system!!!
 Here again, this is useful in some situations where we want to explicitely bypass
 Owl, but using this function means that the responsability of coordinating
 state update is given to the user code, instead of Owl. Subtle bugs may arise!
+
+Also, normal (non-reactive objects) will be directly returned by `toRaw`:
+
+```js
+const obj = { a: 1 };
+console.log(toRaw(obj) === obj); // true
+```

doc/reference/templates.md

@@ -21,6 +21,7 @@
 - [Fragments](#fragments)
 - [Inline templates](#inline-templates)
 - [Rendering svg](#rendering-svg)
+- [Restrictions](#restrictions)
 
 ## Overview
 
@@ -680,3 +681,13 @@ if a template is supposed to be included in a svg namespace or not. Therefore,
 Owl depends on a heuristic: if a tag is either `svg`, `g` or `path`, then it will
 be considered as svg. In practice, this means that each component or each sub
 templates (included with `t-call`) should have one of these tag as root tag.
+
+## Restrictions
+
+Note that Owl templates forbid the use of tag and or attributes starting with
+the `block-` string. This restriction prevents name collision with the internal
+code of Owl.
+
+```xml
+<div><block-1>this will not be accepted by Owl</block-1></div>
+```

src/app/template_set.ts

@@ -1,6 +1,6 @@
 import { createBlock, html, list, multi, text, toggler, comment } from "../blockdom";
 import { compile, Template } from "../compiler";
-import { component } from "../component/component_node";
+import { component, getCurrent } from "../component/component_node";
 import { UTILS } from "./template_helpers";
 
 const bdom = { text, createBlock, list, multi, html, toggler, component, comment };
@@ -94,7 +94,12 @@ export class TemplateSet {
     if (!(name in this.templates)) {
       const rawTemplate = this.rawTemplates[name];
       if (rawTemplate === undefined) {
-        throw new Error(`Missing template: "${name}"`);
+        let extraInfo = "";
+        try {
+          const componentName = getCurrent().component.constructor.name;
+          extraInfo = ` (for component "${componentName}")`;
+        } catch {}
+        throw new Error(`Missing template: "${name}"${extraInfo}`);
       }
       const templateFn = this._compileTemplate(name, rawTemplate);
       // first add a function to lazily get the template, in case there is a

src/compiler/parser.ts

@@ -305,6 +305,9 @@ function parseDOMNode(node: Element, ctx: ParsingContext): AST | null {
   if (tagName === "t" && !dynamicTag) {
     return null;
   }
+  if (tagName.startsWith("block-")) {
+    throw new Error(`Invalid tag name: '${tagName}'`);
+  }
   ctx = Object.assign({}, ctx);
   if (tagName === "pre") {
     ctx.inPreTag = true;
@@ -371,6 +374,8 @@ function parseDOMNode(node: Element, ctx: ParsingContext): AST | null {
         ctx = Object.assign({}, ctx);
         ctx.tModelInfo = model;
       }
+    } else if (attr.startsWith("block-")) {
+      throw new Error(`Invalid attribute: '${attr}'`);
     } else if (attr !== "t-name") {
       if (attr.startsWith("t-") && !attr.startsWith("t-att")) {
         throw new Error(`Unknown QWeb directive: '${attr}'`);

src/component/component_node.ts

@@ -120,20 +120,30 @@ export class ComponentNode<P = any, E = any> implements VNode<ComponentNode<P, E
     this.initiateRender(fiber);
   }
 
-  async initiateRender(fiber: Fiber | MountFiber) {
+  initiateRender(fiber: Fiber | MountFiber) {
     this.fiber = fiber;
     if (this.mounted.length) {
       fiber.root!.mounted.push(fiber);
     }
     const component = this.component;
+    const finish = () => {
+      if (this.status === STATUS.NEW && this.fiber === fiber) {
+        this._render(fiber);
+      }
+    }
+    let willStartResults;
     try {
-      await Promise.all(this.willStart.map((f) => f.call(component)));
+      willStartResults = this.willStart.map((f) => f.call(component));
     } catch (e) {
       handleError({ node: this, error: e });
       return;
     }
-    if (this.status === STATUS.NEW && this.fiber === fiber) {
-      this._render(fiber);
+    if (willStartResults.some(result => result instanceof Promise)) {
+      Promise.all(willStartResults)
+        .then(finish)
+        .catch(e => handleError({ node: this, error: e }));
+    } else {
+      finish();
     }
   }
 
@@ -153,25 +163,27 @@ export class ComponentNode<P = any, E = any> implements VNode<ComponentNode<P, E
 
     const fiber = makeRootFiber(this);
     this.fiber = fiber;
+    debugger;
+    window.requestAnimationFrame(() => {
+      if (this.status === STATUS.DESTROYED) {
+        return;
+      }
+      // We only want to actually render the component if the following two
+      // conditions are true:
+      // * this.fiber: it could be null, in which case the render has been cancelled
+      // * (current || !fiber.parent): if current is not null, this means that the
+      //   render function was called when a render was already occurring. In this
+      //   case, the pending rendering was cancelled, and the fiber needs to be
+      //   rendered to complete the work.  If current is null, we check that the
+      //   fiber has no parent.  If that is the case, the fiber was downgraded from
+      //   a root fiber to a child fiber in the previous microtick, because it was
+      //   embedded in a rendering coming from above, so the fiber will be rendered
+      //   in the next microtick anyway, so we should not render it again.
+      if (this.fiber === fiber && (current || !fiber.parent)) {
+        this._render(fiber);
+      }
+    });
     this.app.scheduler.addFiber(fiber);
-    await Promise.resolve();
-    if (this.status === STATUS.DESTROYED) {
-      return;
-    }
-    // We only want to actually render the component if the following two
-    // conditions are true:
-    // * this.fiber: it could be null, in which case the render has been cancelled
-    // * (current || !fiber.parent): if current is not null, this means that the
-    //   render function was called when a render was already occurring. In this
-    //   case, the pending rendering was cancelled, and the fiber needs to be
-    //   rendered to complete the work.  If current is null, we check that the
-    //   fiber has no parent.  If that is the case, the fiber was downgraded from
-    //   a root fiber to a child fiber in the previous microtick, because it was
-    //   embedded in a rendering coming from above, so the fiber will be rendered
-    //   in the next microtick anyway, so we should not render it again.
-    if (this.fiber === fiber && (current || !fiber.parent)) {
-      this._render(fiber);
-    }
   }
 
   _render(fiber: Fiber | RootFiber) {
@@ -207,25 +219,32 @@ export class ComponentNode<P = any, E = any> implements VNode<ComponentNode<P, E
     this.status = STATUS.DESTROYED;
   }
 
-  async updateAndRender(props: any, parentFiber: Fiber) {
+  updateAndRender(props: any, parentFiber: Fiber) {
     // update
     const fiber = makeChildFiber(this, parentFiber);
     this.fiber = fiber;
     const component = this.component;
     applyDefaultProps(props, component.constructor as any);
-    const prom = Promise.all(this.willUpdateProps.map((f) => f.call(component, props)));
-    await prom;
-    if (fiber !== this.fiber) {
-      return;
+
+    const finish = () => {
+      if (fiber !== this.fiber) {
+        return;
+      }
+      component.props = props;
+      this._render(fiber);
+      const parentRoot = parentFiber.root!;
+      if (this.willPatch.length) {
+        parentRoot.willPatch.push(fiber);
+      }
+      if (this.patched.length) {
+        parentRoot.patched.push(fiber);
+      }
     }
-    component.props = props;
-    this._render(fiber);
-    const parentRoot = parentFiber.root!;
-    if (this.willPatch.length) {
-      parentRoot.willPatch.push(fiber);
-    }
-    if (this.patched.length) {
-      parentRoot.patched.push(fiber);
+    const willUpdatePropsResults = this.willUpdateProps.map((f) => f.call(component, props));
+    if (willUpdatePropsResults.some(res => res instanceof Promise)) {
+      Promise.all(willUpdatePropsResults).then(finish);
+    } else {
+      finish();
     }
   }
 

src/component/fibers.ts

@@ -6,8 +6,7 @@ import { STATUS } from "./status";
 export function makeChildFiber(node: ComponentNode, parent: Fiber): Fiber {
   let current = node.fiber;
   if (current) {
-    let root = parent.root;
-    cancelFibers(root, current.children);
+    cancelFibers(current.children);
     current.root = null;
   }
   return new Fiber(node, parent);
@@ -17,7 +16,7 @@ export function makeRootFiber(node: ComponentNode): Fiber {
   let current = node.fiber;
   if (current) {
     let root = current.root!;
-    root.counter -= cancelFibers(root, current.children);
+    root.counter -= cancelFibers(current.children);
     current.children = [];
     root.counter++;
     current.bdom = null;
@@ -42,15 +41,14 @@ export function makeRootFiber(node: ComponentNode): Fiber {
 /**
  * @returns number of not-yet rendered fibers cancelled
  */
-function cancelFibers(root: any, fibers: Fiber[]): number {
+function cancelFibers(fibers: Fiber[]): number {
   let result = 0;
   for (let fiber of fibers) {
     fiber.node.fiber = null;
-    fiber.root = root;
     if (!fiber.bdom) {
       result++;
     }
-    result += cancelFibers(root, fiber.children);
+    result += cancelFibers(fiber.children);
   }
   return result;
 }

src/reactivity.ts

@@ -57,7 +57,7 @@ export function markRaw<T extends Target>(value: T): NonReactive<T> {
  * @returns the underlying value
  */
 export function toRaw<T extends object>(value: Reactive<T>): T {
-  return value[TARGET];
+  return value[TARGET] || value;
 }
 
 const targetToKeysToCallbacks = new WeakMap<Target, Map<ObjectKey, Set<Callback>>>();

tests/compiler/blacklist.test.ts

@@ -0,0 +1,13 @@
+import { renderToString } from "../helpers";
+
+describe("blacklisted tags and attributes", () => {
+  test("template with block-text tag", () => {
+    const template = `<div><block-text-0/>hello</div>`;
+    expect(() => renderToString(template)).toThrow("Invalid tag name: 'block-text-0'");
+  });
+
+  test("template with block-handler tag", () => {
+    const template = `<div block-handler-0="click">hello</div>`;
+    expect(() => renderToString(template)).toThrow("Invalid attribute: 'block-handler-0'");
+  });
+});

tests/components/basics.test.ts

@@ -204,7 +204,7 @@ describe("basics", () => {
       error = e as Error;
     }
     expect(error!).toBeDefined();
-    expect(error!.message).toBe('Missing template: "wrongtemplate"');
+    expect(error!.message).toBe('Missing template: "wrongtemplate" (for component "Test")');
   });
 
   test("class component with dynamic text", async () => {

tests/components/concurrency.test.ts

@@ -97,17 +97,14 @@ test("destroying/recreating a subwidget with different props (if start is not ov
   expect(["W:setup", "W:willStart", "W:willRender", "W:rendered", "W:mounted"]).toBeLogged();
 
   expect(n).toBe(0);
-
   w.state.val = 2;
-  await nextMicroTick();
-  await nextMicroTick();
+  await new Promise(requestAnimationFrame); // calls to _render are batched per animation frame
   expect(n).toBe(1);
 
   expect(["W:willRender", "Child:setup", "Child:willStart", "W:rendered"]).toBeLogged();
 
   w.state.val = 3;
-  await nextMicroTick();
-  await nextMicroTick();
+  await new Promise(requestAnimationFrame);
   expect(n).toBe(2);
 
   expect([

tests/reactivity.test.ts

@@ -1131,6 +1131,11 @@ describe("toRaw", () => {
     expect(reactiveObj).not.toBe(obj);
     expect(toRaw(reactiveObj as Reactive<typeof obj>)).toBe(obj);
   });
+
+  test("giving a non reactive to toRaw return the object itself", () => {
+    const obj = { value: 1 };
+    expect(toRaw(obj as Reactive<typeof obj>)).toBe(obj);
+  });
 });
 
 describe("Reactivity: useState", () => {