Review and update prop validation logic for accuracy

packages/connect-react/src/hooks/form-context.tsx

@@ -9,7 +9,10 @@ import type {
 import { useFrontendClient } from "./frontend-client-context";
 import type { ComponentFormProps } from "../components/ComponentForm";
 import type { FormFieldContext } from "./form-field-context";
-import { appPropError } from "./use-app";
+import {
+  appPropErrors, arrayPropErrors, booleanPropErrors, integerPropErrors,
+  stringPropErrors,
+} from "../utils/component";
 
 export type DynamicProps<T extends ConfigurableProps> = { id: string; configurableProps: T; }; // TODO
 
@@ -19,6 +22,7 @@ export type FormContext<T extends ConfigurableProps> = {
   configuredProps: ConfiguredProps<T>;
   dynamicProps?: DynamicProps<T>; // lots of calls require dynamicProps?.id, so need to expose
   dynamicPropsQueryIsFetching?: boolean;
+  errors: Record<string, string[]>;
   fields: Record<string, FormFieldContext<ConfigurableProp>>;
   id: string;
   isValid: boolean;
@@ -168,55 +172,39 @@ export const FormContextProvider = <T extends ConfigurableProps>({
   // so can't rely on that base control form validation
   const propErrors = (prop: ConfigurableProp, value: unknown): string[] => {
     const errs: string[] = [];
-    if (value === undefined) {
-      if (!prop.optional) {
-        errs.push("required");
-      }
-    } else if (prop.type === "integer") { // XXX type should be "number"? we don't support floats otherwise...
-      if (typeof value !== "number") {
-        errs.push("not a number");
-      } else {
-        if (prop.min != null && value < prop.min) {
-          errs.push("number too small");
-        }
-        if (prop.max != null && value > prop.max) {
-          errs.push("number too big");
-        }
-      }
-    } else if (prop.type === "boolean") {
-      if (typeof value !== "boolean") {
-        errs.push("not a boolean");
-      }
-    } else if (prop.type === "string") {
-      type StringProp = ConfigurableProp & {
-        min?: number;
-        max?: number;
-      }
-      const {
-        min = 1, max,
-      } = prop as StringProp;
-      if (typeof value !== "string") {
-        errs.push("not a string");
-      } else {
-        if (value.length < min) {
-          errs.push(`string length must be at least ${min} characters`);
-        }
-        if (max && value.length > max) {
-          errs.push(`string length must not exceed ${max} characters`);
-        }
-      }
-    } else if (prop.type === "app") {
+    if (prop.optional || prop.hidden || prop.disabled) return []
+    if (prop.type === "app") {
       const field = fields[prop.name]
       if (field) {
         const app = field.extra.app
-        const err = appPropError({
+        errs.push(...(appPropErrors({
+          prop,
           value,
           app,
-        })
-        if (err) errs.push(err)
+        }) ?? []))
       } else {
         errs.push("field not registered")
       }
+    } else if (prop.type === "boolean") {
+      errs.push(...(booleanPropErrors({
+        prop,
+        value,
+      }) ?? []))
+    } else if (prop.type === "integer") {
+      errs.push(...(integerPropErrors({
+        prop,
+        value,
+      }) ?? []))
+    } else if (prop.type === "string") {
+      errs.push(...(stringPropErrors({
+        prop,
+        value,
+      }) ?? []))
+    } else if (prop.type === "string[]") {
+      errs.push(...(arrayPropErrors({
+        prop,
+        value,
+      }) ?? []))
     }
     return errs;
   };
@@ -377,6 +365,7 @@ export const FormContextProvider = <T extends ConfigurableProps>({
     configuredProps,
     dynamicProps,
     dynamicPropsQueryIsFetching,
+    errors,
     fields,
     optionalPropIsEnabled,
     optionalPropSetEnabled,

packages/connect-react/src/hooks/use-app.tsx

@@ -1,16 +1,13 @@
 import {
   useQuery, type UseQueryOptions,
 } from "@tanstack/react-query";
+import type { GetAppResponse } from "@pipedream/sdk";
 import { useFrontendClient } from "./frontend-client-context";
-import type {
-  AppRequestResponse, AppResponse, ConfigurablePropApp,
-  PropValue,
-} from "@pipedream/sdk";
 
 /**
  * Get details about an app
  */
-export const useApp = (slug: string, opts?:{ useQueryOpts?: Omit<UseQueryOptions<AppRequestResponse>, "queryKey" | "queryFn">;}) => {
+export const useApp = (slug: string, opts?:{ useQueryOpts?: Omit<UseQueryOptions<GetAppResponse>, "queryKey" | "queryFn">;}) => {
   const client = useFrontendClient();
   const query = useQuery({
     queryKey: [
@@ -26,65 +23,3 @@ export const useApp = (slug: string, opts?:{ useQueryOpts?: Omit<UseQueryOptions
     app: query.data?.data,
   };
 };
-
-type AppResponseWithExtractedCustomFields = AppResponse & {
-  extracted_custom_fields_names: string[]
-}
-
-type AppCustomField = {
-  name: string
-  optional?: boolean
-}
-
-type OauthAppPropValue = PropValue<"app"> & {
-  oauth_access_token?: string
-}
-
-function getCustomFields(app: AppResponse): AppCustomField[] {
-  const isOauth = app.auth_type === "oauth"
-  const userDefinedCustomFields = JSON.parse(app.custom_fields_json || "[]")
-  if ("extracted_custom_fields_names" in app && app.extracted_custom_fields_names) {
-    const extractedCustomFields = ((app as AppResponseWithExtractedCustomFields).extracted_custom_fields_names || []).map(
-      (name) => ({
-        name,
-      }),
-    )
-    userDefinedCustomFields.push(...extractedCustomFields)
-  }
-  return userDefinedCustomFields.map((cf: AppCustomField) => {
-    return {
-      ...cf,
-      // if oauth, treat all as optional (they are usually needed for getting access token)
-      optional: cf.optional || isOauth,
-    }
-  })
-}
-
-export function appPropError(opts: { value: any, app: AppResponse | undefined }): string | undefined {
-  const { app, value } = opts
-  if (!app) {
-    return "app field not registered"
-  }
-  if (!value) {
-    return "no app configured"
-  }
-  if (typeof value !== "object") {
-    return "not an app"
-  }
-  const _value = value as PropValue<"app">
-  if ("authProvisionId" in _value && !_value.authProvisionId) {
-    if (app.auth_type) {
-      if (app.auth_type === "oauth" && !(_value as OauthAppPropValue).oauth_access_token) {
-        return "missing oauth token"
-      }
-      if (app.auth_type === "oauth" || app.auth_type === "keys") {
-        for (const cf of getCustomFields(app)) {
-          if (!cf.optional && !_value[cf.name]) {
-            return "missing custom field"
-          }
-        }
-      }
-      return "no auth provision configured"
-    }
-  }
-}

packages/connect-react/src/utils/component.ts

@@ -0,0 +1,183 @@
+import type {
+  App, ConfigurableProp, ConfigurablePropApp, ConfigurablePropBoolean, ConfigurablePropInteger, ConfigurablePropString, ConfigurablePropStringArray, PropValue,
+} from "@pipedream/sdk";
+
+export type PropOptionValue<T> = {
+  __lv: {
+    value: T
+  }
+}
+
+export function valueFromOption<T>(value: T | PropOptionValue<T>): T | undefined | null {
+  if (typeof value === "object" && value && "__lv" in value) {
+    return (value as PropOptionValue<T>).__lv.value
+  }
+  return value
+}
+
+export type PropOption<T> = {
+  emitValue: T | PropOptionValue<T>
+}
+export type PropOptions<T> = {
+  selectedOptions: PropOption<T>[]
+}
+
+export function valuesFromOptions<T>(value: unknown | T[] | PropOptions<T>): T[] {
+  if (typeof value === "object" && value && "selectedOptions" in value && Array.isArray(value.selectedOptions)) {
+    const selectedOptions = value.selectedOptions as PropOption<T>[]
+    const results = []
+    for (const so of selectedOptions) {
+      if (typeof so === "object" && so && "emitValue" in so) {
+        const emitValue = so.emitValue as T | PropOptionValue<T>
+        if (typeof emitValue === "object" && emitValue && "__lv" in emitValue) {
+          results.push(emitValue.__lv.value)
+        }
+        results.push(emitValue)
+      }
+      throw "unexpected value"
+    }
+  }
+  if (!Array.isArray(value))
+    throw "unexpected value"
+  return value as T[]
+}
+
+export type ValidationOpts<T extends ConfigurableProp> = {
+  prop: T
+  value: unknown
+  app?: App
+}
+
+export function arrayPropErrors(opts: ValidationOpts<ConfigurablePropStringArray>): string[] | undefined {
+  const _values = valuesFromOptions(opts.value)
+  if (!opts.prop.default && typeof _values === "undefined") {
+    return [
+      "required",
+    ]
+  }
+  if (!opts.prop.default && Array.isArray(_values) && !_values.length) return [
+    "empty array",
+  ]
+}
+
+export function booleanPropErrors(opts: ValidationOpts<ConfigurablePropBoolean>): string[] | undefined {
+  const _value = valueFromOption(opts.value)
+  if (_value == null || typeof _value === "undefined") return [
+    "required",
+  ]
+}
+
+export function integerPropErrors(opts: ValidationOpts<ConfigurablePropInteger>): string[] | undefined {
+  const {
+    prop, value: valueOpt,
+  } = opts
+  const value = valueFromOption(valueOpt)
+
+  if (!prop.default && value == null || typeof value === "undefined") return [
+    "required",
+  ]
+
+  const _value: number = typeof value === "number"
+    ? value
+    : parseInt(String(value))
+
+  if (_value !== _value) return [
+    "not a number",
+  ] // NaN
+  const errs = []
+  if (typeof prop.min === "number" && _value < prop.min) errs.push("number too small")
+  if (typeof prop.max === "number" && _value > prop.max) errs.push("number too big")
+  return errs
+}
+
+export function stringPropErrors(opts: ValidationOpts<ConfigurablePropString>): string[] | undefined {
+  const _value = valueFromOption(opts.value)
+
+  if (!opts.prop.default) {
+    if (typeof _value === "undefined" || _value == null) return [
+      "required",
+    ]
+    if (!String(_value).length) return [
+      "string must not be empty",
+    ]
+  }
+}
+
+type AppWithExtractedCustomFields = App & {
+  extracted_custom_fields_names: string[]
+}
+
+type AppCustomField = {
+  name: string
+  optional?: boolean
+}
+
+type OauthAppPropValue = PropValue<"app"> & {
+  oauth_access_token: string
+}
+
+type AppPropValueWithCustomFields<T extends AppCustomField[]> = PropValue<"app"> & {
+  [K in T[number]["name"]]: T[number]
+}
+
+function getCustomFields(app: App): AppCustomField[] {
+  const isOauth = app.auth_type === "oauth"
+  const userDefinedCustomFields = JSON.parse(app.custom_fields_json || "[]")
+  if ("extracted_custom_fields_names" in app && app.extracted_custom_fields_names) {
+    const extractedCustomFields = ((app as AppWithExtractedCustomFields).extracted_custom_fields_names || []).map(
+      (name) => ({
+        name,
+      }),
+    )
+    userDefinedCustomFields.push(...extractedCustomFields)
+  }
+  return userDefinedCustomFields.map((cf: AppCustomField) => {
+    return {
+      ...cf,
+      // if oauth, treat all as optional (they are usually needed for getting access token)
+      optional: cf.optional || isOauth,
+    }
+  })
+}
+
+export function appPropErrors(opts: ValidationOpts<ConfigurablePropApp>): string[] | undefined {
+  const {
+    app, value,
+  } = opts
+  if (!app) {
+    return [
+      "app field not registered",
+    ]
+  }
+  if (!value) {
+    return [
+      "no app configured",
+    ]
+  }
+  if (typeof value !== "object") {
+    return [
+      "not an app",
+    ]
+  }
+  const _value = value as PropValue<"app">
+  if ("authProvisionId" in _value && !_value.authProvisionId) {
+    if (app.auth_type) {
+      const errs = []
+      if (app.auth_type === "oauth" && !(_value as OauthAppPropValue).oauth_access_token) {
+        errs.push("missing oauth token")
+      }
+      if (app.auth_type === "oauth" || app.auth_type === "keys") {
+        const customFields = getCustomFields(app)
+        const _valueWithCustomFields = _value as AppPropValueWithCustomFields<typeof customFields>
+        for (const cf of customFields) {
+          if (!cf.optional && !_valueWithCustomFields[cf.name]) {
+            errs.push(`missing custom field: ${cf.name}`)
+          }
+        }
+      }
+      if (app.auth_type !== "none")
+        errs.push("no auth provision configured")
+      return errs
+    }
+  }
+}