Skip to content

Commit bbcf596

Browse files
authored
Review and update prop validation logic for accuracy (#14986)
* Review and update all prop value validation for accuracy * Relax validation for string prop values that don't match string type
1 parent e0cadd6 commit bbcf596

File tree

6 files changed

+226
-111
lines changed

6 files changed

+226
-111
lines changed

packages/connect-react/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
<!-- markdownlint-disable MD024 -->
22
# Changelog
33

4+
# [1.0.0-preview.11] - 2024-12-13
5+
6+
- Make prop validation more consistent with app behavior
7+
- Relax validation of string props when value is not a string
8+
49
# [1.0.0-preview.10] - 2024-12-12
510

611
- Enforce string length limits

packages/connect-react/examples/nextjs/package-lock.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/connect-react/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@pipedream/connect-react",
3-
"version": "1.0.0-preview.10",
3+
"version": "1.0.0-preview.11",
44
"description": "Pipedream Connect library for React",
55
"files": [
66
"dist"

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

Lines changed: 31 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ import type {
99
import { useFrontendClient } from "./frontend-client-context";
1010
import type { ComponentFormProps } from "../components/ComponentForm";
1111
import type { FormFieldContext } from "./form-field-context";
12-
import { appPropError } from "./use-app";
12+
import {
13+
appPropErrors, arrayPropErrors, booleanPropErrors, integerPropErrors,
14+
stringPropErrors,
15+
} from "../utils/component";
1316

1417
export type DynamicProps<T extends ConfigurableProps> = { id: string; configurableProps: T; }; // TODO
1518

@@ -19,6 +22,7 @@ export type FormContext<T extends ConfigurableProps> = {
1922
configuredProps: ConfiguredProps<T>;
2023
dynamicProps?: DynamicProps<T>; // lots of calls require dynamicProps?.id, so need to expose
2124
dynamicPropsQueryIsFetching?: boolean;
25+
errors: Record<string, string[]>;
2226
fields: Record<string, FormFieldContext<ConfigurableProp>>;
2327
id: string;
2428
isValid: boolean;
@@ -168,55 +172,39 @@ export const FormContextProvider = <T extends ConfigurableProps>({
168172
// so can't rely on that base control form validation
169173
const propErrors = (prop: ConfigurableProp, value: unknown): string[] => {
170174
const errs: string[] = [];
171-
if (value === undefined) {
172-
if (!prop.optional) {
173-
errs.push("required");
174-
}
175-
} else if (prop.type === "integer") { // XXX type should be "number"? we don't support floats otherwise...
176-
if (typeof value !== "number") {
177-
errs.push("not a number");
178-
} else {
179-
if (prop.min != null && value < prop.min) {
180-
errs.push("number too small");
181-
}
182-
if (prop.max != null && value > prop.max) {
183-
errs.push("number too big");
184-
}
185-
}
186-
} else if (prop.type === "boolean") {
187-
if (typeof value !== "boolean") {
188-
errs.push("not a boolean");
189-
}
190-
} else if (prop.type === "string") {
191-
type StringProp = ConfigurableProp & {
192-
min?: number;
193-
max?: number;
194-
}
195-
const {
196-
min = 1, max,
197-
} = prop as StringProp;
198-
if (typeof value !== "string") {
199-
errs.push("not a string");
200-
} else {
201-
if (value.length < min) {
202-
errs.push(`string length must be at least ${min} characters`);
203-
}
204-
if (max && value.length > max) {
205-
errs.push(`string length must not exceed ${max} characters`);
206-
}
207-
}
208-
} else if (prop.type === "app") {
175+
if (prop.optional || prop.hidden || prop.disabled) return []
176+
if (prop.type === "app") {
209177
const field = fields[prop.name]
210178
if (field) {
211179
const app = field.extra.app
212-
const err = appPropError({
180+
errs.push(...(appPropErrors({
181+
prop,
213182
value,
214183
app,
215-
})
216-
if (err) errs.push(err)
184+
}) ?? []))
217185
} else {
218186
errs.push("field not registered")
219187
}
188+
} else if (prop.type === "boolean") {
189+
errs.push(...(booleanPropErrors({
190+
prop,
191+
value,
192+
}) ?? []))
193+
} else if (prop.type === "integer") {
194+
errs.push(...(integerPropErrors({
195+
prop,
196+
value,
197+
}) ?? []))
198+
} else if (prop.type === "string") {
199+
errs.push(...(stringPropErrors({
200+
prop,
201+
value,
202+
}) ?? []))
203+
} else if (prop.type === "string[]") {
204+
errs.push(...(arrayPropErrors({
205+
prop,
206+
value,
207+
}) ?? []))
220208
}
221209
return errs;
222210
};
@@ -377,6 +365,7 @@ export const FormContextProvider = <T extends ConfigurableProps>({
377365
configuredProps,
378366
dynamicProps,
379367
dynamicPropsQueryIsFetching,
368+
errors,
380369
fields,
381370
optionalPropIsEnabled,
382371
optionalPropSetEnabled,
Lines changed: 2 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,13 @@
11
import {
22
useQuery, type UseQueryOptions,
33
} from "@tanstack/react-query";
4+
import type { GetAppResponse } from "@pipedream/sdk";
45
import { useFrontendClient } from "./frontend-client-context";
5-
import type {
6-
AppRequestResponse, AppResponse, ConfigurablePropApp,
7-
PropValue,
8-
} from "@pipedream/sdk";
96

107
/**
118
* Get details about an app
129
*/
13-
export const useApp = (slug: string, opts?:{ useQueryOpts?: Omit<UseQueryOptions<AppRequestResponse>, "queryKey" | "queryFn">;}) => {
10+
export const useApp = (slug: string, opts?:{ useQueryOpts?: Omit<UseQueryOptions<GetAppResponse>, "queryKey" | "queryFn">;}) => {
1411
const client = useFrontendClient();
1512
const query = useQuery({
1613
queryKey: [
@@ -26,65 +23,3 @@ export const useApp = (slug: string, opts?:{ useQueryOpts?: Omit<UseQueryOptions
2623
app: query.data?.data,
2724
};
2825
};
29-
30-
type AppResponseWithExtractedCustomFields = AppResponse & {
31-
extracted_custom_fields_names: string[]
32-
}
33-
34-
type AppCustomField = {
35-
name: string
36-
optional?: boolean
37-
}
38-
39-
type OauthAppPropValue = PropValue<"app"> & {
40-
oauth_access_token?: string
41-
}
42-
43-
function getCustomFields(app: AppResponse): AppCustomField[] {
44-
const isOauth = app.auth_type === "oauth"
45-
const userDefinedCustomFields = JSON.parse(app.custom_fields_json || "[]")
46-
if ("extracted_custom_fields_names" in app && app.extracted_custom_fields_names) {
47-
const extractedCustomFields = ((app as AppResponseWithExtractedCustomFields).extracted_custom_fields_names || []).map(
48-
(name) => ({
49-
name,
50-
}),
51-
)
52-
userDefinedCustomFields.push(...extractedCustomFields)
53-
}
54-
return userDefinedCustomFields.map((cf: AppCustomField) => {
55-
return {
56-
...cf,
57-
// if oauth, treat all as optional (they are usually needed for getting access token)
58-
optional: cf.optional || isOauth,
59-
}
60-
})
61-
}
62-
63-
export function appPropError(opts: { value: any, app: AppResponse | undefined }): string | undefined {
64-
const { app, value } = opts
65-
if (!app) {
66-
return "app field not registered"
67-
}
68-
if (!value) {
69-
return "no app configured"
70-
}
71-
if (typeof value !== "object") {
72-
return "not an app"
73-
}
74-
const _value = value as PropValue<"app">
75-
if ("authProvisionId" in _value && !_value.authProvisionId) {
76-
if (app.auth_type) {
77-
if (app.auth_type === "oauth" && !(_value as OauthAppPropValue).oauth_access_token) {
78-
return "missing oauth token"
79-
}
80-
if (app.auth_type === "oauth" || app.auth_type === "keys") {
81-
for (const cf of getCustomFields(app)) {
82-
if (!cf.optional && !_value[cf.name]) {
83-
return "missing custom field"
84-
}
85-
}
86-
}
87-
return "no auth provision configured"
88-
}
89-
}
90-
}

0 commit comments

Comments
 (0)