Support arrays for JSON Schema types

[andresmgot]

Fixes #1245

[absoludity]

It's not clear why we are adding undefined to the union here? The code change only adds the possibility of it being an array of json schema types (though this isn't tested, i don't think?).

If the reason to add undefined here is so existing tests which don't define it pass, can we instead update the tests? Or why is it added?

[andresmgot]

nope, it's a compilation error when readying the schema that it says that the type may be undefined but we can do the cast to something else there.

[absoludity]

Sure, but it's a compilation error because of something we're doing. We have explicitly defined the type here as either JSONScheme4TypeName or an array of those, not null. Let me test something, the I'll reply inline on your more recent change.

[absoludity]

+1, but please check my suggestion below - I think it makes more sense than explicitly converting an undefined value into one of the valid defined values for a JSONSchemaType.

[absoludity]

So I think I understand now: you are adding this because the type property is a JSON4SchemaType only allows 7 types (as per the spec) yet TypeScript knows that the variable type which we're trying to assign can additionally be undefined. The reason that it can be undefined is simply because the type property of a JSONSchema4 type is optional (and we got the value from an object destructuring assignment). Which leaves me thinking it should similarly be an optional param on IBasicFormParam.

Instead, this change explicitly converts an undefined into one of the expected values (in this case, "null").

Why not update update IBasicFormParam so that the type is optional (and can therefore be undefined in the true sense). For example, this works:

diff --git dashboard/src/shared/schema.ts dashboard/src/shared/schema.ts
index 116703f0..06d3313c 100644
--- dashboard/src/shared/schema.ts
+++ dashboard/src/shared/schema.ts
@@ -26,7 +26,7 @@ export function retrieveBasicFormParams(
     Object.keys(properties).map(propertyKey => {
       // The param path is its parent path + the object key
       const itemPath = `${parentPath || ""}${propertyKey}`;
-      const { type, form } = properties[propertyKey];
+      const { form } = properties[propertyKey];
       // If the property has the key "form", it's a basic parameter
       if (form) {
         // Use the default value either from the JSON schema or the default values
@@ -34,7 +34,6 @@ export function retrieveBasicFormParams(
         const param: IBasicFormParam = {
           ...properties[propertyKey],
           path: itemPath,
-          type: type || "null",
           value,
           children:
             properties[propertyKey].type === "object"
diff --git dashboard/src/shared/types.ts dashboard/src/shared/types.ts
index 8bc46294..dffbfb74 100644
--- dashboard/src/shared/types.ts
+++ dashboard/src/shared/types.ts
@@ -380,7 +380,7 @@ export interface IKubeState {
 
 export interface IBasicFormParam {
   path: string;
-  type: jsonSchema.JSONSchema4TypeName | jsonSchema.JSONSchema4TypeName[];
+  type?: jsonSchema.JSONSchema4TypeName | jsonSchema.JSONSchema4TypeName[];
   value?: any;
   title?: string;
   minimum?: number;

This is similar in result to your original change, except that there you specified that the type property must be specified, but can have the undefined value, where as this just matches the JSONSchema in that it is not required to be specified.

[andresmgot]

right, I am fine with any of those solution. I never thought about the difference between type?: and type: ... | undefined.