[IMP] component: add props validation and default props system

closes #116

doc/component.md

@@ -7,9 +7,11 @@
 - [Composition](#composition)
 - [Reference](#reference)
   - [Properties](#properties)
+  - [Static Properties](#static-properties)
   - [Methods](#methods)
   - [Lifecycle](#lifecycle)
   - [Semantics](#semantics)
+  - [Props Validation](#props-validation)
 - [Asynchronous rendering](#asynchronous-rendering)
 
 ## Overview
@@ -144,11 +146,22 @@ find a template with the component name (or one of its ancestor).
 - **`props`** (Object): this is an object given (in the constructor) by the parent
   to configure the component. It can be dynamically changed later by the parent,
   in some case. Note that `props` are owned by the parent, not by the component.
-  As such, it should not ever be modified by the component.
+  As such, it should not ever be modified by the component!!
 
 - **`refs`** (Object): the `refs` object contains all references to sub DOM nodes
   or sub widgets defined by a `t-ref` directive in the component's template.
 
+### Static Properties
+
+- **`props`** (Object, optional): if given, this is an object that describes the
+  type and shape of the (actual) props given to the component. If Owl mode is
+  `dev`, this will be used to validate the props each time the component is
+  created/updated. See [Props Validation](#props-validation) for more information.
+- **`defaultProps`** (Object, optional): if given, this object define default
+  values for (top-level) props. Whenever `props` are given to the object, they
+  will be altered to add default value (if missing). Note that it does not
+  change the initial object, a new object will be created instead.
+
 ### Methods
 
 - **`mount(target)`** (async): this is the main way a component's hierarchy is added to the
@@ -432,6 +445,93 @@ Here is what Owl will do:
 
 6. `patched` hooks are called on `D`, `C`
 
+### Props Validation
+
+As an application becomes complex, it may be quite unsafe to define props in an informal way. This leads to two issues:
+
+- hard to tell how a component should be used, by looking at its code.
+- unsafe, it is easy to send wrong props into a component, either by refactoring a component, or one of its parent.
+
+A props type system would solve both issues, by describing the types and shapes
+of the props. Here is how it works in Owl:
+
+- `props` key is a static key (so, different from `this.props` in a component instance)
+- it is optional: it is ok for a component to not define a `props` key.
+- props are validated whenever a component is created/updated
+- props are only validated in `dev` mode (see [tooling page](tooling.md#development-mode))
+- if a key does not match the description, an error is thrown
+- it only validates keys defined in (static) `props`. Additional keys in (component) `props` are not validated.
+
+For example:
+
+```js
+class ComponentA extends owl.Component {
+    static props = ['id', 'url'];
+
+    ...
+}
+
+class ComponentB extends owl.Component {
+  static props = {
+    count: {type: Number},
+    messages: {
+      type: Array,
+      element: {type: Object, shape: {id: Boolean, text: 'string' }
+    },
+   date: Date,
+   combinedVal: [Number, Boolean]
+  };
+
+  ...
+}
+```
+
+- it is an object or a list of strings
+- a list of strings is a simplified props definition, which only lists the name
+  of the props.
+- all props are by default required, unless they are defined with `optional: true`
+  (in that case, validation is only done if there is a value)
+- valid types are: `Number, String, Boolean, Object, Array, Date, Function`, and all
+  constructor functions (so, if you have a `Person` class, it can be used as a type)
+- arrays are homogeneous (all elements have the same type/shape)
+
+For each key, a `prop` definition is either a constructor, a list of constructors, or an object:
+
+- a constructor: this should describe the type, for example: `id: Number` describe
+  the props `id` as a number
+- a list of constructors. In that case, this means that we allow more than one
+  type. For example, `id: [Number, String]` means that `id` can be either a string
+  or a number.
+- an object. This makes it possible to have more expressive definition. The following sub keys are then allowed:
+  - `type`: the main type of the prop being validated
+  - `element`: if the type was `Array`, then the `element` key describes the type of each element in the array. It is optional (not set means that we only validate the array, not its elements),
+  - `shape`: if the type was `Object`, then the `shape` key describes the interface of the object. It is optional (not set means that we only validate the object, not its elements)
+
+Examples:
+
+```js
+  // only the existence of those 3 keys is documented
+  static props = ['message', 'id', 'date'];
+```
+
+```js
+  static props = {
+    messageIds: {type: Array, element: Number},  // list of number
+    otherArr: {type: Array},   // just array. no validation is made on sub elements
+    otherArr2: Array,   // same as otherArr
+    someObj: {type: Object},  // just an object, no internal validation
+    someObj2: {
+      type: Object,
+      shape: {
+        id: Number,
+        name: {type: String, optional: true},
+        url: String
+      ]},    // object, with keys id (number), name (string, optional) and url (string)
+    someFlag: Boolean,     // a boolean, mandatory (even if `false`)
+    someVal: [Boolean, Date]   // either a boolean or a date
+  };
+```
+
 ## Asynchronous rendering
 
 Working with asynchronous code always adds a lot of complexity to a system. Whenever

src/component.ts

@@ -68,6 +68,11 @@ export class Component<
   env: T;
   state?: State;
   props: Props;
+
+  // type of props is not easily representable in typescript...
+  static props?: any;
+  static defaultProps?: any;
+
   refs: {
     [key: string]: Component<T, any, any> | HTMLElement | undefined;
   } = {};
@@ -98,6 +103,13 @@ export class Component<
   constructor(parent: Component<T, any, any> | T, props?: Props) {
     super();
 
+    const defaultProps = (<any>this.constructor).defaultProps;
+    if (defaultProps) {
+      props = this._applyDefaultProps(props, defaultProps);
+    }
+    if (QWeb.dev) {
+      this._validateProps(props || {});
+    }
     // is this a good idea?
     //   Pro: if props is empty, we can create easily a widget
     //   Con: this is not really safe
@@ -359,6 +371,13 @@ export class Component<
   ): Promise<void> {
     const shouldUpdate = forceUpdate || this.shouldUpdate(nextProps);
     if (shouldUpdate) {
+      const defaultProps = (<any>this.constructor).defaultProps;
+      if (defaultProps) {
+        nextProps = this._applyDefaultProps(nextProps, defaultProps);
+      }
+      if (QWeb.dev) {
+        this._validateProps(nextProps);
+      }
       await this.willUpdateProps(nextProps);
       this.props = nextProps;
       await this.render(forceUpdate, patchQueue);
@@ -454,16 +473,16 @@ export class Component<
    */
   _mount(vnode: VNode, elm: HTMLElement): VNode {
     this.__owl__.vnode = patch(elm, vnode);
-    if (
+    if (this.__owl__.parent!.__owl__.isMounted && !this.__owl__.isMounted) {
-      this.__owl__.parent!.__owl__.isMounted &&
-      !this.__owl__.isMounted
-    ) {
       this._callMounted();
     }
     return this.__owl__.vnode;
   }
 
-  __mount() {
+  /**
+   * Only called by qweb t-widget directive (when t-keepalive is set)
+   */
+  _remount() {
     if (!this.__owl__.isMounted) {
       this.__owl__.isMounted = true;
       this.mounted();
@@ -477,4 +496,103 @@ export class Component<
       this.__owl__.observer.notifyCB = this.render.bind(this);
     }
   }
+
+  /**
+   * Apply default props (only top level).
+   *
+   * Note that this method does not modify in place the props, it returns a new
+   * prop object
+   */
+  _applyDefaultProps(props: Object | undefined, defaultProps: Object): Props {
+    props = props ? Object.create(props) : {};
+    for (let propName in defaultProps) {
+      if (props![propName] === undefined) {
+        props![propName] = defaultProps[propName];
+      }
+    }
+    return <Props>props;
+  }
+
+  /**
+   * Validate the component props (or next props) against the (static) props
+   * description.  This is potentially an expensive operation: it may needs to
+   * visit recursively the props and all the children to check if they are valid.
+   * This is why it is only done in 'dev' mode.
+   */
+  _validateProps(props: Object) {
+    const propsDef = (<any>this.constructor).props;
+    if (propsDef instanceof Array) {
+      // list of strings (prop names)
+      for (let i = 0, l = propsDef.length; i < l; i++) {
+        if (!(propsDef[i] in props)) {
+          throw new Error(
+            `Missing props '${propsDef[i]}' (widget '${this.constructor.name}')`
+          );
+        }
+      }
+    } else if (propsDef) {
+      // propsDef is an object now
+      for (let propName in propsDef) {
+        if (!(propName in props)) {
+          if (propsDef[propName] && !propsDef[propName].optional) {
+            throw new Error(
+              `Missing props '${propName}' (widget '${this.constructor.name}')`
+            );
+          } else {
+            break;
+          }
+        }
+        let isValid = isValidProp(props[propName], propsDef[propName]);
+        if (!isValid) {
+          throw new Error(
+            `Props '${propName}' of invalid type in widget '${
+              this.constructor.name
+            }'`
+          );
+        }
+      }
+    }
+  }
+}
+
+//------------------------------------------------------------------------------
+// Prop validation helper
+//------------------------------------------------------------------------------
+
+/**
+ * Check if an invidual prop value matches its (static) prop definition
+ */
+function isValidProp(prop, propDef): boolean {
+  if (typeof propDef === "function") {
+    // Check if a value is constructed by some Constructor.  Note that there is a
+    // slight abuse of language: we want to consider primitive values as well.
+    //
+    // So, even though 1 is not an instance of Number, we want to consider that
+    // it is valid.
+    if (typeof prop === "object") {
+      return prop instanceof propDef;
+    }
+    return typeof prop === propDef.name.toLowerCase();
+  } else if (propDef instanceof Array) {
+    // If this code is executed, this means that we want to check if a prop
+    // matches at least one of its descriptor.
+    let result = false;
+    for (let i = 0; i < propDef.length; i++) {
+      result = result || isValidProp(prop, propDef[i]);
+    }
+    return result;
+  }
+  // propsDef is an object
+  let result = isValidProp(prop, propDef.type);
+  if (propDef.type === Array) {
+    for (let i = 0; i < prop.length; i++) {
+      result = result && isValidProp(prop[i], propDef.element);
+    }
+  }
+  if (propDef.type === Object) {
+    for (let key in propDef.shape) {
+      result = result && isValidProp(prop[key], propDef.shape[key]);
+    }
+  }
+  return result;
 }

src/qweb_extensions.ts

@@ -490,7 +490,7 @@ QWeb.addDirective({
     );
     let keepAliveCode = "";
     if (keepAlive) {
-      keepAliveCode = `pvnode.data.hook.insert = vn => {vn.elm.parentNode.replaceChild(w${widgetID}.el,vn.elm);vn.elm=w${widgetID}.el;w${widgetID}.__mount();};`;
+      keepAliveCode = `pvnode.data.hook.insert = vn => {vn.elm.parentNode.replaceChild(w${widgetID}.el,vn.elm);vn.elm=w${widgetID}.el;w${widgetID}._remount();};`;
     }
     ctx.addLine(
       `def${defID} = def${defID}.then(()=>{if (w${widgetID}.__owl__.isDestroyed) {return};${

tests/props_validation.test.ts

@@ -0,0 +1,263 @@
+import { Component, Env } from "../src/component";
+import { makeTestFixture, makeTestEnv } from "./helpers";
+import { QWeb } from "../src";
+
+//------------------------------------------------------------------------------
+// Setup and helpers
+//------------------------------------------------------------------------------
+
+let fixture: HTMLElement;
+let env: Env;
+let dev: boolean = false;
+
+beforeEach(() => {
+  fixture = makeTestFixture();
+  env = makeTestEnv();
+  dev = QWeb.dev;
+  QWeb.dev = true;
+});
+
+afterEach(() => {
+  fixture.remove();
+  QWeb.dev = dev;
+});
+
+class Widget extends Component<any, any, any> {}
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+describe("props validation", () => {
+  test("validation is only done in dev mode", async () => {
+    class TestWidget extends Widget {
+      static props = ["message"];
+    }
+
+    QWeb.dev = true;
+    expect(() => {
+      new TestWidget(env);
+    }).toThrow();
+
+    QWeb.dev = false;
+
+    expect(() => {
+      new TestWidget(env);
+    }).not.toThrow();
+  });
+
+  test("props validation is also done on update props", async () => {
+    expect.assertions(1);
+    class TestWidget extends Widget {
+      static props = ["message"];
+    }
+    const w = new TestWidget(env, { message: "bottle" });
+    try {
+      await w._updateProps({});
+    } catch (e) {
+      expect(e.message).toBe("Missing props 'message' (widget 'TestWidget')");
+    }
+  });
+
+  test("props: list of strings", async () => {
+    class TestWidget extends Widget {
+      static props = ["message"];
+    }
+
+    expect(() => {
+      new TestWidget(env);
+    }).toThrow("Missing props 'message' (widget 'TestWidget')");
+  });
+
+  test("validate simple types", async () => {
+    const Tests = [
+      { type: Number, ok: 1, ko: "1" },
+      { type: Boolean, ok: true, ko: "1" },
+      { type: String, ok: "1", ko: 1 },
+      { type: Object, ok: {}, ko: "1" },
+      { type: Date, ok: new Date(), ko: "1" },
+      { type: Function, ok: () => {}, ko: "1" }
+    ];
+
+    for (let test of Tests) {
+      let TestWidget = class extends Widget {
+        static props = { p: test.type };
+      };
+
+      expect(() => {
+        new TestWidget(env);
+      }).toThrow("Missing props 'p'");
+
+      expect(() => {
+        new TestWidget(env, { p: test.ok });
+      }).not.toThrow();
+
+      expect(() => {
+        new TestWidget(env, { p: test.ko });
+      }).toThrow("Props 'p' of invalid type in widget");
+    }
+  });
+
+  test("validate simple types, alternate form", async () => {
+    const Tests = [
+      { type: Number, ok: 1, ko: "1" },
+      { type: Boolean, ok: true, ko: "1" },
+      { type: String, ok: "1", ko: 1 },
+      { type: Object, ok: {}, ko: "1" },
+      { type: Date, ok: new Date(), ko: "1" },
+      { type: Function, ok: () => {}, ko: "1" }
+    ];
+
+    for (let test of Tests) {
+      let TestWidget = class extends Widget {
+        static props = { p: { type: test.type } };
+      };
+
+      expect(() => {
+        new TestWidget(env);
+      }).toThrow("Missing props 'p'");
+
+      expect(() => {
+        new TestWidget(env, { p: test.ok });
+      }).not.toThrow();
+
+      expect(() => {
+        new TestWidget(env, { p: test.ko });
+      }).toThrow("Props 'p' of invalid type in widget");
+    }
+  });
+
+
+  test("can validate a prop with multiple types", async () => {
+    let TestWidget = class extends Widget {
+      static props = { p: [String, Boolean] };
+    };
+
+    expect(() => {
+      new TestWidget(env, { p: "string" });
+      new TestWidget(env, { p: true });
+    }).not.toThrow();
+
+    expect(() => {
+      new TestWidget(env, { p: 1 });
+    }).toThrow("Props 'p' of invalid type in widget");
+  });
+
+  test("can validate an optional props", async () => {
+    let TestWidget = class extends Widget {
+      static props = { p: { type: String, optional: true } };
+    };
+
+    expect(() => {
+      new TestWidget(env, { p: "hey" });
+      new TestWidget(env, { });
+    }).not.toThrow();
+
+    expect(() => {
+      new TestWidget(env, { p: 1 });
+    }).toThrow();
+
+  });
+
+
+  test("can validate an array with given primitive type", async () => {
+    let TestWidget = class extends Widget {
+      static props = { p: { type: Array, element: String } };
+    };
+
+    expect(() => {
+      new TestWidget(env, { p: [] });
+      new TestWidget(env, { p: ["string"] });
+    }).not.toThrow();
+
+    expect(() => {
+      new TestWidget(env, { p: [1] });
+    }).toThrow();
+
+    expect(() => {
+      new TestWidget(env, { p: ["string", 1] });
+    }).toThrow();
+  });
+
+  test("can validate an array with multiple sub element types", async () => {
+    let TestWidget = class extends Widget {
+      static props = { p: { type: Array, element: [String, Boolean] } };
+    };
+
+    expect(() => {
+      new TestWidget(env, { p: [] });
+      new TestWidget(env, { p: ["string"] });
+      new TestWidget(env, { p: [false, true, "string"] });
+    }).not.toThrow();
+
+    expect(() => {
+      new TestWidget(env, { p: [true, 1] });
+    }).toThrow();
+  });
+
+  test("can validate an object with simple shape", async () => {
+    let TestWidget = class extends Widget {
+      static props = {
+        p: { type: Object, shape: { id: Number, url: String } }
+      };
+    };
+
+    expect(() => {
+      new TestWidget(env, { p: { id: 1, url: "url" } });
+      new TestWidget(env, { p: { id: 1, url: "url", extra: true } });
+    }).not.toThrow();
+
+    expect(() => {
+      new TestWidget(env, { p: { id: "1", url: "url" } });
+    }).toThrow();
+
+    expect(() => {
+      new TestWidget(env, { p: { id: 1 } });
+    }).toThrow();
+  });
+
+  test("can validate recursively complicated prop def", async () => {
+    let TestWidget = class extends Widget {
+      static props = {
+        p: {
+          type: Object,
+          shape: {
+            id: Number,
+            url: [Boolean, {type: Array, element: Number}],
+          }
+        }
+      };
+    };
+
+    expect(() => {
+      new TestWidget(env, { p: { id: 1, url: true } });
+      new TestWidget(env, { p: { id: 1, url: [12]} });
+    }).not.toThrow();
+
+    expect(() => {
+      new TestWidget(env, { p: { id: 1, url: [12, true]} });
+    }).toThrow();
+
+  });
+});
+
+
+describe("default props", () => {
+  test("can set default values", async () => {
+    class TestWidget extends Widget {
+      static defaultProps = {p: 4}
+    }
+
+    const w = new TestWidget(env, {});
+    expect(w.props.p).toBe(4);
+  });
+
+  test("default values are also set whenever component is updated", async () => {
+    class TestWidget extends Widget {
+      static defaultProps = {p: 4}
+    }
+
+    const w = new TestWidget(env, {p: 1});
+    await w._updateProps({});
+    expect(w.props.p).toBe(4);
+  });
+});