[FIX] component: properly validate optional types in objects

closes #440
This commit is contained in:
Géry Debongnie
2019-11-13 12:45:13 +01:00
parent 263f31fea9
commit 749f0063ea
4 changed files with 63 additions and 15 deletions
+1 -2
View File
@@ -729,7 +729,6 @@ update a number whenever the change is done.
Note: the online playground has an example to show how it works. Note: the online playground has an example to show how it works.
### Semantics ### Semantics
We give here an informal description of the way components are created/updated 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. - 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. - 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: 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) - `props` key is a static key (so, different from `this.props` in a component instance)
+1 -2
View File
@@ -67,7 +67,7 @@ The component system in Owl requires additional directives, to express various
needs. Here is a list of all Owl specific directives: needs. Here is a list of all Owl specific directives:
| Name | Description | | Name | Description |
| ------------------------ | ----------------------------------------------------------------------------------- | | ------------------------ | ------------------------------------------------------------------------------- |
| `t-component`, `t-props` | [Defining a sub component](component.md#composition) | | `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-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-key` | [Defining a key (to help virtual dom reconciliation)](#loops) |
@@ -388,7 +388,6 @@ into the global context.
<!-- new_variable undefined --> <!-- new_variable undefined -->
``` ```
Even though Owl tries to be as declarative as possible, the DOM does not fully 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, 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 the current user selection, the focused element or the state of an input are not
+17 -1
View File
@@ -40,7 +40,13 @@ QWeb.utils.validateProps = function(Widget, props: Object) {
break; 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) { if (!isValid) {
throw new Error(`Props '${propName}' of invalid type in component '${Widget.name}'`); throw new Error(`Props '${propName}' of invalid type in component '${Widget.name}'`);
} }
@@ -80,6 +86,9 @@ function isValidProp(prop, propDef): boolean {
return result; return result;
} }
// propsDef is an object // propsDef is an object
if (propDef.optional && prop === undefined) {
return true;
}
let result = isValidProp(prop, propDef.type); let result = isValidProp(prop, propDef.type);
if (propDef.type === Array) { if (propDef.type === Array) {
for (let i = 0, iLen = prop.length; i < iLen; i++) { for (let i = 0, iLen = prop.length; i < iLen; i++) {
@@ -91,6 +100,13 @@ function isValidProp(prop, propDef): boolean {
for (let key in shape) { for (let key in shape) {
result = result && isValidProp(prop[key], shape[key]); 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; return result;
} }
+36 -2
View File
@@ -240,7 +240,7 @@ describe("props validation", () => {
error = e; error = e;
} }
expect(error).toBeDefined(); 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 () => { test("can validate an optional props", async () => {
@@ -425,7 +425,8 @@ describe("props validation", () => {
} catch (e) { } catch (e) {
error = e; error = e;
} }
expect(error).toBeUndefined(); expect(error).toBeDefined();
expect(error.message).toBe("Invalid prop 'p' in component TestWidget (unknown prop 'extra')");
try { try {
props = { p: { id: "1", url: "url" } }; 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'`); 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<any, any> {
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 () => { test("props are validated in dev mode (code snapshot)", async () => {
env.qweb.addTemplates(` env.qweb.addTemplates(`
<templates> <templates>