From 749f0063eadcc5fc450840e8c7fea55544f275a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Wed, 13 Nov 2019 12:45:13 +0100 Subject: [PATCH] [FIX] component: properly validate optional types in objects closes #440 --- doc/reference/component.md | 3 +- doc/reference/qweb_templating_language.md | 19 ++++++------ src/component/props_validation.ts | 18 ++++++++++- tests/component/props_validation.test.ts | 38 +++++++++++++++++++++-- 4 files changed, 63 insertions(+), 15 deletions(-) diff --git a/doc/reference/component.md b/doc/reference/component.md index da4ae60fb..f212912c9 100644 --- a/doc/reference/component.md +++ b/doc/reference/component.md @@ -729,7 +729,6 @@ update a number whenever the change is done. Note: the online playground has an example to show how it works. - ### Semantics We give here an informal description of the way components are created/updated @@ -824,7 +823,7 @@ As an application becomes complex, it may be quite unsafe to define props in an - 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 parents. -A props type system would solve both issues, by describing the types and shapes +A props type system solves 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) diff --git a/doc/reference/qweb_templating_language.md b/doc/reference/qweb_templating_language.md index 29f141d15..a4709c3b7 100644 --- a/doc/reference/qweb_templating_language.md +++ b/doc/reference/qweb_templating_language.md @@ -66,15 +66,15 @@ For reference, here is a list of all standard QWeb directives: The component system in Owl requires additional directives, to express various needs. Here is a list of all Owl specific directives: -| Name | Description | -| ------------------------ | ----------------------------------------------------------------------------------- | -| `t-component`, `t-props` | [Defining a sub component](component.md#composition) | -| `t-ref` | [Setting a reference to a dom node or a sub component](component.md#references) | -| `t-key` | [Defining a key (to help virtual dom reconciliation)](#loops) | -| `t-on-*` | [Event handling](component.md#event-handling) | -| `t-transition` | [Defining an animation](animations.md#css-transitions) | -| `t-slot` | [Rendering a slot](component.md#slots) | -| `t-model` | [Form input bindings](component.md#form-input-bindings) | +| Name | Description | +| ------------------------ | ------------------------------------------------------------------------------- | +| `t-component`, `t-props` | [Defining a sub component](component.md#composition) | +| `t-ref` | [Setting a reference to a dom node or a sub component](component.md#references) | +| `t-key` | [Defining a key (to help virtual dom reconciliation)](#loops) | +| `t-on-*` | [Event handling](component.md#event-handling) | +| `t-transition` | [Defining an animation](animations.md#css-transitions) | +| `t-slot` | [Rendering a slot](component.md#slots) | +| `t-model` | [Form input bindings](component.md#form-input-bindings) | ## Reference @@ -388,7 +388,6 @@ into the global context. ``` - Even though Owl tries to be as declarative as possible, the DOM does not fully expose its state declaratively in the DOM tree. For example, the scrolling state, the current user selection, the focused element or the state of an input are not diff --git a/src/component/props_validation.ts b/src/component/props_validation.ts index 9973223b7..b83ba13c8 100644 --- a/src/component/props_validation.ts +++ b/src/component/props_validation.ts @@ -40,7 +40,13 @@ QWeb.utils.validateProps = function(Widget, props: Object) { break; } } - let isValid = isValidProp(props[propName], propsDef[propName]); + let isValid; + try { + isValid = isValidProp(props[propName], propsDef[propName]); + } catch (e) { + e.message = `Invalid prop '${propName}' in component ${Widget.name} (${e.message})`; + throw e; + } if (!isValid) { throw new Error(`Props '${propName}' of invalid type in component '${Widget.name}'`); } @@ -80,6 +86,9 @@ function isValidProp(prop, propDef): boolean { return result; } // propsDef is an object + if (propDef.optional && prop === undefined) { + return true; + } let result = isValidProp(prop, propDef.type); if (propDef.type === Array) { for (let i = 0, iLen = prop.length; i < iLen; i++) { @@ -91,6 +100,13 @@ function isValidProp(prop, propDef): boolean { for (let key in shape) { result = result && isValidProp(prop[key], shape[key]); } + if (result) { + for (let propName in prop) { + if (!(propName in shape)) { + throw new Error(`unknown prop '${propName}'`); + } + } + } } return result; } diff --git a/tests/component/props_validation.test.ts b/tests/component/props_validation.test.ts index a9d1ced8e..03753cf14 100644 --- a/tests/component/props_validation.test.ts +++ b/tests/component/props_validation.test.ts @@ -240,7 +240,7 @@ describe("props validation", () => { error = e; } expect(error).toBeDefined(); - expect(error.message).toBe(`Props 'p' of invalid type in component 'TestWidget'`); + expect(error.message).toBe("Props 'p' of invalid type in component 'TestWidget'"); }); test("can validate an optional props", async () => { @@ -425,7 +425,8 @@ describe("props validation", () => { } catch (e) { error = e; } - expect(error).toBeUndefined(); + expect(error).toBeDefined(); + expect(error.message).toBe("Invalid prop 'p' in component TestWidget (unknown prop 'extra')"); try { props = { p: { id: "1", url: "url" } }; @@ -501,6 +502,39 @@ describe("props validation", () => { expect(error.message).toBe(`Props 'p' of invalid type in component 'TestWidget'`); }); + test("can validate optional attributes in nested sub props", () => { + class TestComponent extends Component { + static props = { + myprop: { + type: Array, + element: { + type: Object, + shape: { + num: { type: Number, optional: true } + } + } + } + }; + } + let error; + try { + QWeb.utils.validateProps(TestComponent, { myprop: [{}] }); + } catch (e) { + error = e; + } + expect(error).toBeUndefined(); + + try { + QWeb.utils.validateProps(TestComponent, { myprop: [{ a: 1 }] }); + } catch (e) { + error = e; + } + expect(error).toBeDefined(); + expect(error.message).toBe( + "Invalid prop 'myprop' in component TestComponent (unknown prop 'a')" + ); + }); + test("props are validated in dev mode (code snapshot)", async () => { env.qweb.addTemplates(`