From 97b69f164f31235e00e45aacc40141b2c2cefb82 Mon Sep 17 00:00:00 2001
From: Samuel Degueldre <sad@odoo.com>
Date: Tue, 26 Mar 2024 13:52:43 +0100
Subject: [PATCH] [IMP] runtime: allow using any class as a type in props
 validation

Previously, we had a fixed whitelist for types that were allowed during
props validation. The implementation however supports using arbitrary
classes, and in practice it's desirable to do so, and already done when
not using typescript (when using typescript, it will error if the class
is not whitelisted), eg in Odoo, we use "Element" for the arch in the
standard view props, but this causes all view controllers to fail type
checking because it's not whitelisted.

This commit simply replaces existing constructors by a generic
constructor type, and adds a test with a validation success and a test
with a validation failure.
---
 src/runtime/validation.ts                     | 11 +----
 .../props_validation.test.ts.snap             | 39 ++++++++++++++++
 tests/components/props_validation.test.ts     | 46 +++++++++++++++++++
 3 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/src/runtime/validation.ts b/src/runtime/validation.ts
index 36ed9f3ee..78594b6bc 100644
--- a/src/runtime/validation.ts
+++ b/src/runtime/validation.ts
@@ -1,16 +1,7 @@
 import { OwlError } from "../common/owl_error";
 import { toRaw } from "./reactivity";
 
-type BaseType =
-  | typeof String
-  | typeof Boolean
-  | typeof Number
-  | typeof Date
-  | typeof Object
-  | typeof Array
-  | typeof Function
-  | true
-  | "*";
+type BaseType = { new (...args: any[]): any } | true | "*";
 
 interface TypeInfo {
   type?: TypeDescription;
diff --git a/tests/components/__snapshots__/props_validation.test.ts.snap b/tests/components/__snapshots__/props_validation.test.ts.snap
index 910f0e5b7..56351a125 100644
--- a/tests/components/__snapshots__/props_validation.test.ts.snap
+++ b/tests/components/__snapshots__/props_validation.test.ts.snap
@@ -167,6 +167,45 @@ exports[`props validation can specify that additional props are allowed (object)
 }"
 `;
 
+exports[`props validation can use custom class as type 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  const comp1 = app.createComponent(\`Child\`, true, false, false, [\\"customObj\\"]);
+  
+  return function template(ctx, node, key = \\"\\") {
+    const props1 = {customObj: ctx['customObj']};
+    helpers.validateProps(\`Child\`, props1, this);
+    return comp1(props1, key + \`__1\`, node, this, null);
+  }
+}"
+`;
+
+exports[`props validation can use custom class as type 2`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  return function template(ctx, node, key = \\"\\") {
+    return text(ctx['props'].customObj.val);
+  }
+}"
+`;
+
+exports[`props validation can use custom class as type: validation failure 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  const comp1 = app.createComponent(\`Child\`, true, false, false, [\\"customObj\\"]);
+  
+  return function template(ctx, node, key = \\"\\") {
+    const props1 = {customObj: ctx['customObj']};
+    helpers.validateProps(\`Child\`, props1, this);
+    return comp1(props1, key + \`__1\`, node, this, null);
+  }
+}"
+`;
+
 exports[`props validation can validate a prop with multiple types 1`] = `
 "function anonymous(app, bdom, helpers
 ) {
diff --git a/tests/components/props_validation.test.ts b/tests/components/props_validation.test.ts
index f5f744b07..b1b8e81b9 100644
--- a/tests/components/props_validation.test.ts
+++ b/tests/components/props_validation.test.ts
@@ -829,6 +829,52 @@ describe("props validation", () => {
     expect(error!).toBeDefined();
     expect(error!.message).toBe("Invalid props for component 'Child': 'message' is missing");
   });
+
+  test("can use custom class as type", async () => {
+    class CustomClass {
+      val = "hey";
+    }
+    class Child extends Component {
+      static props = { customObj: CustomClass };
+      static template = xml`<t t-esc="props.customObj.val"/>`;
+    }
+
+    class Parent extends Component {
+      static components = { Child };
+      static template = xml`<Child customObj="customObj" />`;
+      customObj = new CustomClass();
+    }
+
+    const app = new App(Parent, { test: true });
+    await app.mount(fixture);
+    expect(fixture.innerHTML).toBe("hey");
+  });
+
+  test("can use custom class as type: validation failure", async () => {
+    class CustomClass {}
+    class Child extends Component {
+      static props = { customObj: CustomClass };
+      static template = xml`<div>hey</div>`;
+    }
+
+    class Parent extends Component {
+      static components = { Child };
+      static template = xml`<Child customObj="customObj" />`;
+      customObj = {};
+    }
+
+    const app = new App(Parent, { test: true });
+    let error: OwlError | undefined;
+    const mountProm = app.mount(fixture).catch((e: Error) => (error = e));
+    await expect(nextAppError(app)).resolves.toThrow(
+      "Invalid props for component 'Child': 'customObj' is not a customclass"
+    );
+    await mountProm;
+    expect(error!).toBeDefined();
+    expect(error!.message).toBe(
+      "Invalid props for component 'Child': 'customObj' is not a customclass"
+    );
+  });
 });
 
 //------------------------------------------------------------------------------