[IMP] props_validation: have clearer error messages

With this commit, props validation error messages will include a more
developer-friendly error message, avoiding the need to investigate in
the Developer Tools why a complex props structure is invalid.

src/component/props_validation.ts

@@ -40,15 +40,16 @@ QWeb.utils.validateProps = function (Widget, props: Object) {
           continue;
         }
       }
-      let isValid;
+      let whyInvalid;
       try {
-        isValid = isValidProp(props[propName], propsDef[propName]);
+        whyInvalid = whyInvalidProp(props[propName], propsDef[propName]);
       } catch (e) {
         e.message = `Invalid prop '${propName}' in component ${Widget.name} (${e.message})`;
         throw e;
       }
-      if (!isValid) {
+      if (whyInvalid !== null) {
-        throw new Error(`Invalid Prop '${propName}' in component '${Widget.name}'`);
+        whyInvalid = whyInvalid.replace(/\${propName}/g, propName);
+        throw new Error(`Invalid Prop '${propName}' in component '${Widget.name}': ${whyInvalid}`);
       }
     }
     for (let propName in props) {
@@ -60,11 +61,11 @@ QWeb.utils.validateProps = function (Widget, props: Object) {
 };
 
 /**
- * Check if an invidual prop value matches its (static) prop definition
+ * Check why an invidual prop value doesn't match its (static) prop definition
  */
-function isValidProp(prop, propDef): boolean {
+function whyInvalidProp(prop, propDef): string | null {
   if (propDef === true) {
-    return true;
+    return null;
   }
   if (typeof propDef === "function") {
     // Check if a value is constructed by some Constructor.  Note that there is a
@@ -73,43 +74,66 @@ function isValidProp(prop, propDef): boolean {
     // 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;
+      if (prop instanceof propDef) {
+        return null;
+      }
+      return `\${propName} is not an instance of ${propDef.name}`;
     }
-    return typeof prop === propDef.name.toLowerCase();
+    if (typeof prop === propDef.name.toLowerCase()) {
+      return null;
+    }
+    return `type of \${propName} is not ${propDef.name}`;
   } 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;
+    let reasons: string[] = [];
     for (let i = 0, iLen = propDef.length; i < iLen; i++) {
-      result = result || isValidProp(prop, propDef[i]);
+      const why = whyInvalidProp(prop, propDef[i]);
+      if (why === null) {
+        return null;
+      }
+      reasons.push(why);
+    }
+    if (reasons.length > 1) {
+      return reasons.slice(0, -1).join(", ") + " and " + reasons[reasons.length - 1];
+    } else {
+      return reasons[0];
     }
-    return result;
   }
   // propsDef is an object
   if (propDef.optional && prop === undefined) {
-    return true;
+    return null;
   }
-  let result = propDef.type ? isValidProp(prop, propDef.type) : true;
+  if (propDef.type) {
-  if (propDef.validate) {
+    const why = whyInvalidProp(prop, propDef.type);
-    result = result && propDef.validate(prop);
+    if (why !== null) {
+      return why;
+    }
+  }
+  if (propDef.validate && !propDef.validate(prop)) {
+    return "${propName} could not be validated by `validate` function";
   }
   if (propDef.type === Array && propDef.element) {
     for (let i = 0, iLen = prop.length; i < iLen; i++) {
-      result = result && isValidProp(prop[i], propDef.element);
+      const why = whyInvalidProp(prop[i], propDef.element);
+      if (why !== null) {
+        return why.replace(/\${propName}/g, `\${propName}[${i}]`);
+      }
     }
   }
   if (propDef.type === Object && propDef.shape) {
     const shape = propDef.shape;
     for (let key in shape) {
-      result = result && isValidProp(prop[key], shape[key]);
+      const why = whyInvalidProp(prop[key], shape[key]);
+      if (why !== null) {
+        return why.replace(/\${propName}/g, `\${propName}['${key}']`);
+      }
     }
-    if (result) {
+    for (let propName in prop) {
-      for (let propName in prop) {
+      if (!(propName in shape)) {
-        if (!(propName in shape)) {
+        return `unknown prop \${propName}['${propName}']`;
-          throw new Error(`unknown prop '${propName}'`);
-        }
       }
     }
   }
-  return result;
+  return null;
 }

tests/component/props_validation.test.ts

@@ -138,7 +138,9 @@ describe("props validation", () => {
         error = e;
       }
       expect(error).toBeDefined();
-      expect(error.message).toBe("Invalid Prop 'p' in component '_a'");
+      expect(error.message).toBe(
+        `Invalid Prop 'p' in component '_a': type of p is not ${test.type.name}`
+      );
     }
   });
 
@@ -195,7 +197,9 @@ describe("props validation", () => {
         error = e;
       }
       expect(error).toBeDefined();
-      expect(error.message).toBe("Invalid Prop 'p' in component '_a'");
+      expect(error.message).toBe(
+        `Invalid Prop 'p' in component '_a': type of p is not ${test.type.name}`
+      );
     }
   });
 
@@ -240,7 +244,9 @@ describe("props validation", () => {
       error = e;
     }
     expect(error).toBeDefined();
-    expect(error.message).toBe("Invalid Prop 'p' in component 'TestWidget'");
+    expect(error.message).toBe(
+      "Invalid Prop 'p' in component 'TestWidget': type of p is not String and type of p is not Boolean"
+    );
   });
 
   test("can validate an optional props", async () => {
@@ -284,7 +290,9 @@ describe("props validation", () => {
       error = e;
     }
     expect(error).toBeDefined();
-    expect(error.message).toBe("Invalid Prop 'p' in component 'TestWidget'");
+    expect(error.message).toBe(
+      "Invalid Prop 'p' in component 'TestWidget': type of p is not String"
+    );
   });
 
   test("can validate an array with given primitive type", async () => {
@@ -389,7 +397,9 @@ describe("props validation", () => {
       error = e;
     }
     expect(error).toBeDefined();
-    expect(error.message).toBe("Invalid Prop 'p' in component 'TestWidget'");
+    expect(error.message).toBe(
+      "Invalid Prop 'p' in component 'TestWidget': type of p[1] is not String and type of p[1] is not Boolean"
+    );
   });
 
   test("can validate an object with simple shape", async () => {
@@ -426,7 +436,9 @@ describe("props validation", () => {
       error = e;
     }
     expect(error).toBeDefined();
-    expect(error.message).toBe("Invalid prop 'p' in component TestWidget (unknown prop 'extra')");
+    expect(error.message).toBe(
+      "Invalid Prop 'p' in component 'TestWidget': unknown prop p['extra']"
+    );
 
     try {
       props = { p: { id: "1", url: "url" } };
@@ -436,7 +448,9 @@ describe("props validation", () => {
       error = e;
     }
     expect(error).toBeDefined();
-    expect(error.message).toBe("Invalid Prop 'p' in component 'TestWidget'");
+    expect(error.message).toBe(
+      "Invalid Prop 'p' in component 'TestWidget': type of p['id'] is not Number"
+    );
 
     error = undefined;
     try {
@@ -447,7 +461,9 @@ describe("props validation", () => {
       error = e;
     }
     expect(error).toBeDefined();
-    expect(error.message).toBe("Invalid Prop 'p' in component 'TestWidget'");
+    expect(error.message).toBe(
+      "Invalid Prop 'p' in component 'TestWidget': type of p['url'] is not String"
+    );
   });
 
   test("can validate recursively complicated prop def", async () => {
@@ -499,7 +515,9 @@ describe("props validation", () => {
       error = e;
     }
     expect(error).toBeDefined();
-    expect(error.message).toBe("Invalid Prop 'p' in component 'TestWidget'");
+    expect(error.message).toBe(
+      "Invalid Prop 'p' in component 'TestWidget': p['url'] is not an instance of Boolean and type of p['url'][1] is not Number"
+    );
   });
 
   test("can validate optional attributes in nested sub props", () => {
@@ -531,7 +549,7 @@ describe("props validation", () => {
     }
     expect(error).toBeDefined();
     expect(error.message).toBe(
-      "Invalid prop 'myprop' in component TestComponent (unknown prop 'a')"
+      "Invalid Prop 'myprop' in component 'TestComponent': unknown prop myprop[0]['a']"
     );
   });
 
@@ -557,7 +575,9 @@ describe("props validation", () => {
       error = e;
     }
     expect(error).toBeDefined();
-    expect(error.message).toBe("Invalid Prop 'size' in component 'TestComponent'");
+    expect(error.message).toBe(
+      "Invalid Prop 'size' in component 'TestComponent': size could not be validated by `validate` function"
+    );
   });
 
   test("can validate with a custom validator, and a type", () => {
@@ -585,7 +605,9 @@ describe("props validation", () => {
       error = e;
     }
     expect(error).toBeDefined();
-    expect(error.message).toBe("Invalid Prop 'n' in component 'TestComponent'");
+    expect(error.message).toBe(
+      "Invalid Prop 'n' in component 'TestComponent': type of n is not Number"
+    );
     expect(validator).toBeCalledTimes(1);
 
     error = null;
@@ -595,7 +617,9 @@ describe("props validation", () => {
       error = e;
     }
     expect(error).toBeDefined();
-    expect(error.message).toBe("Invalid Prop 'n' in component 'TestComponent'");
+    expect(error.message).toBe(
+      "Invalid Prop 'n' in component 'TestComponent': n could not be validated by `validate` function"
+    );
     expect(validator).toBeCalledTimes(2);
   });
 

tests/misc/portal.test.ts

@@ -78,7 +78,9 @@ describe("Portal: Props validation", () => {
       error = e;
     }
     expect(error).toBeDefined();
-    expect(error.message).toBe(`Invalid Prop 'target' in component 'Portal'`);
+    expect(error.message).toBe(
+      "Invalid Prop 'target' in component 'Portal': target is not an instance of String"
+    );
 
     QWeb.dev = dev;
   });