[IMP] component: do not set props in root components

Also remove props from Fiber, as it was not useful anymore.

src/component/component.ts

@@ -114,23 +114,17 @@ export class Component<T extends Env, Props extends {}> {
   constructor(parent?: Component<T, any>, props?: Props) {
     Component.current = this;
 
-    const defaultProps = (<any>this.constructor).defaultProps;
-    if (defaultProps) {
-      props = this.__applyDefaultProps(props, defaultProps);
-    }
-    // is this a good idea?
-    //   Pro: if props is empty, we can create easily a component
-    //   Con: this is not really safe
-    //   Pro: but creating component (by a template) is always unsafe anyway
-    this.props = <Props>props || <Props>{};
-    if (QWeb.dev) {
-      QWeb.utils.validateProps(this.constructor, this.props);
-    }
-
     const id: number = nextId++;
-
     let depth;
     if (parent) {
+      const defaultProps = (<any>this.constructor).defaultProps;
+      if (defaultProps) {
+        props = this.__applyDefaultProps(props, defaultProps);
+      }
+      this.props = <Props>props;
+      if (QWeb.dev) {
+        QWeb.utils.validateProps(this.constructor, this.props);
+      }
       this.env = parent.env;
       const __powl__ = parent.__owl__;
       __powl__.children[id] = this;
@@ -138,6 +132,7 @@ export class Component<T extends Env, Props extends {}> {
     } else {
       // we are the root component
       this.env = config.env as T;
+      this.props = undefined as unknown as Props;
       this.env.qweb.on("update", this, () => {
         if (this.__owl__.isMounted) {
           this.render(true);
@@ -287,7 +282,7 @@ export class Component<T extends Env, Props extends {}> {
       return;
     }
     return new Promise((resolve, reject) => {
-      const fiber = new Fiber(null, this, this.props, undefined, undefined, false);
+      const fiber = new Fiber(null, this, undefined, undefined, false);
       scheduler.addFiber(fiber, err => {
         if (err) {
           reject(err);
@@ -339,7 +334,7 @@ export class Component<T extends Env, Props extends {}> {
       return;
     }
     return new Promise((resolve, reject) => {
-      const fiber = new Fiber(null, this, this.props, undefined, undefined, force);
+      const fiber = new Fiber(null, this, undefined, undefined, force);
       scheduler.addFiber(fiber.root, err => {
         if (err) {
           reject(err);
@@ -488,7 +483,7 @@ export class Component<T extends Env, Props extends {}> {
     const shouldUpdate = parentFiber.force || this.shouldUpdate(nextProps);
     if (shouldUpdate) {
       const __owl__ = this.__owl__;
-      const fiber = new Fiber(parentFiber, this, this.props, scope, vars, parentFiber.force);
+      const fiber = new Fiber(parentFiber, this, scope, vars, parentFiber.force);
       if (!parentFiber.child) {
         parentFiber.child = fiber;
       } else {
@@ -532,7 +527,7 @@ export class Component<T extends Env, Props extends {}> {
    * parent template.
    */
   __prepare(parentFiber: Fiber, scope: any, vars: any, previousSibling?: Fiber | null) {
-    const fiber = new Fiber(parentFiber, this, this.props, scope, vars, parentFiber.force);
+    const fiber = new Fiber(parentFiber, this, scope, vars, parentFiber.force);
     fiber.shouldPatch = false;
     if (!parentFiber.child) {
       parentFiber.child = fiber;

src/component/fiber.ts

@@ -44,7 +44,6 @@ export class Fiber {
 
   scope: any;
   vars: any;
-  props: any;
 
   component: Component<any, any>;
   vnode: VNode | null = null;
@@ -56,11 +55,10 @@ export class Fiber {
 
   error?: Error;
 
-  constructor(parent: Fiber | null, component: Component<any, any>, props, scope, vars, force) {
+  constructor(parent: Fiber | null, component: Component<any, any>, scope, vars, force) {
     this.force = force;
     this.scope = scope;
     this.vars = vars;
-    this.props = props;
     this.component = component;
 
     this.root = parent ? parent.root : this;

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

@@ -1036,7 +1036,7 @@ exports[`random stuff/miscellaneous t-on with handler bound to dynamic argument
     var h = this.h;
     let c1 = [], p1 = {key:1};
     var vn1 = h('div', p1, c1);
-    var _2 = context['props'].items;
+    var _2 = context['items'];
     if (!_2) { throw new Error('QWeb error: Invalid loop expression')}
     var _3 = _4 = _2;
     if (!(_2 instanceof Array)) {

tests/component/component.test.ts

@@ -72,9 +72,9 @@ class WidgetA extends Widget {
 //------------------------------------------------------------------------------
 
 describe("basic widget properties", () => {
-  test("props is properly defined", async () => {
+  test("props is not defined on root components", async () => {
     const widget = new Widget();
-    expect(widget.props).toEqual({});
+    expect(widget.props).toBe(undefined);
   });
 
   test("has no el after creation", async () => {
@@ -1645,10 +1645,11 @@ describe("props evaluation ", () => {
     class Parent extends Widget {
       static components = { child: Child };
       get greetings() {
-        return `hello ${this.props.name}`;
+        const name = "aaron";
+        return `hello ${name}`;
       }
     }
-    const widget = new Parent(undefined, { name: "aaron" });
+    const widget = new Parent();
     await widget.mount(fixture);
     expect(fixture.innerHTML).toBe("<div><span>hello aaron</span></div>");
   });
@@ -2336,30 +2337,27 @@ describe("random stuff/miscellaneous", () => {
 
   test("t-on with handler bound to dynamic argument on a t-foreach", async () => {
     expect.assertions(3);
-    env.qweb.addTemplates(`
-      <templates>
-        <div t-name="ParentWidget">
-          <t t-foreach="props.items" t-as="item">
-            <Child t-key="item" t-on-ev="onEv(item)"/>
-          </t>
-        </div>
-      </templates>
-    `);
-    const items = [1, 2, 3, 4];
 
     class Child extends Widget {}
     class ParentWidget extends Widget {
+      static template = xml`
+        <div>
+          <t t-foreach="items" t-as="item">
+            <Child t-key="item" t-on-ev="onEv(item)"/>
+          </t>
+        </div>`;
       static components = { Child };
+      items = [1, 2, 3, 4];
       onEv(n, ev) {
         expect(n).toBe(1);
         expect(ev.detail).toBe(43);
       }
     }
 
-    const widget = new ParentWidget(undefined, { items });
+    const widget = new ParentWidget();
     await widget.mount(fixture);
     children(widget)[0].trigger("ev", 43);
-    expect(env.qweb.templates.ParentWidget.fn.toString()).toMatchSnapshot();
+    expect(env.qweb.templates[ParentWidget.template].fn.toString()).toMatchSnapshot();
   });
 
   test("updating widget immediately", async () => {

tests/component/props_validation.test.ts

@@ -37,17 +37,32 @@ describe("props validation", () => {
       static props = ["message"];
       static template = xml`<div>hey</div>`;
     }
+    class Parent extends Widget {
+      static components = { TestWidget };
+      static template = xml`<div><TestWidget /></div>`;
+    }
 
+    let error;
     QWeb.dev = true;
-    expect(() => {
-      new TestWidget();
-    }).toThrow();
+    try {
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeDefined();
+    expect(error.message).toBe(`Missing props 'message' (component 'TestWidget')`);
+
+    error = undefined;
 
     QWeb.dev = false;
-
-    expect(() => {
-      new TestWidget();
-    }).not.toThrow();
+    try {
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeUndefined();
   });
 
   test("props: list of strings", async () => {
@@ -55,10 +70,20 @@ describe("props validation", () => {
       static props = ["message"];
       static template = xml`<div>hey</div>`;
     }
+    class Parent extends Widget {
+      static components = { TestWidget };
+      static template = xml`<div><TestWidget /></div>`;
+    }
 
-    expect(() => {
-      new TestWidget();
-    }).toThrow("Missing props 'message' (component 'TestWidget')");
+    let error;
+    try {
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeDefined();
+    expect(error.message).toBe(`Missing props 'message' (component 'TestWidget')`);
   });
 
   test("validate simple types", async () => {
@@ -71,23 +96,50 @@ describe("props validation", () => {
       { type: Function, ok: () => {}, ko: "1" }
     ];
 
+    let props;
+    class Parent extends Component<any, any> {
+      static template = xml`<div><TestWidget p="p"/></div>`;
+      get p() {
+        return props.p;
+      }
+    }
     for (let test of Tests) {
       let TestWidget = class extends Widget {
         static template = xml`<div>hey</div>`;
         static props = { p: test.type };
       };
+      Parent.components = { TestWidget };
 
-      expect(() => {
-        new TestWidget();
-      }).toThrow("Missing props 'p'");
+      let error;
+      props = {};
+      try {
+        const p = new Parent();
+        await p.mount(fixture);
+      } catch (e) {
+        error = e;
+      }
+      expect(error).toBeDefined();
+      expect(error.message).toBe(`Missing props 'p' (component '_a')`);
 
-      expect(() => {
-        new TestWidget(undefined, { p: test.ok });
-      }).not.toThrow();
+      error = undefined;
+      props = {p: test.ok};
+      try {
+        const p = new Parent();
+        await p.mount(fixture);
+      } catch (e) {
+        error = e;
+      }
+      expect(error).toBeUndefined();
 
-      expect(() => {
-        new TestWidget(undefined, { p: test.ko });
-      }).toThrow("Props 'p' of invalid type in component");
+      props = {p: test.ko};
+      try {
+        const p = new Parent();
+        await p.mount(fixture);
+      } catch (e) {
+        error = e;
+      }
+      expect(error).toBeDefined();
+      expect(error.message).toBe(`Props 'p' of invalid type in component '_a'`);
     }
   });
 
@@ -101,119 +153,305 @@ describe("props validation", () => {
       { type: Function, ok: () => {}, ko: "1" }
     ];
 
+    let props;
+    class Parent extends Component<any, any> {
+      static template = xml`<div><TestWidget p="p"/></div>`;
+      get p() {
+        return props.p;
+      }
+    }
     for (let test of Tests) {
-      let TestWidget = class extends Widget {
-        static template = xml`<div>hey</div>`;
+      let TestWidget = class extends Component<any, any> {
         static props = { p: { type: test.type } };
+        static template = xml`<div>hey</div>`;
       };
+      Parent.components = { TestWidget };
 
-      expect(() => {
-        new TestWidget();
-      }).toThrow("Missing props 'p'");
+      let error;
+      props = {};
+      try {
+        const p = new Parent();
+        await p.mount(fixture);
+      } catch (e) {
+        error = e;
+      }
+      expect(error).toBeDefined();
+      expect(error.message).toBe(`Missing props 'p' (component '_a')`);
 
-      expect(() => {
-        new TestWidget(undefined, { p: test.ok });
-      }).not.toThrow();
+      error = undefined;
+      props = {p: test.ok};
+      try {
+        const p = new Parent();
+        await p.mount(fixture);
+      } catch (e) {
+        error = e;
+      }
+      expect(error).toBeUndefined();
 
-      expect(() => {
-        new TestWidget(undefined, { p: test.ko });
-      }).toThrow("Props 'p' of invalid type in component");
+      props = {p: test.ko};
+      try {
+        const p = new Parent();
+        await p.mount(fixture);
+      } catch (e) {
+        error = e;
+      }
+      expect(error).toBeDefined();
+      expect(error.message).toBe(`Props 'p' of invalid type in component '_a'`);
     }
   });
 
   test("can validate a prop with multiple types", async () => {
-    let TestWidget = class extends Widget {
+    class TestWidget extends Component<any, any> {
       static template = xml`<div>hey</div>`;
       static props = { p: [String, Boolean] };
     };
+    class Parent extends Component<any, any> {
+      static template = xml`<div><TestWidget p="p"/></div>`;
+      static components = { TestWidget };
+      get p() {
+        return props.p;
+      }
+    };
 
-    expect(() => {
-      new TestWidget(undefined, { p: "string" });
-      new TestWidget(undefined, { p: true });
-    }).not.toThrow();
+    let error;
+    let props;
+    try {
+      props = { p: "string" };
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeUndefined();
 
-    expect(() => {
-      new TestWidget(undefined, { p: 1 });
-    }).toThrow("Props 'p' of invalid type in component");
+    try {
+      props = { p: true };
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeUndefined();
+
+    try {
+      props = { p: 1 };
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeDefined();
+    expect(error.message).toBe(`Props 'p' of invalid type in component 'TestWidget'`);
   });
 
   test("can validate an optional props", async () => {
-    let TestWidget = class extends Widget {
+    class TestWidget extends Component<any, any> {
       static template = xml`<div>hey</div>`;
       static props = { p: { type: String, optional: true } };
     };
+    class Parent extends Component<any, any> {
+      static template = xml`<div><TestWidget p="p"/></div>`;
+      static components = { TestWidget };
+      get p() {
+        return props.p;
+      }
+    };
 
-    expect(() => {
-      new TestWidget(undefined, { p: "hey" });
-      new TestWidget(undefined, {});
-    }).not.toThrow();
+    let error;
+    let props;
+    try {
+      props = { p: "key" };
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeUndefined();
 
-    expect(() => {
-      new TestWidget(undefined, { p: 1 });
-    }).toThrow();
+    try {
+      props = {};
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeUndefined();
+
+    try {
+      props = { p: 1 };
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeDefined();
+    expect(error.message).toBe(`Props 'p' of invalid type in component 'TestWidget'`);
   });
 
   test("can validate an array with given primitive type", async () => {
-    let TestWidget = class extends Widget {
+    class TestWidget extends Component<any, any> {
       static template = xml`<div>hey</div>`;
       static props = { p: { type: Array, element: String } };
     };
+    class Parent extends Component<any, any> {
+      static template = xml`<div><TestWidget p="p"/></div>`;
+      static components = { TestWidget };
+      get p() {
+        return props.p;
+      }
+    };
 
-    expect(() => {
-      new TestWidget(undefined, { p: [] });
-      new TestWidget(undefined, { p: ["string"] });
-    }).not.toThrow();
+    let error;
+    let props;
+    try {
+      props = { p: [] };
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeUndefined();
 
-    expect(() => {
-      new TestWidget(undefined, { p: [1] });
-    }).toThrow();
+    try {
+      props = { p: ["string"] };
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeUndefined();
 
-    expect(() => {
-      new TestWidget(undefined, { p: ["string", 1] });
-    }).toThrow();
+    try {
+      props = { p: [1] };
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeDefined();
+
+    error = undefined;
+    try {
+      props = { p: ["string", 1] };
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
   });
 
   test("can validate an array with multiple sub element types", async () => {
-    let TestWidget = class extends Widget {
+    class TestWidget extends Component<any, any> {
       static template = xml`<div>hey</div>`;
       static props = { p: { type: Array, element: [String, Boolean] } };
     };
+    class Parent extends Component<any, any> {
+      static template = xml`<div><TestWidget p="p"/></div>`;
+      static components = { TestWidget };
+      get p() {
+        return props.p;
+      }
+    }
 
-    expect(() => {
-      new TestWidget(undefined, { p: [] });
-      new TestWidget(undefined, { p: ["string"] });
-      new TestWidget(undefined, { p: [false, true, "string"] });
-    }).not.toThrow();
+    let error;
+    let props;
+    try {
+      props = { p: [] };
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeUndefined();
 
-    expect(() => {
-      new TestWidget(undefined, { p: [true, 1] });
-    }).toThrow();
+    try {
+      props = { p: ["string"] };
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeUndefined();
+
+    try {
+      props = { p: [false, true, "string"] };
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeUndefined();
+
+    try {
+      props = { p: [true, 1] };
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeDefined();
+    expect(error.message).toBe(`Props 'p' of invalid type in component 'TestWidget'`);
   });
 
   test("can validate an object with simple shape", async () => {
-    let TestWidget = class extends Widget {
+    class TestWidget extends Component<any, any> {
       static template = xml`<div>hey</div>`;
       static props = {
         p: { type: Object, shape: { id: Number, url: String } }
       };
     };
+    class Parent extends Component<any, any> {
+      static template = xml`<div><TestWidget p="p"/></div>`;
+      static components = { TestWidget };
+      get p() {
+        return props.p;
+      }
+    }
 
-    expect(() => {
-      new TestWidget(undefined, { p: { id: 1, url: "url" } });
-      new TestWidget(undefined, { p: { id: 1, url: "url", extra: true } });
-    }).not.toThrow();
+    let error;
+    let props;
+    try {
+      props = { p: { id: 1, url: "url" } };
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeUndefined();
 
-    expect(() => {
-      new TestWidget(undefined, { p: { id: "1", url: "url" } });
-    }).toThrow();
+    try {
+      props = { p: { id: 1, url: "url", extra: true } };
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeUndefined();
 
-    expect(() => {
-      new TestWidget(undefined, { p: { id: 1 } });
-    }).toThrow();
+    try {
+      props = { p: { id: "1", url: "url" } };
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeDefined();
+    expect(error.message).toBe(`Props 'p' of invalid type in component 'TestWidget'`);
+
+    error = undefined;
+    try {
+      props = { p: { id: 1 } };
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeDefined();
+    expect(error.message).toBe(`Props 'p' of invalid type in component 'TestWidget'`);
   });
 
   test("can validate recursively complicated prop def", async () => {
-    let TestWidget = class extends Widget {
+    class TestWidget extends Component<any, any> {
       static template = xml`<div>hey</div>`;
       static props = {
         p: {
@@ -225,15 +463,43 @@ describe("props validation", () => {
         }
       };
     };
+    class Parent extends Component<any, any> {
+      static template = xml`<div><TestWidget p="p"/></div>`;
+      static components = { TestWidget };
+      get p() {
+        return props.p;
+      }
+    }
 
-    expect(() => {
-      new TestWidget(undefined, { p: { id: 1, url: true } });
-      new TestWidget(undefined, { p: { id: 1, url: [12] } });
-    }).not.toThrow();
+    let error;
+    let props;
+    try {
+      props = { p: { id: 1, url: true } };
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeUndefined();
 
-    expect(() => {
-      new TestWidget(undefined, { p: { id: 1, url: [12, true] } });
-    }).toThrow();
+    try {
+      props = { p: { id: 1, url: [12] } };
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeUndefined();
+
+    try {
+      props = { p: { id: 1, url: [12, true] } };
+      const p = new Parent();
+      await p.mount(fixture);
+    } catch (e) {
+      error = e;
+    }
+    expect(error).toBeDefined();
+    expect(error.message).toBe(`Props 'p' of invalid type in component 'TestWidget'`);
   });
 
   test("props are validated in dev mode (code snapshot)", async () => {
@@ -401,13 +667,18 @@ describe("props validation", () => {
 
 describe("default props", () => {
   test("can set default values", async () => {
-    class TestWidget extends Widget {
+    class TestWidget extends Component<any, any> {
       static defaultProps = { p: 4 };
-      static template = xml`<div>hey</div>`;
+      static template = xml`<div><t t-esc="props.p"/></div>`;
+    }
+    class Parent extends Component<any, any> {
+      static template = xml`<div><TestWidget /></div>`;
+      static components = { TestWidget };
     }
 
-    const w = new TestWidget(undefined, {});
-    expect(w.props.p).toBe(4);
+    const w = new Parent();
+    await w.mount(fixture);
+    expect(fixture.innerHTML).toBe('<div><div>4</div></div>');
   });
 
   test("default values are also set whenever component is updated", async () => {