[IMP] component: render does not return a promise anymore

src/component/component.ts

@@ -26,7 +26,7 @@ export class Component {
 
   setup() {}
 
-  render(): Promise<void> {
-    return this.__owl__.render();
+  render() {
+    this.__owl__.render();
   }
 }

src/component/component_node.ts

@@ -139,10 +139,9 @@ export class ComponentNode<T extends typeof Component = typeof Component>
   async render() {
     const current = this.fiber;
     if (current && !current.bdom && !fibersInError.has(current)) {
-      return current.root.promise;
+      return;
     }
     if (!this.bdom && !current) {
-      // should find a way to return the future mounting promise
       return;
     }
     const fiber = makeRootFiber(this);
@@ -166,7 +165,6 @@ export class ComponentNode<T extends typeof Component = typeof Component>
     if (this.fiber && (current || !fiber.parent)) {
       this._render(fiber);
     }
-    return fiber.root.promise;
   }
 
   _render(fiber: Fiber | RootFiber) {

src/component/fibers.ts

@@ -33,7 +33,7 @@ export function makeRootFiber(node: ComponentNode): Fiber {
     }
     return current;
   }
-  const fiber = new RootFiber(node);
+  const fiber = new RootFiber(node, null);
   if (node.willPatch.length) {
     fiber.willPatch.push(fiber);
   }
@@ -84,25 +84,12 @@ export class Fiber {
 
 export class RootFiber extends Fiber {
   counter: number = 1;
-  resolve: any;
-  promise: Promise<any>;
-  reject: any;
 
   // only add stuff in this if they have registered some hooks
   willPatch: Fiber[] = [];
   patched: Fiber[] = [];
   mounted: Fiber[] = [];
 
-  constructor(node: ComponentNode) {
-    super(node, null);
-    this.counter = 1;
-
-    this.promise = new Promise((resolve, reject) => {
-      this.resolve = resolve;
-      this.reject = reject;
-    });
-  }
-
   complete() {
     const node = this.node;
     let current: Fiber | undefined = undefined;
@@ -150,9 +137,7 @@ export class RootFiber extends Fiber {
       // unregistering the fiber
       node.fiber = null;
     } catch (e) {
-      if (!handleError({ fiber: current || this, error: e })) {
-        this.reject(e);
-      }
+      handleError({ fiber: current || this, error: e });
     }
   }
 }
@@ -166,11 +151,18 @@ export interface MountOptions {
 export class MountFiber extends RootFiber {
   target: HTMLElement;
   position: Position;
+  resolve: any;
+  promise: Promise<any>;
+  reject: any;
 
   constructor(node: ComponentNode, target: HTMLElement, options: MountOptions = {}) {
-    super(node);
+    super(node, null);
     this.target = target;
     this.position = options.position || "last-child";
+    this.promise = new Promise((resolve, reject) => {
+      this.resolve = resolve;
+      this.reject = reject;
+    });
   }
   complete() {
     let current: Fiber | undefined = this;
@@ -199,5 +191,6 @@ export class MountFiber extends RootFiber {
         this.reject(e);
       }
     }
+    this.resolve();
   }
 }

src/component/scheduler.ts

@@ -1,4 +1,4 @@
-import { Fiber, RootFiber } from "./fibers";
+import { Fiber, MountFiber, RootFiber } from "./fibers";
 import { fibersInError } from "./error_handling";
 import { STATUS } from "./status";
 
@@ -38,19 +38,15 @@ export class Scheduler {
   flush() {
     this.tasks.forEach((fiber) => {
       if (fiber.root !== fiber) {
-        // this is wrong! should be something like
-        // if (this.tasks.has(fiber.root)) {
-        //   // parent rendering has completed
-        //   fiber.resolve();
-        //   this.tasks.delete(fiber);
-        // }
         this.tasks.delete(fiber);
         return;
       }
       const hasError = fibersInError.has(fiber);
       if (hasError && fiber.counter !== 0) {
         this.tasks.delete(fiber);
-        fiber.reject(fibersInError.get(fiber));
+        if (fiber instanceof MountFiber) {
+          fiber.reject(fibersInError.get(fiber));
+        }
         return;
       }
       if (fiber.node.status === STATUS.DESTROYED) {
@@ -61,7 +57,6 @@ export class Scheduler {
       if (fiber.counter === 0) {
         if (!hasError) {
           fiber.complete();
-          fiber.resolve();
         }
         this.tasks.delete(fiber);
       }

tests/components/__snapshots__/concurrency.test.ts.snap

@@ -800,71 +800,6 @@ exports[`concurrent renderings scenario 15 3`] = `
 }"
 `;
 
-exports[`concurrent renderings scenario 16 1`] = `
-"function anonymous(bdom, helpers
-) {
-  let { text, createBlock, list, multi, html, toggler, component } = bdom;
-  let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue, toNumber } = helpers;
-  
-  let block1 = createBlock(\`<ul>DDD</ul>\`);
-  
-  return function template(ctx, node, key = \\"\\") {
-    return block1();
-  }
-}"
-`;
-
-exports[`concurrent renderings scenario 16 2`] = `
-"function anonymous(bdom, helpers
-) {
-  let { text, createBlock, list, multi, html, toggler, component } = bdom;
-  let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue, toNumber } = helpers;
-  
-  let block1 = createBlock(\`<p><span><block-text-0/></span><span><block-text-1/></span><span><block-text-2/></span><block-child-0/></p>\`);
-  
-  return function template(ctx, node, key = \\"\\") {
-    let b2;
-    let d1 = ctx['props'].fromA;
-    let d2 = ctx['props'].fromB;
-    let d3 = ctx['state'].fromC;
-    if (ctx['state'].fromC===13) {
-      b2 = component(\`D\`, {}, key + \`__1\`, node, ctx);
-    }
-    return block1([d1, d2, d3], [b2]);
-  }
-}"
-`;
-
-exports[`concurrent renderings scenario 16 3`] = `
-"function anonymous(bdom, helpers
-) {
-  let { text, createBlock, list, multi, html, toggler, component } = bdom;
-  let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue, toNumber } = helpers;
-  
-  let block1 = createBlock(\`<p><block-child-0/></p>\`);
-  
-  return function template(ctx, node, key = \\"\\") {
-    let b2 = component(\`C\`, {fromB: ctx['state'].fromB,fromA: ctx['props'].fromA}, key + \`__1\`, node, ctx);
-    return block1([], [b2]);
-  }
-}"
-`;
-
-exports[`concurrent renderings scenario 16 4`] = `
-"function anonymous(bdom, helpers
-) {
-  let { text, createBlock, list, multi, html, toggler, component } = bdom;
-  let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue, toNumber } = helpers;
-  
-  let block1 = createBlock(\`<p><block-child-0/></p>\`);
-  
-  return function template(ctx, node, key = \\"\\") {
-    let b2 = component(\`B\`, {fromA: ctx['state'].fromA}, key + \`__1\`, node, ctx);
-    return block1([], [b2]);
-  }
-}"
-`;
-
 exports[`creating two async components, scenario 1 1`] = `
 "function anonymous(bdom, helpers
 ) {
@@ -1115,21 +1050,6 @@ exports[`properly behave when destroyed/unmounted while rendering  3`] = `
 }"
 `;
 
-exports[`render method wait until rendering is done 1`] = `
-"function anonymous(bdom, helpers
-) {
-  let { text, createBlock, list, multi, html, toggler, component, comment } = bdom;
-  let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue, toNumber, safeOutput } = helpers;
-  
-  let block1 = createBlock(\`<div><block-text-0/></div>\`);
-  
-  return function template(ctx, node, key = \\"\\") {
-    let d1 = ctx['state'].drinks;
-    return block1([d1]);
-  }
-}"
-`;
-
 exports[`rendering component again in next microtick 1`] = `
 "function anonymous(bdom, helpers
 ) {

tests/components/basics.test.ts

@@ -106,7 +106,8 @@ describe("basics", () => {
     expect(fixture.innerHTML).toBe("<span>1</span>");
 
     component.value = 2;
-    await component.render();
+    component.render();
+    await nextTick();
     expect(fixture.innerHTML).toBe("<span>2</span>");
   });
 
@@ -123,7 +124,8 @@ describe("basics", () => {
 
     expect(fixture.innerHTML).toBe("onetwothree");
     component.items = ["two", "three", "one"];
-    await component.render();
+    component.render();
+    await nextTick();
     expect(fixture.innerHTML).toBe("twothreeone");
   });
 
@@ -258,7 +260,8 @@ describe("basics", () => {
     const test = await mount(Test, fixture);
     expect(fixture.innerHTML).toBe("<div>3</div>");
     test.value = 5;
-    await test.render();
+    test.render();
+    await nextTick();
     expect(fixture.innerHTML).toBe("<div>5</div>");
   });
 

tests/components/concurrency.test.ts

@@ -1758,7 +1758,9 @@ test("concurrent renderings scenario 11", async () => {
     def.resolve();
   }, 20);
   child.val = 5;
-  await child.render();
+  child.render();
+  await def;
+  await nextTick();
   expect(fixture.innerHTML).toBe("<div><span>2|5</span></div>");
   expect([
     "Parent:willRender",
@@ -2315,7 +2317,8 @@ test("calling render in destroy", async () => {
 
   a.state = "A";
   a.key = 2;
-  await a.render();
+  a.render();
+  await nextTick();
   // this nextTick is critical, otherwise jest may silently swallow errors
   await nextTick();
   expect([
@@ -2362,7 +2365,8 @@ test("change state and call manually render: no unnecessary rendering", async ()
   expect(numberOfRender).toBe(1);
 
   test.state.val = 2;
-  await test.render();
+  test.render();
+  await nextTick();
   expect(fixture.innerHTML).toBe("<div>2</div>");
   expect(numberOfRender).toBe(2);
   expect(["Test:willRender", "Test:rendered", "Test:willPatch", "Test:patched"]).toBeLogged();
@@ -2456,22 +2460,6 @@ test("changing state before first render does not trigger a render (with parent)
   ]).toBeLogged();
 });
 
-test("render method wait until rendering is done", async () => {
-  class TestW extends Component {
-    static template = xml`<div><t t-esc="state.drinks"/></div>`;
-    state = { drinks: 1 };
-  }
-  const widget = await mount(TestW, 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("two renderings initiated between willPatch and patched", async () => {
   let parent: any = null;
 

tests/components/error_handling.test.ts

@@ -1,16 +1,15 @@
 import { Component, mount } from "../../src";
-import { status } from "../../src/component/status";
-import { xml } from "../../src/tags";
-import { makeTestFixture, nextTick, snapshotEverything } from "../helpers";
 import {
+  onError,
   onMounted,
   onPatched,
   onWillPatch,
   onWillStart,
   onWillUnmount,
   useState,
-  onError,
 } from "../../src/index";
+import { xml } from "../../src/tags";
+import { makeTestFixture, nextTick, snapshotEverything } from "../helpers";
 
 let fixture: HTMLElement;
 
@@ -35,18 +34,9 @@ describe("basics", () => {
     expect(fixture.innerHTML).toBe("<div><div>heyfalse</div></div>");
     parent.state.flag = true;
 
-    let error: Error;
-    try {
-      await parent.render();
-    } catch (e) {
-      error = e as Error;
-    }
+    parent.render();
+    await nextTick();
     expect(fixture.innerHTML).toBe("");
-    expect(status(parent)).toBe("destroyed");
-    expect(error!).toBeDefined();
-    const regexp =
-      /Cannot read properties of undefined \(reading 'this'\)|Cannot read property 'this' of undefined/g;
-    expect(error!.message).toMatch(regexp);
   });
 
   test("display a nice error if it cannot find component", async () => {
@@ -154,28 +144,24 @@ describe("errors and promises", () => {
     const consoleError = console.error;
     console.error = jest.fn(() => {});
 
-    class App extends Component {
+    class Root extends Component {
       static template = xml`<div><t t-esc="val"/></div>`;
       val = 3;
       setup() {
         onWillPatch(() => {
           throw new Error("boom");
         });
+        onError((e) => (error = e));
       }
     }
 
-    const app = await mount(App, fixture);
-    app.val = 4;
+    const root = await mount(Root, fixture);
+    root.val = 4;
     let error: Error;
-    try {
-      await app.render();
-    } catch (e) {
-      error = e as Error;
-    }
+    root.render();
+    await nextTick();
     expect(error!).toBeDefined();
     expect(error!.message).toBe("boom");
-    expect(fixture.innerHTML).toBe("");
-
     expect(console.error).toBeCalledTimes(0);
     console.error = consoleError;
   });
@@ -184,28 +170,24 @@ describe("errors and promises", () => {
     const consoleError = console.error;
     console.error = jest.fn(() => {});
 
-    class App extends Component {
+    class Root extends Component {
       static template = xml`<div><t t-esc="val"/></div>`;
       val = 3;
       setup() {
         onPatched(() => {
           throw new Error("boom");
         });
+        onError((e) => (error = e));
       }
     }
 
-    const app = await mount(App, fixture);
-    app.val = 4;
+    const root = await mount(Root, fixture);
+    root.val = 4;
     let error: Error;
-    try {
-      await app.render();
-    } catch (e) {
-      error = e as Error;
-    }
+    root.render();
+    await nextTick();
     expect(error!).toBeDefined();
     expect(error!.message).toBe("boom");
-    expect(fixture.innerHTML).toBe("");
-
     expect(console.error).toBeCalledTimes(0);
     console.error = consoleError;
   });
@@ -241,20 +223,20 @@ describe("errors and promises", () => {
     const consoleError = console.error;
     console.error = jest.fn(() => {});
     // we do not catch error in willPatch anymore
-    class App extends Component {
+    class Root extends Component {
       static template = xml`<div><t t-if="flag" t-esc="this.will.crash"/></div>`;
       flag = false;
+      setup() {
+        onError((e) => (error = e));
+      }
     }
 
-    const app = await mount(App, fixture);
+    const root = await mount(Root, fixture);
     expect(fixture.innerHTML).toBe("<div></div>");
-    app.flag = true;
+    root.flag = true;
     let error: Error;
-    try {
-      await app.render();
-    } catch (e) {
-      error = e as Error;
-    }
+    root.render();
+    await nextTick();
     expect(error!).toBeDefined();
     const regexp =
       /Cannot read properties of undefined \(reading 'crash'\)|Cannot read property 'crash' of undefined/g;

tests/components/higher_order_component.test.ts

@@ -69,7 +69,8 @@ describe("basics", () => {
     expect(fixture.innerHTML).toBe("<span>CHILD 1</span>");
 
     env.options.flag = false;
-    await parent.render();
+    parent.render();
+    await nextTick();
     expect(fixture.innerHTML).toBe("<div>CHILD 2</div>");
   });
 

tests/components/props_validation.test.ts

@@ -1,5 +1,5 @@
-import { makeTestFixture, snapshotApp } from "../helpers";
-import { Component, xml } from "../../src";
+import { makeTestFixture, nextTick, snapshotApp } from "../helpers";
+import { Component, onError, xml } from "../../src";
 import { App, DEV_MSG } from "../../src/app/app";
 import { validateProps } from "../../src/component/props_validation";
 
@@ -667,15 +667,15 @@ describe("props validation", () => {
       static template = xml`<div><SubComp p="state.p"/></div>`;
       static components = { SubComp };
       state: any = { p: 1 };
+      setup() {
+        onError((e) => (error = e));
+      }
     }
     const app = await mountApp(Parent);
     expect(fixture.innerHTML).toBe("<div><div>1</div></div>");
-    try {
-      (app as any).root.component.state.p = undefined;
-      await (app as any).root.component.render();
-    } catch (e) {
-      error = e as Error;
-    }
+    (app as any).root.component.state.p = undefined;
+    (app as any).root.component.render();
+    await nextTick();
     expect(error!).toBeDefined();
     expect(error!.message).toBe("Missing props 'p' (component 'SubComp')");
   });
@@ -696,7 +696,8 @@ describe("props validation", () => {
     const app = await mountApp(Parent);
     expect(fixture.innerHTML).toBe("<div><div>1</div></div>");
     (app as any).root.component.state.p = undefined;
-    await (app as any).root.component.render();
+    (app as any).root.component.render();
+    await nextTick();
     expect(fixture.innerHTML).toBe("<div><div>4</div></div>");
   });
 
@@ -754,7 +755,8 @@ describe("default props", () => {
     const app = await mountApp(Parent);
     expect(fixture.innerHTML).toBe("<div><div>1</div></div>");
     (app as any).root.component.state.p = undefined;
-    await (app as any).root.component.render();
+    (app as any).root.component.render();
+    await nextTick();
     expect(fixture.innerHTML).toBe("<div><div>4</div></div>");
   });
 

tests/components/t_foreach.test.ts

@@ -291,7 +291,8 @@ describe("list of components", () => {
     const parent = await mount(Parent, fixture);
     expect((parent.el as HTMLElement).innerHTML).toBe("<div>1</div><div>2</div>");
     parent.clist = [2, 1];
-    await parent.render();
+    parent.render();
+    await nextTick();
     expect((parent.el as HTMLElement).innerHTML).toBe("<div>2</div><div>1</div>");
     expect(childInstances.length).toBe(2);
   });

tests/components/t_key.test.ts

@@ -1,4 +1,4 @@
-import { snapshotEverything, makeTestFixture, addTemplate } from "../helpers";
+import { snapshotEverything, makeTestFixture, addTemplate, nextTick } from "../helpers";
 import { Component, mount, xml } from "../../src/index";
 
 snapshotEverything();
@@ -32,7 +32,8 @@ describe("t-key", () => {
 
     const oldChild = childInstance;
     parent.key = 2;
-    await parent.render();
+    parent.render();
+    await nextTick();
     expect((parent.el as HTMLElement).innerHTML).toBe("<div>2</div>");
     expect(oldChild === childInstance).toBeFalsy();
   });
@@ -65,7 +66,8 @@ describe("t-key", () => {
 
     const oldChild = childInstance;
     __key = 2;
-    await parent.render();
+    parent.render();
+    await nextTick();
     expect((parent.el as HTMLElement).innerHTML).toBe("<div>2</div>");
     expect(oldChild === childInstance).toBeFalsy();
     expect(keyCalls).toBe(4);
@@ -97,7 +99,8 @@ describe("t-key", () => {
 
     parent.key1 = 2;
     parent.key2 = 1;
-    await parent.render();
+    parent.render();
+    await nextTick();
     expect((parent.el as HTMLElement).innerHTML).toBe("<div>2</div><div>1</div>");
     expect(childInstances.length).toBe(4);
   });
@@ -130,7 +133,8 @@ describe("t-key", () => {
 
     parent.key1 = 2;
     parent.key2 = 1;
-    await parent.render();
+    parent.render();
+    await nextTick();
     expect((parent.el as HTMLElement).innerHTML).toBe("<div>2</div><div>1</div>");
     expect(childInstances.length).toBe(4);
   });
@@ -165,7 +169,8 @@ describe("t-key", () => {
 
     parent.key1 = 2;
     parent.key2 = 1;
-    await parent.render();
+    parent.render();
+    await nextTick();
     expect((parent.el as HTMLElement).innerHTML).toBe("<div>2</div><div>1</div>");
     expect(childInstances.length).toBe(4);
   });
@@ -194,14 +199,16 @@ describe("t-key", () => {
     const parent = await mount(Parent, fixture);
     expect((parent.el as HTMLElement).innerHTML).toBe("<div>1key1</div><div>2key1</div>");
     parent.clist = [2, 1];
-    await parent.render();
+    parent.render();
+    await nextTick();
     expect((parent.el as HTMLElement).innerHTML).toBe("<div>2key1</div><div>1key1</div>");
     expect(childInstances.length).toBe(2);
     childInstances.length = 0;
 
     parent.clist = [1, 2];
     parent.key1 = "key2";
-    await parent.render();
+    parent.render();
+    await nextTick();
     expect((parent.el as HTMLElement).innerHTML).toBe("<div>1key2</div><div>2key2</div>");
     expect(childInstances.length).toBe(2);
   });

tests/components/t_set.test.ts

@@ -1,5 +1,5 @@
 import { Component, mount, xml } from "../../src";
-import { makeTestFixture, snapshotEverything } from "../helpers";
+import { makeTestFixture, nextTick, snapshotEverything } from "../helpers";
 
 snapshotEverything();
 
@@ -37,10 +37,12 @@ describe("t-set", () => {
 
     expect(fixture.innerHTML).toBe("<div><p>2</p></div>");
     comp.state.flag = "elif";
-    await comp.render();
+    comp.render();
+    await nextTick();
     expect(fixture.innerHTML).toBe("<div><p>3</p></div>");
     comp.state.flag = "false";
-    await comp.render();
+    comp.render();
+    await nextTick();
     expect(fixture.innerHTML).toBe("<div><p>4</p></div>");
   });
 
@@ -68,10 +70,12 @@ describe("t-set", () => {
 
     expect(fixture.innerHTML).toBe("<div><p>2</p></div>");
     comp.state.flag = "elif";
-    await comp.render();
+    comp.render();
+    await nextTick();
     expect(fixture.innerHTML).toBe("<div><p>3</p></div>");
     comp.state.flag = "false";
-    await comp.render();
+    comp.render();
+    await nextTick();
     expect(fixture.innerHTML).toBe("<div><p>4</p></div>");
   });
 

tests/misc/portal.test.ts

@@ -2,6 +2,7 @@ import {
   App,
   Component,
   mount,
+  onError,
   onMounted,
   onPatched,
   onWillPatch,
@@ -391,17 +392,17 @@ describe("Portal", () => {
           </Portal>
         </div>`;
       state = { error: false };
+      setup() {
+        onError((e) => (error = e));
+      }
     }
     addOutsideDiv(fixture);
     const parent = await mount(Parent, fixture);
-    parent.state.error = true;
 
     let error: Error;
-    try {
-      await parent.render();
-    } catch (e) {
-      error = e as Error;
-    }
+    parent.state.error = true;
+    parent.render();
+    await nextTick();
     expect(error!).toBeDefined();
     const regexp =
       /Cannot read properties of undefined \(reading 'crash'\)|Cannot read property 'crash' of undefined/g;