Fix build issues in connect-react
#18187
Conversation
* Configure Vite to abort if there are compile-time errors * Use explicit types from the Connect SDK (i.e. `@pipedream/sdk`) to make the v2 migration easier * Fix pre-existing compile-time errors
jverce
added
enhancement
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughStandardizes select option shapes to LabelValueOption, broadens react-select customization generics, tightens form/field generics, migrates remote options to configureComponent with sanitized paging, introduces structured Observation/DynamicProps types, adds connect-account flow, fails build on TypeScript diagnostics, and removes the Next.js example. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant ControlApp
participant AccountsAPI as client.getAccounts
participant Connector as client.connectAccount
participant State as React State
User->>ControlApp: open/select account
ControlApp->>AccountsAPI: fetch accounts (external_user_id)
AccountsAPI-->>ControlApp: accounts list
ControlApp->>State: set options = [accounts..., "Connect new ..."]
User->>ControlApp: select option
alt selects existing account
ControlApp->>State: set authProvisionId
else selects "Connect new"
ControlApp->>Connector: startConnectAccount()
Connector-->>ControlApp: success (provision id)
ControlApp->>AccountsAPI: refetch accounts
AccountsAPI-->>ControlApp: updated accounts
ControlApp->>State: set authProvisionId to new id
else clears selection
ControlApp->>State: set authProvisionId = undefined
end
sequenceDiagram
autonumber
participant RemoteOptions as RemoteOptionsContainer
participant SDK as client.configureComponent
participant UI as ControlSelect
RemoteOptions->>SDK: configureComponent(input, context)
SDK-->>RemoteOptions: { data: RawPropOption<string>[], observations }
RemoteOptions->>RemoteOptions: dedupe values (Set)
RemoteOptions->>RemoteOptions: map sanitizeOption -> LabelValueOption[]
RemoteOptions-->>UI: options (LabelValueOption[])
opt load more
UI->>RemoteOptions: request next page
RemoteOptions->>SDK: configureComponent(next context)
SDK-->>RemoteOptions: next data
RemoteOptions->>UI: append sanitized options
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes build issues and prepares the connect-react package for SDK v2 migration by adding type safety and error handling. It configures Vite to fail on TypeScript errors, uses explicit SDK types for easier migration, and cleans up unused utility functions.
- Configure Vite to abort on TypeScript compilation errors
- Add explicit type annotations using current SDK types to facilitate v2 migration
- Fix pre-existing compile-time errors and clean up unused utilities
Reviewed Changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config.mts | Configure build to fail on TypeScript errors and fix quote consistency |
| tsconfig.json | Update module setting for better compatibility |
| src/types.ts | Add comprehensive type definitions for observations, options, and SDK responses |
| src/utils/type-guards.ts | Simplify and fix type guard functions, remove unused utilities |
| src/utils/component.ts | Fix type assertion for __lv property access |
| src/hooks/*.tsx | Update to use explicit SDK types and fix type errors |
| src/components/*.tsx | Fix component typing issues and improve type safety |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/connect-react/src/utils/component.ts (1)
170-187: Ensure validation covers both missing and falsyauthProvisionIdThe current check only runs validation when the
authProvisionIdproperty exists but is falsy, so any truly missing property slips through unchecked. We should normalize “absent or falsy” into one condition:• File:
packages/connect-react/src/utils/component.ts
Lines: 170–187Apply this diff:
- if ("authProvisionId" in _value && !_value.authProvisionId) { + const hasProvisionId = "authProvisionId" in _value && Boolean((_value as any).authProvisionId) + if (!hasProvisionId) { 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}`) } } } return errs }This change flags both omitted and empty
authProvisionIdconsistently.packages/connect-react/src/components/Errors.tsx (1)
3-5: Fix SDK import resolution and remove deprecated “/browser” imports
Dependency check
packages/connect-react/package.jsonalready declares@pipedream/sdkas a workspace dependency—no further changes needed there.Type-only import in Errors.tsx
Inpackages/connect-react/src/components/Errors.tsx, update the SDK imports to be type-only to prevent Vite from bundling runtime code:-import { - ConfigurableProp, ConfigurablePropAlert, ConfigurableProps, -} from "@pipedream/sdk"; +import type { + ConfigurableProp, ConfigurablePropAlert, ConfigurableProps, +} from "@pipedream/sdk";Remove or replace all
@pipedream/sdk/browserimports
The following files currently import from the internal/browserentrypoint, which breaks resolution. Replace each one with either:
- A type-only import from the public SDK package (
import type { … } from "@pipedream/sdk"), or- A proper runtime import from
"@pipedream/sdk"if you need to pull in executable code.•
packages/connect-react/src/types.ts:4
•packages/connect-react/src/hooks/use-accounts.tsx:6
•packages/connect-react/src/hooks/form-field-context.tsx:7
•packages/connect-react/src/hooks/frontend-client-context.tsx:7
•packages/connect-react/src/components/ControlApp.tsx:15
•packages/connect-react/src/components/RemoteOptionsContainer.tsx:1
•packages/connect-react/src/components/ControlSubmit.tsx:4
•packages/connect-react/README.md:77, 119
•packages/connect-react/examples/nextjs/src/app/page.tsx:4Ensure that all type-only imports use the
import type { … }syntax so Vite/tree-shaking doesn’t bundle unintended code, and that any runtime APIs come from the correct public module.packages/connect-react/src/components/Control.tsx (1)
4-6: Use type-only import for SDK typesImporting types from
@pipedream/sdkas a value import pulls in the entire runtime SDK and increases bundle size. Please switch to a type-only import to ensure these are erased at compile time and tree-shaken from the final build. We’ve confirmed that@pipedream/sdkis already declared inpackages/connect-react/package.json.• File to update:
packages/connect-react/src/components/Control.tsx• Verified dependency:
@pipedream/sdkappears on line 31 ofpackages/connect-react/package.json.Suggested diff:
-import { - ConfigurableProp, - ConfigurableProps, -} from "@pipedream/sdk"; +import type { ConfigurableProp, ConfigurableProps } from "@pipedream/sdk";packages/connect-react/src/hooks/form-context.tsx (1)
433-441: Avoid mutating React state in place in registerField.setFields mutates the existing object and returns the same reference, which can skip re-renders and cause subtle bugs.
Return a new object:
const registerField = <T extends ConfigurableProp>(field: FormFieldContext<T>) => { - setFields((fields) => { - fields[field.prop.name] = field - return fields - }); + setFields((prev) => ({ + ...prev, + [field.prop.name]: field, + })); checkPropsNeedConfiguring() };
♻️ Duplicate comments (5)
packages/connect-react/src/components/InternalField.tsx (1)
31-33: Remove @ts-ignore and type ‘suspense’ correctly on useQueryOpts.Suppressing with @ts-ignore undermines the “abort on compile errors” goal and can hide real regressions. Let’s assert the exact option shape (keeping runtime identical) and drop the ignore.
Option A — extract and validate with satisfies (preferred; TS ≥ 4.9):
// above the useApp() call const queryOpts = { enabled: !!appSlug, ...(appSlug ? { suspense: true } : {}), } satisfies (typeof formCtx)['useQueryOpts'] & { suspense?: boolean }; // ... const { app } = useApp(appSlug || "", { useQueryOpts: queryOpts });Option B — inline assertion (if you’d rather not introduce a local):
- useQueryOpts: { - enabled: !!appSlug, - // this seems to work (this overrides enabled so don't just set to true) - // @ts-ignore - suspense: !!appSlug, - }, + useQueryOpts: { + enabled: !!appSlug, + // enable suspense only when we actually have an appSlug + suspense: !!appSlug, + } as (typeof formCtx)['useQueryOpts'] & { suspense: boolean },Note: “suspense overrides enabled” is not generally accurate with TanStack Query; enabled still gates fetching. If the intent is “only use suspense when enabled would be true,” Option A models that explicitly.
If TS complains about the indexed access type, confirm that formCtx exposes useQueryOpts at the type level or switch to the hook’s option type:
NonNullable<Parameters<typeof useApp>[1]>['useQueryOpts'].packages/connect-react/src/components/ControlApp.tsx (1)
96-99: Remove @ts-ignore by allowing suspense in use-accounts opts.This duplicates an earlier review comment. After updating the opts type in use-accounts to include suspense, you can drop the ts-ignore here.
- // this seems to work (this overrides enabled so don't just set to true) - // @ts-ignore - suspense: !!app, + // this seems to work (this overrides enabled so don't just set to true) + suspense: !!app,packages/connect-react/src/components/Errors.tsx (1)
32-39: LGTM: carry prop.name into alerts for contextIncluding
name: prop.nameon eachConfigurablePropAlerthelps downstream renderers/logging. Based on the SDK types,ConfigurablePropAlertextends the base prop withname, so this is type-safe.packages/connect-react/src/hooks/form-context.tsx (1)
501-509: Type error details instead of using 'any'.You can avoid the any by typing the error detail payload, aligning with your public types. This also addresses a previously raised concern.
- const errorFromObservationErrorDetails = (item: ObservationErrorDetails, ret: SdkError[]) => { + const errorFromObservationErrorDetails = (item: ObservationErrorDetails, ret: SdkError[]) => { const err: SdkError = { name: item.name, message: item.message } if (err.name && err.message) ret.push(err) } @@ - const errorFromDetails = (data: any, ret: SdkError[]) => { - ret.push({ name: data.error, message: JSON.stringify(data.details) }) - } + interface ErrorDetailsData { error: string; details: unknown } + const errorFromDetails = (data: ErrorDetailsData, ret: SdkError[]) => { + if (data?.error) { + ret.push({ name: data.error, message: JSON.stringify(data.details) }) + } + }Also applies to: 532-543
packages/connect-react/src/hooks/customization-context.tsx (1)
18-18: Fix TS2307: Cannot find '@pipedream/sdk' types (same root cause).This file also imports ConfigurableProp from @pipedream/sdk. Apply the same workspace dependency or tsconfig “paths” mapping fix suggested in form-context.tsx.
#!/bin/bash rg -nP --type=ts "\bfrom\s+['\"]@pipedream/sdk['\"]" -C1 packages/connect-react/src
🧹 Nitpick comments (39)
packages/connect-react/src/utils/component.ts (5)
59-67: Dead branch:valuesFromOptionsnever returnsundefined
valuesFromOptionsalways returns an array, sotypeof _values === "undefined"is unreachable. Either (a) adjustvaluesFromOptionsto returnundefinedfor nullish inputs, or (b) simplify this check to rely on array length only. Given the PR’s “non-behavioral” intent, I recommend (b).- if (!opts.prop.default && typeof _values === "undefined") { - return [ - "required", - ] - } - if (!opts.prop.default && Array.isArray(_values) && !_values.length) return [ + if (!opts.prop.default && Array.isArray(_values) && _values.length === 0) return [ "empty array", ]
70-74: Simplify null/undefined check
_value == nullalready coversnull | undefined. The extratypeof _value === "undefined"is redundant.- if (_value == null || typeof _value === "undefined") return [ + if (_value == null) return [ "required", ]
86-97: Parse integers in base 10 and returnundefinedwhen no errors
- Use radix 10 with
parseIntto avoid legacy octal quirks.- Return
undefinedwhen there are no errors to match the function’s declared return type shape and be consistent with the other validators.- const _value: number = typeof value === "number" - ? value - : parseInt(String(value)) + const _value: number = typeof value === "number" + ? value + : parseInt(String(value), 10) if (Number.isNaN(_value)) return [ "not a number", ] 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 + return errs.length ? errs : undefined
99-107: Unwrap option values for consistency with other validatorsFor string props, use
valueFromOptionto keep behavior consistent with boolean/integer validators and to support wrapped values.const { prop, value: valueOpt, } = opts - if (!prop.default && (valueOpt == null || typeof valueOpt === "undefined")) return [ + const _value = valueFromOption(valueOpt) + if (!prop.default && (_value == null)) return [ "required", ]
126-144: Harden custom fields parsingIf
custom_fields_jsonis invalid JSON,JSON.parsewill throw and abort validation. Wrap in try/catch and fall back to an empty array.- const userDefinedCustomFields = JSON.parse(app.custom_fields_json || "[]") + let userDefinedCustomFields: AppCustomField[] = [] + try { + userDefinedCustomFields = JSON.parse(app.custom_fields_json || "[]") + } catch { + userDefinedCustomFields = [] + }packages/connect-react/src/components/InternalField.tsx (1)
48-49: Ensure FormFieldContext.extra is typed to include ‘app’ (and allow undefined).You’re now always setting
extra: { app }. IfappSlugis falsy,appwill beundefined. ConfirmFormFieldContext<T>['extra']permits this shape; otherwise consumers may see type friction.Two options:
- Make
appoptional at the type level for app fields, or- Only add the key when available.
Minimal runtime-neutral tweak:
- extra: { - app, - }, + extra: app ? { app } : {},If you prefer keeping the key present, update the type to
extra: { app?: ReturnType<typeof useApp>['app'] }(or a narrower SDK type) so consumers don’t need non-null assertions.packages/connect-react/vite.config.mts (2)
34-36: Consider deduping react and react-dom as well.This avoids duplicate React copies in consumer bundles in some setups.
- dedupe: [ - "@emotion/react", - ], + dedupe: [ + "react", + "react-dom", + "@emotion/react", + ],
21-26: UMD globals: react/jsx-runtime usually shouldn't be a global.For UMD, react/jsx-runtime is typically not referenced directly at runtime. You can omit it from globals and external, or map it to React if you keep it externalized.
output: { globals: { - "react": "React", - "react-dom": "ReactDOM", - "react/jsx-runtime": "react/jsx-runtime", - "@emotion/react": "@emotion/react", + react: "React", + "react-dom": "ReactDOM", + "@emotion/react": "emotionReact", }, },packages/connect-react/src/hooks/use-accounts.tsx (1)
21-28: Minor: stabilize the queryKey to reduce unnecessary refetches.Using the raw input object as a key can cause cache misses on identity changes. Prefer a stable tuple of primitive fields.
- const query = useQuery<GetAccountsResponse>({ + const { external_user_id, app: appSlug, oauth_app_id } = input; + const query = useQuery<GetAccountsResponse>({ ...opts?.useQueryOpts, - queryKey: [ - "accounts", - input, - ], - queryFn: () => client.getAccounts(input), + queryKey: ["accounts", external_user_id, appSlug, oauth_app_id], + queryFn: () => client.getAccounts({ external_user_id, app: appSlug, oauth_app_id }), });packages/connect-react/src/components/ControlApp.tsx (4)
28-33: Use a discriminant to prevent id collisions.If an Account with id "_new" ever existed, it would conflict. Add a discriminant to the placeholder.
-type AccountPlaceholder = { - id: "_new"; - name: string; -} +type AccountPlaceholder = { + kind: "placeholder"; + id: "_new"; + name: string; +}And adjust the usage checks to also narrow on kind where applicable.
66-79: Prop typing for react-select is on the right track; make Select generic to tighten onChange.Without generics, react-select’s onChange param often widens to unknown/any. Set the component generic to SelectValue.
- <Select + <Select<SelectValue, false> instanceId={id} value={selectValue} options={selectOptions} {...selectProps} - required={true} placeholder={`Select ${app.name} account...`}Also, react-select doesn’t honor HTML required; prefer aria-required for a11y if needed:
+ aria-required="true"
124-128: Include newAccountPlaceholder in deps to keep label in sync.If app.name changes, the memoized options would keep an old label.
- ], [accounts]); + ], [accounts, newAccountPlaceholder]);
129-142: Defaulting to the “Connect new …” option is a product choice; just flagging UX.Selecting a default “connect new” may surprise users who have existing accounts. Consider leaving value undefined so the placeholder text shows until a user picks an account.
- const selectValue = useMemo<SelectValue>(() => { + const selectValue = useMemo<SelectValue | null>(() => { if (value?.authProvisionId) { for (const item of accounts) { if (value.authProvisionId === item.id) { return item; } } } - return newAccountPlaceholder; + return null; }, [ accounts, value, ]);packages/connect-react/src/types.ts (1)
17-21: Make stack optional for cross-environment compatibility.Error.stack can be undefined in some runtimes; making it optional prevents needless narrowing or non-null assertions.
export type ObservationErrorDetails = { name: string; message: string; - stack: string; + stack?: string; }packages/connect-react/src/components/Errors.tsx (1)
46-48: Prefer a stable key over the array indexUsing
idxas the key may cause unnecessary re-renders. Use a deterministic key from the alert itself.- return <>{formattedErrors.map((fe, idx: number) => <Alert prop={fe} key={idx}/>)}</> + return <>{formattedErrors.map((fe) => ( + <Alert prop={fe} key={`${fe.name}:${fe.content}`} /> + ))}</>packages/connect-react/src/hooks/form-field-context.tsx (1)
9-12: Use a stricter empty type for the non-App branch
{}is very permissive. If the intent is “no extra fields”, preferRecord<never, never>to maintain a true empty shape.-export type FormFieldContextExtra<T extends ConfigurableProp> = T extends ConfigurablePropApp ? { - app?: App; -} : {}; +export type FormFieldContextExtra<T extends ConfigurableProp> = + T extends ConfigurablePropApp + ? { app?: App } + : Record<never, never>;packages/connect-react/src/utils/type-guards.ts (2)
71-76: Null/undefined handling: avoid fabricating a string value of type TReturning
value: "" as Tlies to the type system whenTisn’tstring. Preferundefinedto reflect “no value”.if (option === null || option === undefined) { return { label: "", - value: "" as T, + value: undefined as unknown as T, }; }Please confirm all call sites tolerate
value: undefinedfor the “empty” case. If not, we can feature-flag this change or keep the current string fallback for now.
101-104: Prefer JSON stringification for object labels
String(option)on objects yields “[object Object]”, which is not user-friendly.- return { - label: String(option), - value: option, - }; + return { + label: (() => { + try { return JSON.stringify(option); } catch { return String(option); } + })(), + value: option, + };packages/connect-react/src/components/RemoteOptionsContainer.tsx (3)
52-63: Type newOptions to avoid “any” and drop the cast laterGive
newOptionsan explicit type and remove the downstreamas RawPropOption<string>[].- ] = useState<{ + ] = useState<{ page: number; prevContext: ConfigureComponentContext | undefined; data: RawPropOption<string>[]; values: Set<string | number>; }>({ @@ - const newOptions = [] + const newOptions: RawPropOption<string>[] = [] @@ - data = [ + data = [ ...pageable.data, ...newOptions, - ] as RawPropOption<string>[] + ]Also applies to: 145-171
214-219: Avoid stale refetches on input changeCalling
refetch()immediately aftersetQuery(v)can refetch with the previous query value and then again when the key updates. Either rely on the query key change alone or debounce. Minimal change: remove the manualrefetch()and let the key drive fetching.onInputChange(v) { if (prop.useQuery) { setQuery(v); - refetch(); } },Alternatively, debounce:
// outside JSX const [localQuery, setLocalQuery] = useState(""); useEffect(() => { const id = setTimeout(() => setQuery(localQuery), 250); return () => clearTimeout(id); }, [localQuery]); // in selectProps onInputChange(v) { if (prop.useQuery) setLocalQuery(v); }
139-143: Style: tighten stringOptions mappingMinor readability tweak.
- if (stringOptions?.length) { - const options = []; - for (const stringOption of stringOptions) { - options.push({ - label: stringOption, - value: stringOption, - }); - } - _options = options; - } + if (stringOptions?.length) { + _options = stringOptions.map((s) => ({ label: s, value: s })); + }packages/connect-react/src/components/Control.tsx (1)
71-73: Guard the non-null assertion on app
app!assumesextra.appis always present whenprop.type === "app". If upstream context fails to set it, this will throw later. Prefer an explicit guard.- case "app": - return <ControlApp app={app!} />; + case "app": + if (!app) { + throw new Error("Missing App in FormFieldContext.extra for 'app' prop"); + } + return <ControlApp app={app} />;Please confirm the provider guarantees
extra.appforConfigurablePropAppfields.packages/connect-react/src/components/ComponentFormContainer.tsx (2)
12-16: DRY the XOR type across componentsThe same externalUserId | userId XOR also exists in ComponentFormProps. To avoid drift, export a shared type alias (e.g., UserIdentityProps) from a central file (src/types.ts) and reuse it here and in ComponentForm.
-} & ( - // Either externalUserId or userId must be provided - | { externalUserId: string; userId?: never } - | { userId: string; externalUserId?: never } - ); +} & UserIdentityProps;
43-45: Avoid forwarding componentKey to ComponentFormcomponentKey isn’t part of ComponentFormProps and gets forwarded via the spread. While harmless at runtime, it weakens the prop boundary and may confuse consumers or trigger excess-prop checks.
- return <ComponentForm component={component} {...props} />; + const { componentKey, ...rest } = props; + return <ComponentForm component={component} {...rest} />;packages/connect-react/src/components/Field.tsx (2)
1-1: Type-only import and tighten FormContext genericUse a type-only import for FormContext to avoid emitting value imports, and align the generic to the readonly nature of ConfigurableProps to better match the SDK’s typing (Readonly<ConfigurableProp[]>).
-import { FormContext } from "../hooks/form-context"; +import type { FormContext } from "../hooks/form-context"; @@ -export type FieldProps<T extends ConfigurableProp> = { - form: FormContext<T[]>; +export type FieldProps<T extends ConfigurableProp> = { + form: FormContext<Readonly<T[]>>;Also applies to: 9-9
62-62: Downcast in getProps third arg is a smellCasting to FieldProps drops the specific T and can hide type issues. Prefer passing a minimal, untyped bag for customization or widen the getProps third-arg type to unknown.
- <div {...getProps("field", baseStyles, props as FieldProps<ConfigurableProp>)}> + <div {...getProps("field", baseStyles, { form, field })}>If getProps relies on the full props shape, consider overloading it to accept unknown and let consumers narrow.
packages/connect-react/src/components/ComponentForm.tsx (1)
39-41: Share the XOR identity type with the container to prevent driftSame suggestion as in ComponentFormContainer: factor the union into a single exported UserIdentityProps and reuse here.
-} & ( - | { externalUserId: string; userId?: never } - | { userId: string; externalUserId?: never } - ); +} & UserIdentityProps;packages/connect-react/src/components/ControlSubmit.tsx (1)
43-47: Casting to ControlSubmitProps is unnecessarygetProps doesn’t need a precise third-arg type; the cast adds noise without safety. Prefer passing a minimal object or keep props uncast.
- baseStyles(!!propsNeedConfiguring.length || submitting), - props as ControlSubmitProps<ConfigurableProps> + baseStyles(!!propsNeedConfiguring.length || submitting), + { form }packages/connect-react/src/components/ControlSelect.tsx (2)
147-169: Creation path typing: avoid forcing inputValue to TinputValue is string; forcing it to T risks mis-typing when T is non-string. Let sanitizeOption infer string and widen only at the boundary.
- const handleCreate = (inputValue: string) => { - const newOption = sanitizeOption(inputValue as T) + const handleCreate = (inputValue: string) => { + const newOption = sanitizeOption<string>(inputValue) let newRawValue: LabelValueOption<T> | LabelValueOption<T>[] = newOptionIf strict typing is needed, consider a second generic constrained to string values for creatable flows.
79-115: selectValue normalization logic is sound but can be simplifiedThis block is correct but dense. Consider extracting to a small pure helper normalizeSelectValue(rawValue, selectOptions) to improve readability and testability.
packages/connect-react/src/hooks/form-context.tsx (5)
189-197: Narrow result type from reloadComponentProps and centralize error extraction.Casting to ReloadComponentPropsResponse is fine, but you immediately branch on observations/errors. Consider early-returns to cut duplication and ensure only one error source wins.
- const { dynamicProps, observations, errors: __errors } = result as ReloadComponentPropsResponse; - // Prioritize errors from observations over the errors array - if (observations && observations.filter((o) => o.k === "error").length > 0) { - handleSdkErrors(observations) - } else { - handleSdkErrors(__errors) - } + const { dynamicProps, observations, errors: rawErrors } = result as ReloadComponentPropsResponse; + const hasObsErrors = Array.isArray(observations) && observations.some((o) => o.k === "error"); + handleSdkErrors(hasObsErrors ? observations : rawErrors);Simplifies the branch and avoids traversing observations twice.
221-240: Minimize casts and keep prop filtering strictly typed.The temporary cast to typeof props is unnecessary after the ConfigurableProps memo typing. Also, the slice branch doesn’t need Array.isArray since props is always an array.
- props = _configurableProps as typeof props; // XXX + props = _configurableProps; @@ - if (reloadPropIdx != null) { - props = Array.isArray(props) - ? props.slice(0, reloadPropIdx + 1) // eslint-disable-line react/prop-types - : props; // XXX - } + if (reloadPropIdx != null) { + props = props.slice(0, reloadPropIdx + 1); + }
341-346: sdkResponse side-effect: ensure type stability.handleSdkErrors accepts multiple shapes, but the Form props’ sdkResponse type likely isn’t ConfiguredProps. Passing a plain configured-props object here will clear errors (the final else branch). If sdkResponse actually represents SDK error envelopes, retype the function parameter accordingly.
- const handleSdkErrors = (sdkResponse?: ConfiguredProps<T> | Observation[] | string[]) => { + type SdkEnvelope = + | { observations: Observation[] } + | { os: Observation[] } + | Observation[] + | string[] + | Error; + const handleSdkErrors = (sdkResponse?: SdkEnvelope) => {This matches the branches you already support and avoids accidental “error reset” when an object without error shape is passed.
545-560: Harden HTTP error parsing and observation routing.Parsing Error.message as JSON is brittle. You already guard with try/catch; consider also checking for payload instanceof Error and preserving message when JSON parsing fails. Additionally, both os and observations branches delegate to the same extractor—good consolidation.
- const errorFromHttpError = (payload: Error, ret: SdkError[]) => { + const errorFromHttpError = (payload: unknown, ret: SdkError[]) => { try { - const data = JSON.parse(payload.message)?.data + const msg = payload instanceof Error ? payload.message : String(payload); + const data = JSON.parse(msg)?.data if (data && "os" in data) { errorFromObservations(data.os, ret) } else if (data && "observations" in data) { errorFromObservations(data.observations, ret) } else if (data && "error" in data && "details" in data) { - errorFromDetails(data, ret) + errorFromDetails(data as { error: string; details: unknown }, ret) } } catch { - // pass + // fallback: keep raw message + ret.push({ name: "SDKError", message: payload instanceof Error ? payload.message : String(payload) }) } }Also applies to: 562-579
178-210: React Query usage: consider returning meaningful data or switching to mutation.queryFn returns [] and you rely on side effects to update state. Since you gate with enabled: reloadPropIdx != null, this is acceptable, but a mutation (useMutation) could better represent “reload props” semantics and avoid empty query data.
If staying on useQuery, return the dynamicProps id or a boolean to make cache/state transitions more explicit.
packages/connect-react/src/hooks/customization-context.tsx (4)
144-170: Generic select customization API looks correct; consider naming clarity for ReactSelectComponents.The new generic signatures for getClassNames/getComponents/getStyles/getProps align with react-select types. However, you have both:
- a value import: components as ReactSelectComponents
- a type alias: export type ReactSelectComponents = { … }
While legal (type vs value namespaces), it’s easy to confuse. Consider renaming the type alias to SelectComponentKeys or ReactSelectControlComponents.
-export type ReactSelectComponents = { +export type ReactSelectControlComponents = { controlAppSelect: typeof ControlApp; controlSelect: typeof ControlSelect; }; @@ -export type CustomComponents<Option, IsMulti extends boolean, Group extends GroupBase<Option>> = { - [K in keyof typeof defaultComponents]: typeof defaultComponents[K] -} & { - [K in keyof ReactSelectComponents]: ReactSelectComponentsConfig<Option, IsMulti, Group> -}; +export type CustomComponents<Option, IsMulti extends boolean, Group extends GroupBase<Option>> = { + [K in keyof typeof defaultComponents]: typeof defaultComponents[K] +} & { + [K in keyof ReactSelectControlComponents]: ReactSelectComponentsConfig<Option, IsMulti, Group> +};And update keyed usages accordingly.
181-201: Minor: redundant condition inside getClassNames.You wrap the re-assignment in if (typeof classNames?.container == "function") and then check the same condition again in the ternary. The outer check makes the inner redundant.
- if (typeof classNames?.container == "function") { - classNames.container = typeof classNames?.container == "function" - ? (...args) => ([ - classNames?.container?.(...args), - baseClassName, - ]).join(" ") - : () => baseClassName; - } + if (typeof classNames?.container == "function") { + classNames.container = (...args) => ([ + classNames.container?.(...args), + baseClassName, + ]).filter(Boolean).join(" "); + }
282-287: Ensure context.components doesn’t leak custom select configs into non-select slots.getComponents merges context?.components into defaultComponents, but context.components also holds select component configs keyed by ReactSelect… keys. Consider splitting the provider config into:
- components: ComponentLibrary overrides
- selectComponents: keyed overrides for select
This avoids accidental prop leakage and tightens typing on CustomizationContext. If keeping a single bucket, document the expected shape clearly.
324-330: Provider typing: consider narrowing generics to concrete defaults.The provider currently takes CustomizationConfig<any, any, any>. If you don’t allow passing typed Option/IsMulti/Group at the top-level, replace with CustomizationConfig<unknown, boolean, GroupBase> for clearer intent.
-}: CustomizationConfig<any, any, any> & { children: ReactNode; }) => { +}: CustomizationConfig<unknown, boolean, GroupBase<unknown>> & { children: ReactNode; }) => {Import GroupBase to the scope if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
packages/connect-react/src/components/ComponentForm.tsx(3 hunks)packages/connect-react/src/components/ComponentFormContainer.tsx(1 hunks)packages/connect-react/src/components/Control.tsx(2 hunks)packages/connect-react/src/components/ControlApp.tsx(9 hunks)packages/connect-react/src/components/ControlSelect.tsx(6 hunks)packages/connect-react/src/components/ControlSubmit.tsx(3 hunks)packages/connect-react/src/components/Errors.tsx(2 hunks)packages/connect-react/src/components/Field.tsx(2 hunks)packages/connect-react/src/components/InternalField.tsx(3 hunks)packages/connect-react/src/components/RemoteOptionsContainer.tsx(7 hunks)packages/connect-react/src/hooks/customization-context.tsx(4 hunks)packages/connect-react/src/hooks/form-context.tsx(13 hunks)packages/connect-react/src/hooks/form-field-context.tsx(1 hunks)packages/connect-react/src/hooks/use-accounts.tsx(2 hunks)packages/connect-react/src/types.ts(1 hunks)packages/connect-react/src/utils/component.ts(1 hunks)packages/connect-react/src/utils/type-guards.ts(3 hunks)packages/connect-react/tsconfig.json(1 hunks)packages/connect-react/vite.config.mts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (12)
packages/connect-react/src/components/ControlSubmit.tsx (2)
packages/sdk/src/shared/component.ts (1)
ConfigurableProps(203-203)packages/connect-react/src/hooks/form-context.tsx (2)
FormContext(29-54)FormContext(64-64)
packages/connect-react/src/components/Control.tsx (3)
packages/connect-react/src/types.ts (1)
LabelValueOption(36-39)packages/connect-react/src/utils/type-guards.ts (1)
sanitizeOption(70-105)packages/connect-react/src/components/ControlSelect.tsx (1)
ControlSelect(32-227)
packages/connect-react/src/hooks/form-field-context.tsx (2)
packages/sdk/src/shared/component.ts (2)
ConfigurableProp(181-201)ConfigurablePropApp(79-82)packages/sdk/src/shared/index.ts (1)
App(87-122)
packages/connect-react/src/components/Errors.tsx (1)
packages/sdk/src/shared/component.ts (1)
ConfigurableProp(181-201)
packages/connect-react/src/hooks/use-accounts.tsx (1)
packages/sdk/src/shared/index.ts (1)
GetAccountsResponse(546-548)
packages/connect-react/src/components/Field.tsx (2)
packages/connect-react/src/hooks/form-context.tsx (2)
FormContext(29-54)FormContext(64-64)packages/sdk/src/shared/component.ts (1)
ConfigurableProp(181-201)
packages/connect-react/src/components/ControlApp.tsx (2)
packages/sdk/src/shared/index.ts (1)
App(87-122)packages/connect-react/src/hooks/customization-context.tsx (1)
BaseReactSelectProps(126-133)
packages/connect-react/src/components/ComponentForm.tsx (1)
packages/connect-react/src/types.ts (1)
DynamicProps(60-60)
packages/connect-react/src/utils/type-guards.ts (1)
packages/connect-react/src/types.ts (2)
LabelValueOption(36-39)RawPropOption(45-48)
packages/connect-react/src/hooks/form-context.tsx (2)
packages/sdk/src/shared/component.ts (3)
ConfigurableProps(203-203)V1Component(230-238)ConfiguredProps(225-227)packages/connect-react/src/types.ts (5)
DynamicProps(60-60)ReloadComponentPropsResponse(56-58)Observation(31-34)ObservationErrorDetails(17-21)SdkError(6-9)
packages/connect-react/src/components/RemoteOptionsContainer.tsx (3)
packages/connect-react/src/types.ts (2)
ConfigureComponentContext(50-50)RawPropOption(45-48)packages/sdk/src/shared/index.ts (1)
ConfigureComponentOpts(432-473)packages/connect-react/src/utils/type-guards.ts (1)
sanitizeOption(70-105)
packages/connect-react/src/components/ControlSelect.tsx (3)
packages/connect-react/src/types.ts (1)
LabelValueOption(36-39)packages/connect-react/src/hooks/customization-context.tsx (1)
BaseReactSelectProps(126-133)packages/connect-react/src/utils/type-guards.ts (2)
isOptionWithLabel(23-33)sanitizeOption(70-105)
🪛 GitHub Actions: .github/workflows/publish-packages.yaml
packages/connect-react/src/components/ControlSubmit.tsx
[error] vite build failed during prepare step for packages/connect-react. Exit code 1. Build produced 41 TypeScript errors.
[error] 4-4: TS2307: Cannot find module '@pipedream/sdk/browser' or its corresponding type declarations.
packages/connect-react/src/utils/component.ts
[error] vite build failed during prepare step for packages/connect-react. Exit code 1. Build produced 41 TypeScript errors.
packages/connect-react/src/components/Control.tsx
[error] vite build failed during prepare step for packages/connect-react. Exit code 1. Build produced 41 TypeScript errors.
[error] 6-6: TS2307: Cannot find module '@pipedream/sdk' or its corresponding type declarations.
packages/connect-react/vite.config.mts
[error] vite build failed during prepare step for packages/connect-react. Exit code 1. Build produced 41 TypeScript errors.
packages/connect-react/tsconfig.json
[error] vite build failed during prepare step for packages/connect-react. Exit code 1. Build produced 41 TypeScript errors.
packages/connect-react/src/hooks/form-field-context.tsx
[error] vite build failed during prepare step for packages/connect-react. Exit code 1. Build produced 41 TypeScript errors.
[error] 6-6: TS2307: Cannot find module '@pipedream/sdk' or its corresponding type declarations.
[error] 7-7: TS2307: Cannot find module '@pipedream/sdk/browser' or its corresponding type declarations.
packages/connect-react/src/components/Errors.tsx
[error] vite build failed during prepare step for packages/connect-react. Exit code 1. Build produced 41 TypeScript errors.
[error] 5-5: TS2307: Cannot find module '@pipedream/sdk' or its corresponding type declarations.
packages/connect-react/src/hooks/use-accounts.tsx
[error] vite build failed during prepare step for packages/connect-react. Exit code 1. Build produced 41 TypeScript errors.
[error] 4-4: TS2307: Cannot find module '@pipedream/sdk' or its corresponding type declarations.
[error] 6-6: TS2307: Cannot find module '@pipedream/sdk/browser' or its corresponding type declarations.
packages/connect-react/src/components/ComponentFormContainer.tsx
[error] vite build failed during prepare step for packages/connect-react. Exit code 1. Build produced 41 TypeScript errors.
packages/connect-react/src/components/Field.tsx
[error] vite build failed during prepare step for packages/connect-react. Exit code 1. Build produced 41 TypeScript errors.
[error] 3-3: TS2307: Cannot find module '@pipedream/sdk' or its corresponding type declarations.
packages/connect-react/src/types.ts
[error] vite build failed during prepare step for packages/connect-react. Exit code 1. Build produced 41 TypeScript errors.
[error] 4-4: TS2307: Cannot find module '@pipedream/sdk/browser' or its corresponding type declarations.
packages/connect-react/src/components/ControlApp.tsx
[error] vite build failed during prepare step for packages/connect-react. Exit code 1. Build produced 41 TypeScript errors.
[error] 15-15: TS2307: Cannot find module '@pipedream/sdk/browser' or its corresponding type declarations.
packages/connect-react/src/components/ComponentForm.tsx
[error] vite build failed during prepare step for packages/connect-react. Exit code 1. Build produced 41 TypeScript errors.
[error] 9-9: TS2307: Cannot find module '@pipedream/sdk' or its corresponding type declarations.
packages/connect-react/src/components/InternalField.tsx
[error] vite build failed during prepare step for packages/connect-react. Exit code 1. Build produced 41 TypeScript errors.
[error] 1-1: TS2307: Cannot find module '@pipedream/sdk' or its corresponding type declarations.
packages/connect-react/src/utils/type-guards.ts
[error] vite build failed during prepare step for packages/connect-react. Exit code 1. Build produced 41 TypeScript errors.
packages/connect-react/src/hooks/form-context.tsx
[error] vite build failed during prepare step for packages/connect-react. Exit code 1. Build produced 41 TypeScript errors.
[error] 12-12: TS2307: Cannot find module '@pipedream/sdk' or its corresponding type declarations.
[error] 349-349: TS2300: Type 'T' must have a 'Symbol.iterator' method that returns an iterator.
[error] 433-433: TS7006: Parameter 'p' implicitly has an 'any' type.
[error] 449-449: TS2300: Type 'T' must have a 'Symbol.iterator' method that returns an iterator.
packages/connect-react/src/hooks/customization-context.tsx
[error] vite build failed during prepare step for packages/connect-react. Exit code 1. Build produced 41 TypeScript errors.
[error] 4-4: TS2307: Cannot find module '@pipedream/sdk' or its corresponding type declarations.
[error] 4-4: TS2307: Cannot find module '@pipedream/sdk' or its corresponding type declarations.
packages/connect-react/src/components/RemoteOptionsContainer.tsx
[error] vite build failed during prepare step for packages/connect-react. Exit code 1. Build produced 41 TypeScript errors.
packages/connect-react/src/components/ControlSelect.tsx
[error] vite build failed during prepare step for packages/connect-react. Exit code 1. Build produced 41 TypeScript errors.
[error] 124-124: TS2304: Cannot find name 'MenuListProps'.
🪛 GitHub Actions: Pull Request Checks
packages/connect-react/src/components/Control.tsx
[error] 6-6: Cannot find module '@pipedream/sdk' or its corresponding type declarations.
packages/connect-react/src/types.ts
[error] 4-4: Cannot find module '@pipedream/sdk/browser' or its corresponding type declarations.
packages/connect-react/src/components/ControlApp.tsx
[error] 15-15: Cannot find module '@pipedream/sdk/browser' or its corresponding type declarations.
packages/connect-react/src/components/ComponentForm.tsx
[error] 9-9: Cannot find module '@pipedream/sdk' or its corresponding type declarations.
packages/connect-react/src/hooks/form-context.tsx
[error] 12-12: Cannot find module '@pipedream/sdk' or its corresponding type declarations.
[error] 349-349: Type 'T' must have a 'Symbol.iterator' method that returns an iterator.
packages/connect-react/src/components/RemoteOptionsContainer.tsx
[error] 1-1: TS2307: Cannot find module '@pipedream/sdk/browser' or its corresponding type declarations.
packages/connect-react/src/components/ControlSelect.tsx
[error] 124-124: Cannot find name 'MenuListProps'.
🔇 Additional comments (24)
packages/connect-react/src/components/InternalField.tsx (1)
1-1: No action needed:@pipedream/sdkis already declared
A fresh install (pnpm install) and TS server restart should resolve the import error. Since@pipedream/sdkis already listed as a workspace dependency inpackages/connect-react/package.json, the interim import workaround (TODO patch) can be removed.packages/connect-react/src/hooks/use-accounts.tsx (1)
88-91: API field names look correct.external_user_id, app, and oauth_app_id match typical server expectations. No change needed.
packages/connect-react/src/components/ControlApp.tsx (2)
17-26: Typed Option component looks good.Using OptionProps ensures safe access to name/id across Account and the placeholder. No changes.
182-187: Nice UX fallback.Showing a direct “Connect {app.name}” button when no accounts are present is clear.
packages/connect-react/src/types.ts (3)
31-34: Observation union structure looks good.The discriminated union via presence of err vs msg is simple and effective.
36-49: Option types are clean and pragmatic.LabelValueOption, NestedLabelValueOption, and RawPropOption will simplify guard/sanitization logic. No change needed.
52-61: Extending SDK responses with observations is fine; just ensure it mirrors the SDK.If the SDK starts including observations natively, revisit to avoid double wrapping.
You might want to add a quick comment referencing the SDK version this mirrors to aid future maintainers.
packages/connect-react/src/components/Errors.tsx (1)
21-23: Confirm intent: errors are hidden unless enableDebugging is trueThis component returns null when
enableDebuggingis falsy. If these are developer-only diagnostics, fine. If these are user-facing validation errors, this will hide them outside debugging sessions.Would you like these errors to render regardless of
enableDebugging?packages/connect-react/src/hooks/form-field-context.tsx (1)
13-22: LGTM: FormFieldContext shape stays consistent
value,onChange,errors, andenableDebuggingare unchanged. Theextradiscriminated union byConfigurablePropAppreads cleanly with the proposed import fix.packages/connect-react/src/utils/type-guards.ts (1)
69-99: LGTM: sanitizeOption covers primitives, nested __lv, and value-bearing objectsThe normalization path matches how Control/RemoteOptionsContainer consume options and centralizes previously duplicated logic.
packages/connect-react/src/components/Control.tsx (1)
41-46: LGTM: centralizing option normalization with sanitizeOptionUsing
sanitizeOptionhere keeps option shapes consistent withRemoteOptionsContainerandControlSelect.packages/connect-react/src/components/ComponentFormContainer.tsx (1)
12-16: Good XOR prop contract for user identityThe mutually exclusive union enforces exactly one of externalUserId or userId at compile time. This aligns with the PR objective to harden types without changing runtime behavior.
packages/connect-react/src/components/ComponentForm.tsx (3)
4-4: Centralizing DynamicProps import is a winSourcing DynamicProps from ../types creates a single point of truth and reduces circular typing concerns across hooks and components.
28-31: Props typing improvements (onUpdateDynamicProps, sdkResponse) look correct
- onUpdateDynamicProps now uses the canonical DynamicProps.
- sdkResponse typed to U (defaults to ConfiguredProps) improves safety and clarity.
5-9: No action required: @pipedream/sdk is already a workspace dependencyThe connect-react package’s
package.jsonalready lists
"@pipedream/sdk": "workspace:^"(packages/connect-react/package.json, line 31),
so TypeScript will resolve the SDK’s declaration files from the workspace without needing any extrapathsmappings.packages/connect-react/src/components/ControlSubmit.tsx (1)
6-11: Generic props shape for ControlSubmit is consistent with FormContextThe generic constraint ties the button’s disabled logic to the form state correctly.
packages/connect-react/src/components/ControlSelect.tsx (1)
129-130: LoadMoreButton Prop Name VerifiedI confirmed that
LoadMoreButtonis defined with anonChangeprop in itsButtonProps(seeLoadMoreButton.tsx:5-6). UsingonChangehere is intentional, so no prop rename is needed.packages/connect-react/src/hooks/form-context.tsx (3)
247-286: Prop validation guardrails look good.The short-circuit for optional/hidden/skippable props and the safe extraction of app via the "in" guard are solid improvements.
324-332: Initialize queryDisabledIdx from configured props only once per component load.The effect correctly avoids forcing reconfiguration on reloads. No issues.
6-12: Please confirm the SDK package is wired into your workspaceI wasn’t able to locate a local
package.jsondeclaring “@pipedream/sdk” in your monorepo. Before applying the import‐fix below, verify that:• The SDK package actually exists in the repo (e.g. under
packages/sdkor similar) and itspackage.jsonhas
json { "name": "@pipedream/sdk", … }
• Your rootpackage.json(orpnpm-workspace.yaml/turbo.json) lists that SDK folder underworkspaces, so it can be hoisted and referenced.Once you’ve confirmed the SDK lives in your monorepo and is exposed as a workspace package, apply one of these fixes in
packages/connect-react:
- Add a workspace dependency (ensures both types and runtime code resolve correctly):
--- packages/connect-react/package.json +++ packages/connect-react/package.json @@ "version": "…", + "dependencies": { + "@pipedream/sdk": "workspace:*" + }, …
- Or use a TS‐only path alias (if you truly only need types at build time):
--- packages/connect-react/tsconfig.json +++ packages/connect-react/tsconfig.json @@ "compilerOptions": { + "baseUrl": ".", + "paths": { + "@pipedream/sdk": ["../sdk/src"] + }, … }Afterward, re-run your TypeScript build/CI to confirm imports from “@pipedream/sdk” resolve without errors.
packages/connect-react/src/hooks/customization-context.tsx (4)
126-133: Solid: BaseReactSelectProps now correctly generic with defaults.Defaulting IsMulti and Group and typing styles as StylesConfig<Option, IsMulti, Group> improves downstream inference. No issues.
203-214: Components merge order and typing are good.You merge defaults, base overrides, then context overrides. Casting context to the generic config type is acceptable here.
216-223: Style merging: ensure defined objects before calling mergeStyles.You already default to {} for both args—good. If you ever allow falsy, mergeReactSelectStyles expects objects; this implementation is safe.
226-239: getProps ties the generics together correctly.Passing through baseProps?.components/styles and classNames from the generic overloads keeps inference intact. Looks good.
* Use the SDK version published in the NPM registry instead of the one that lives in this repo (it'll be easier to switch) * Fix compilation errors in the example app
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
packages/connect-react/examples/nextjs/package.json(1 hunks)packages/connect-react/examples/nextjs/src/app/page.tsx(3 hunks)packages/connect-react/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/connect-react/package.json
🧰 Additional context used
🧬 Code graph analysis (1)
packages/connect-react/examples/nextjs/src/app/page.tsx (1)
packages/sdk/src/shared/component.ts (2)
ConfiguredProps(225-227)ConfigurableProp(181-201)
🪛 GitHub Actions: Pull Request Checks
packages/connect-react/examples/nextjs/src/app/page.tsx
[error] 5-5: TS2307: Cannot find module '@pipedream/connect-react' or its corresponding type declarations. Build failed during 'vite build' invoked by 'pnpm run build' (package: connect-react).
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: pnpm publish
🔇 Additional comments (3)
packages/connect-react/examples/nextjs/package.json (1)
10-10: SDK version and browser export verifiedAll checks pass:
- Monorepo resolves to a single
@pipedream/sdkversion (1.8.0) across all packages.- The SDK’s package exports include the
./browsersubpath.- The browser entry defines both
ConfigurablePropandConfiguredPropstypes.No further action needed.
packages/connect-react/examples/nextjs/src/app/page.tsx (2)
23-23: Stronger typing for configuredProps is good.Using
useState<ConfiguredProps<ConfigurableProp[]>>matches the SDK’s mapped type and keeps the example non-prescriptive on prop names. LGTM.
35-35: I’ve added scripts to locate the component and confirm both the example’s state type and the SDK’s actual response shape. Once we see these outputs, we can verify whether to broaden fromConfiguredProps<…>to the fullRunActionResponse.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
packages/connect-react/src/components/ControlApp.tsx (1)
11-15: Fix module resolution error by importing from '@pipedream/sdk'The import from '@pipedream/sdk/browser' is causing TS2307 errors. Import these types from the base SDK package instead.
-import type { - Account, - App, - ConfigurablePropApp, -} from "@pipedream/sdk/browser"; +import type { + Account, + App, + ConfigurablePropApp, +} from "@pipedream/sdk";packages/connect-react/src/types.ts (1)
1-4: Fix module resolution error by importing from '@pipedream/sdk'The import from '@pipedream/sdk/browser' is causing TS2307 build failures. Update to import from the base SDK package.
-import type { - ConfigureComponentResponse as SdkConfigureComponentResponse, - ReloadComponentPropsResponse as SdkReloadComponentPropsResponse, -} from "@pipedream/sdk/browser"; +import type { + ConfigureComponentResponse as SdkConfigureComponentResponse, + ReloadComponentPropsResponse as SdkReloadComponentPropsResponse, +} from "@pipedream/sdk";packages/connect-react/src/components/ControlSelect.tsx (1)
138-145: Pass finalComponents to Select to enable LoadMore UIThe
finalComponentsobject is built but never passed to the Select component, preventing the LoadMore UI from rendering.return ( <MaybeCreatableSelect inputId={id} instanceId={id} options={cleanedOptions} value={selectValue} isMulti={prop.type.endsWith("[]")} isClearable={true} required={!prop.optional} getOptionLabel={(option) => sanitizeOption(option).label} getOptionValue={(option) => String(sanitizeOption(option).value)} onChange={handleChange} + components={finalComponents} {...props} {...selectProps} {...additionalProps} /> );packages/connect-react/src/hooks/form-context.tsx (1)
222-239: Fix TypeScript iterator and type inference issuesThe
configurablePropsneeds explicit typing to avoid iterator and implicit-any errors in loops and callbacks.- const configurableProps = useMemo(() => { - let props = dynamicProps?.configurableProps || formProps.component.configurable_props || []; + const configurableProps = useMemo<ConfigurableProps>(() => { + let props: ConfigurableProps = dynamicProps?.configurableProps || formProps.component.configurable_props || []; if (propNames?.length) { const _configurableProps = []; for (const prop of props) { // TODO decided propNames (and hideOptionalProps) should NOT filter dynamic props if (propNames.findIndex((name) => prop.name === name) >= 0) { _configurableProps.push(prop); } } - props = _configurableProps as typeof props; // XXX + props = _configurableProps; } if (reloadPropIdx != null) { props = Array.isArray(props) ? props.slice(0, reloadPropIdx + 1) // eslint-disable-line react/prop-types : props; // XXX } - return props as T; + return props; }, [ dynamicProps?.configurableProps, formProps.component.configurable_props, propNames, reloadPropIdx, ]);Then in the context value assembly (around line 596):
- configurableProps, + configurableProps: configurableProps as T,
🧹 Nitpick comments (5)
packages/connect-react/src/components/ControlApp.tsx (3)
95-97: Consider using a more type-safe approach instead of @ts-expect-errorThe
@ts-expect-errordirective suppresses TypeScript checking. Consider properly typing the options or using a type assertion.- // @ts-expect-error this seems to work (this overrides enabled so don't just set to true) - suspense: !!app, + // Override enabled with suspense when app is truthy + suspense: !!app, + } as const,
123-129: Consider memoizing newAccountPlaceholder to avoid recreationsThe
newAccountPlaceholderobject is recreated on every render. Since it depends only onapp.name, consider memoizing it.- const selectOptions = useMemo<SelectValue[]>(() => [ - ...accounts, - newAccountPlaceholder, - ], [ - accounts, - ]); + const selectOptions = useMemo<SelectValue[]>(() => [ + ...accounts, + newAccountPlaceholder, + ], [ + accounts, + newAccountPlaceholder, + ]);Also update the
newAccountPlaceholderdefinition:- const newAccountPlaceholder: AccountPlaceholder = { - id: "_new", - name: `Connect new ${app.name} account...`, - }; + const newAccountPlaceholder: AccountPlaceholder = useMemo(() => ({ + id: "_new", + name: `Connect new ${app.name} account...`, + }), [app.name]);
112-114: Implement error handling for account connection failureThe TODO comment indicates missing error handling. This should be implemented to provide proper user feedback.
Would you like me to generate the error handling implementation or open a new issue to track this task?
packages/connect-react/src/components/ControlSelect.tsx (1)
81-120: Consider simplifying the selectValue computation logicThe selectValue computation has complex nested conditions. Consider extracting helper functions for clarity.
const findMatchingOption = (value: T, options: LabelValueOption<T>[]): LabelValueOption<T> | undefined => { return options.find(item => item.value === value); }; const createLabelValueOption = (value: T): LabelValueOption<T> => ({ label: String(value), value, }); const selectValue: LabelValueOption<T> | LabelValueOption<T>[] | null = useMemo(() => { if (rawValue == null) { return null; } if (Array.isArray(rawValue)) { return rawValue.map(o => isOptionWithLabel(o) ? o : (findMatchingOption(o, selectOptions) || createLabelValueOption(o)) ); } let ret = rawValue; if (ret && typeof ret === "object" && "__lv" in ret) { ret = ret.__lv; } if (isOptionWithLabel(ret)) { return ret; } return findMatchingOption(ret, selectOptions) || createLabelValueOption(ret); }, [rawValue, selectOptions]);packages/connect-react/src/hooks/form-context.tsx (1)
535-535: Consider defining a proper type for error detailsUsing
anytype reduces type safety. Consider defining an interface for the error details structure.- // eslint-disable-next-line @typescript-eslint/no-explicit-any - const errorFromDetails = (data: any, ret: SdkError[]) => { + interface ErrorDetailsData { + error: string; + details: unknown; + } + const errorFromDetails = (data: ErrorDetailsData, ret: SdkError[]) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
packages/connect-react/package.json(3 hunks)packages/connect-react/src/components/Control.tsx(2 hunks)packages/connect-react/src/components/ControlApp.tsx(9 hunks)packages/connect-react/src/components/ControlSelect.tsx(6 hunks)packages/connect-react/src/components/ControlSubmit.tsx(3 hunks)packages/connect-react/src/components/InternalField.tsx(2 hunks)packages/connect-react/src/components/RemoteOptionsContainer.tsx(7 hunks)packages/connect-react/src/hooks/form-context.tsx(13 hunks)packages/connect-react/src/hooks/form-field-context.tsx(1 hunks)packages/connect-react/src/types.ts(1 hunks)packages/connect-react/src/utils/component.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/connect-react/src/components/Control.tsx
- packages/connect-react/src/utils/component.ts
- packages/connect-react/src/hooks/form-field-context.tsx
- packages/connect-react/package.json
- packages/connect-react/src/components/InternalField.tsx
- packages/connect-react/src/components/RemoteOptionsContainer.tsx
- packages/connect-react/src/components/ControlSubmit.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
packages/connect-react/src/components/ControlSelect.tsx (3)
packages/connect-react/src/types.ts (1)
LabelValueOption(36-39)packages/connect-react/src/hooks/customization-context.tsx (1)
BaseReactSelectProps(126-133)packages/connect-react/src/utils/type-guards.ts (2)
isOptionWithLabel(23-33)sanitizeOption(70-105)
packages/connect-react/src/hooks/form-context.tsx (2)
packages/sdk/src/shared/component.ts (3)
ConfigurableProps(203-203)V1Component(230-238)ConfiguredProps(225-227)packages/connect-react/src/types.ts (5)
DynamicProps(60-60)ReloadComponentPropsResponse(56-58)Observation(31-34)ObservationErrorDetails(17-21)SdkError(6-9)
packages/connect-react/src/components/ControlApp.tsx (2)
packages/sdk/src/shared/index.ts (1)
App(87-122)packages/connect-react/src/hooks/customization-context.tsx (1)
BaseReactSelectProps(126-133)
🔇 Additional comments (7)
packages/connect-react/src/components/ControlApp.tsx (1)
28-32: LGTM! Clean type definition for placeholder accountThe
AccountPlaceholdertype andSelectValueunion type are well-structured for handling both existing accounts and the "connect new" option.packages/connect-react/src/types.ts (2)
11-34: LGTM! Well-structured observation typesThe discriminated union pattern for
Observationwith distinctObservationErrorandObservationLogtypes provides good type safety.
36-49: LGTM! Clean option type definitionsThe
LabelValueOption,NestedLabelValueOption, andRawPropOptiontypes provide a clear hierarchy for handling different option formats.packages/connect-react/src/components/ControlSelect.tsx (2)
59-63: LGTM! Clean option sanitization in useEffectGood approach to sanitize options once on mount/update rather than in render.
147-174: LGTM! Well-handled creation flow with proper sanitizationThe creation logic properly sanitizes new options and maintains clean option objects in the select state.
packages/connect-react/src/hooks/form-context.tsx (2)
503-523: LGTM! Clean error handling refactorThe refactored error handling functions properly work with the new
ObservationandObservationErrorDetailstypes.
255-257: LGTM! Improved type safety for app extractionGood defensive check using
"app" in field.extrabefore accessing the app property.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/connect-react/src/types.ts (1)
1-4: Fix TS2307: import SDK types from the base entry (duplicate of prior review).The subpath "@pipedream/sdk/browser" isn’t exported, causing CI failures. Import from the package base until a browser subpath export is added in the SDK.
Apply this diff:
-import type { - ConfigureComponentResponse as SdkConfigureComponentResponse, - ReloadComponentPropsResponse as SdkReloadComponentPropsResponse, -} from "@pipedream/sdk/browser"; +import type { + ConfigureComponentResponse as SdkConfigureComponentResponse, + ReloadComponentPropsResponse as SdkReloadComponentPropsResponse, +} from "@pipedream/sdk";If you own the SDK and truly need a browser subpath, add an "exports" entry for "./browser" with matching .d.ts files in @pipedream/sdk, then revert to that import.
🧹 Nitpick comments (8)
packages/connect-react/src/types.ts (8)
11-16: Clarify abbreviated field names in ObservationBase.Names like ts, k, h are terse. Add JSDoc to document semantics without breaking callers.
-export type ObservationBase = { - ts: number; - k: string; - h?: string; -} +/** Base metadata for an observation. + * ts: epoch millis + * k: short key / category + * h: optional hash / correlation id + */ +export type ObservationBase = { + ts: number; + k: string; + h?: string; +}
17-22: Avoid duplication with SdkError and make stack optional.Error.stack is optional in some environments. You can also reuse SdkError to DRY the fields.
-export type ObservationErrorDetails = { - name: string; - message: string; - stack: string; -} +export type ObservationErrorDetails = SdkError & { + stack?: string; +}
31-35: Optional: add an explicit discriminant for easier narrowing.You currently discriminate by presence of err vs msg. Adding a type tag enables exhaustive switches with compiler help.
Example:
-export type Observation = ObservationBase & ( - | ObservationError - | ObservationLog -) +export type Observation = + | (ObservationBase & { kind: "error" } & ObservationError) + | (ObservationBase & { kind: "log" } & ObservationLog)
36-40: Minor ergonomics: default generic and immutability for options.Defaults reduce type noise; readonly prevents accidental mutation.
-export type LabelValueOption<T> = { - label: string; - value: T; -} +export type LabelValueOption<T = unknown> = { + readonly label: string; + readonly value: T; +}
41-44: Document the nested option wrapper and prevent mutation.The __lv sentinel is non-obvious; add a brief comment and make it readonly.
-export type NestedLabelValueOption<T> = { - __lv: LabelValueOption<T> | T; -} +/** Internal wrapper used by sanitizeOption to normalize raw option shapes. */ +export type NestedLabelValueOption<T> = { + readonly __lv: LabelValueOption<T> | T; +}
50-50: Consider constraining context to JSON-serializable values.If this context crosses process boundaries or is logged, prefer JSONValue over unknown.
-export type ConfigureComponentContext = Record<string, unknown> +export type JsonValue = string | number | boolean | null | { [k: string]: JsonValue } | JsonValue[]; +export type ConfigureComponentContext = Record<string, JsonValue>;
52-55: Nit: Enforce immutability onobservations
I’ve confirmed via ripgrep thatSdkConfigureComponentResponse(inpackages/sdk/src/shared/index.ts) does not already define anobservationsfield, so there’s no risk of shadowing or collision. As a small, optional improvement, converting your array to a read-only type helps guard against accidental mutations.• File:
packages/connect-react/src/types.ts(lines 52–55)
• Change:-export type ConfigureComponentResponse = SdkConfigureComponentResponse & { - observations: Observation[]; -} +export type ConfigureComponentResponse = SdkConfigureComponentResponse & { + readonly observations: ReadonlyArray<Observation>; +}
56-59: Ensureobservationsis immutable and guard against future collisionsTo align with our immutability conventions and prevent a potential name collision if the SDK ever adds an
observationsfield, please update the type inpackages/connect-react/src/types.ts:• File: packages/connect-react/src/types.ts
Line: ~56-export type ReloadComponentPropsResponse = SdkReloadComponentPropsResponse & { - observations: Observation[]; -} +export type ReloadComponentPropsResponse = + Omit<SdkReloadComponentPropsResponse, "observations"> & { + readonly observations: ReadonlyArray<Observation>; + }This change makes the
observationsarray read-only and usesOmit<…>to avoid a future property collision.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/connect-react/src/types.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (3)
packages/connect-react/src/types.ts (3)
23-26: LGTM: clear error wrapper shape.Nesting error details under err avoids ambiguous unions and simplifies narrowing.
27-30: LGTM: concise log variant.
60-60: dynamicProps is non-nullable; no change requiredI’ve confirmed that in the SDK’s
ReloadComponentPropsResponse(packages/sdk/src/shared/index.ts),dynamicPropsis defined as a required object (no?or| null), soSdkReloadComponentPropsResponse["dynamicProps"]is already non‐nullable. ApplyingNonNullable<>here would be a no‐op.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/connect-react/src/types.ts (2)
1-4: Good fix: import SDK types from the base entry.This resolves the TS2307 error from the former "/browser" subpath and aligns imports across the package.
45-49: Centralize normalization forRawPropOption<T>before accessing.label/.value.Reiterate: consumers must call a single
sanitizeOption(or equivalent) helper. This avoids fragile ad-hoc unwrapping when options are primitives, nested wrappers, or legacy shapes.Run to find potential unsafe direct property access:
#!/bin/bash # Find direct .label/.value access that likely bypasses normalization rg -n --type=ts -C2 '(\.\s*(label|value)\b)' packages/connect-react \ | rg -v 'LabelValueOption|type-guards|sanitize'If matches are found, refactor to use
sanitizeOption(...).packages/connect-react/src/components/ControlApp.tsx (1)
12-15: Good fix: unify imports to '@pipedream/sdk'.Resolves the missing module errors from the '/browser' subpath and keeps imports consistent.
🧹 Nitpick comments (7)
packages/connect-react/src/types.ts (1)
6-9: Unify error type shapes; makestackoptional to match common Error semantics.Avoid duplicating error field sets and reduce friction when mapping native Errors or SDK errors into observations.
Apply:
-export type SdkError = { - name: string; - message: string; -} +export type SdkError = { + name: string; + message: string; + stack?: string; +} @@ -export type ObservationErrorDetails = { - name: string; - message: string; - stack: string; -} +export type ObservationErrorDetails = SdkErrorAlso applies to: 17-21
packages/connect-react/src/components/ControlApp.tsx (6)
96-98: Replace@ts-expect-errorwith a typed path forsuspense.Current suppression hides a type mismatch between your wrapper’s
useQueryOptsand React Query options. Prefer exposingsuspense?: booleanin the hook’s option type so callers remain type-safe.Proposed (outside this file) in
use-accounts.tsx:// Extend the options type the hook accepts: export type UseAccountsOpts = { useQueryOpts?: Omit<UseQueryOptions<AccountsResponse, Error>, "queryKey" | "queryFn"> & { suspense?: boolean; }; };Then this file can drop the suppression and keep:
- // @ts-expect-error this seems to work (this overrides enabled so don't just set to true) - suspense: !!app, + suspense: !!app,Want me to open a follow-up PR to adjust the hook types?
123-129: Memo dependency nit: includenewAccountPlaceholder.
selectOptionsdepends on bothaccountsandnewAccountPlaceholder. Add it to avoid stale options whenapp.namechanges.- const selectOptions = useMemo<SelectValue[]>(() => [ - ...accounts, - newAccountPlaceholder, - ], [ - accounts, - ]); + const selectOptions = useMemo<SelectValue[]>( + () => [...accounts, newAccountPlaceholder], + [accounts, newAccountPlaceholder], + );
130-140: Avoid auto-selecting “Connect new …” when no account is chosen.Selecting the placeholder by default changes UX: the field appears filled instead of showing the placeholder text. Prefer
nullto show the prompt until the user picks something.-type SelectValue = Account | AccountPlaceholder; +type SelectValue = Account | AccountPlaceholder; @@ - const selectValue = useMemo<SelectValue>(() => { + const selectValue = useMemo<SelectValue | null>(() => { if (value?.authProvisionId) { for (const item of accounts) { if (value.authProvisionId === item.id) { return item; } } } - - return newAccountPlaceholder; + return null; }, [ accounts, value, ]); @@ - <Select + <Select<SelectValue, false> instanceId={id} value={selectValue} options={selectOptions}If the intent is to promote the connect flow by default, keep current behavior; otherwise the above restores the typical empty-select UX.
Also applies to: 154-158
164-176: Type theonChangehandler and handle the “new” case tersely.Minor cleanup and stronger typing when paired with the generic Select change.
- onChange={(a) => { - if (a) { - if (a.id === "_new") { - // start connect account and then select it, etc. - // TODO unset / put in loading state - startConnectAccount(); - } else { - onChange({ - authProvisionId: a.id, - }); - } - } else { - onChange(undefined); - } - }} + onChange={(a) => { + if (!a) return onChange(undefined); + if (a.id === "_new") { + // TODO: unset / loading state while connecting + startConnectAccount(); + return; + } + onChange({ authProvisionId: a.id }); + }}
183-188: Consider basic error reporting inonError.Even a minimal toast/log will help users diagnose failed account connects.
- onError: () => { - // TODO - }, + onError: (err) => { + // TODO: replace with app-wide toast/notification + console.error("Failed to connect account", err); + },
154-166: Note:requiredhas no effect on react-select.Browsers ignore
requiredon custom components. Keep if your form handler checks it manually; otherwise safe to remove.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
packages/connect-react/src/components/ControlApp.tsx(9 hunks)packages/connect-react/src/components/ControlSubmit.tsx(3 hunks)packages/connect-react/src/components/RemoteOptionsContainer.tsx(7 hunks)packages/connect-react/src/hooks/form-field-context.tsx(1 hunks)packages/connect-react/src/hooks/use-accounts.tsx(2 hunks)packages/connect-react/src/types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/connect-react/src/hooks/form-field-context.tsx
- packages/connect-react/src/components/ControlSubmit.tsx
- packages/connect-react/src/components/RemoteOptionsContainer.tsx
- packages/connect-react/src/hooks/use-accounts.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/connect-react/src/components/ControlApp.tsx (2)
packages/sdk/src/shared/index.ts (1)
App(87-122)packages/connect-react/src/hooks/customization-context.tsx (1)
BaseReactSelectProps(126-133)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Lint Code Base
- GitHub Check: pnpm publish
🔇 Additional comments (2)
packages/connect-react/src/types.ts (1)
52-58: Confirm server contract: shouldobservationsbe required?You’re forcing
observationsto be present. If the API omits it (e.g., no logs/errors), downstream code will compile against a value that may beundefinedat runtime. If it’s not guaranteed, mark it optional.Option if not guaranteed:
-export type ConfigureComponentResponse = SdkConfigureComponentResponse & { - observations: Observation[]; -} +export type ConfigureComponentResponse = SdkConfigureComponentResponse & { + observations?: Observation[]; +} @@ -export type ReloadComponentPropsResponse = SdkReloadComponentPropsResponse & { - observations: Observation[]; -} +export type ReloadComponentPropsResponse = SdkReloadComponentPropsResponse & { + observations?: Observation[]; +}Also applies to: 60-60
packages/connect-react/src/components/ControlApp.tsx (1)
88-91: Correct param names foruseAccountsquery.Switching to
external_user_id(snake_case) andoauth_app_idlooks consistent with the API surface.
There was a problem hiding this comment.
We deleted this whole directory since we're using this repo instead:
https://github.com/PipedreamHQ/pipedream-connect-examples
Changelog
@pipedream/sdk) to make the v2 migration easierWHY
The purpose of this change is to prepare the
connect-reactpackage to be migrated to the new SDK by backfilling type annotations that relate to the current SDK. This change should not cause runtime behaviour changes, but it does include some minor refactors that helped me get to a point were there are no compile-time errors.The very first step that I took was to make the build process fail in the case of compile-time errors. In order to speed up the development and release of this package, we made the decision to allow these errors as long as they did not affect the runtime behaviour. This PR intends to solve all these existing errors, while preventing new ones from showing up in the future.
The other important change here is to explicitly annotate variables and functions that relate to the current SDK, so as to let the compiler detect which parts of the code we must migrate, and check if we're doing it correctly. This is not infallible but it's a good step forward. For types that the SDK does not export, I redefined them in a single file called
src/types.ts(it already existed) so that we don't forget about replacing those.Last but not least, there was some Claude-generated code in
src/utils/type-guards.tsthat was either unused or unnecessarily complicated. I cleaned up as much as possible, and reutilized thesanitizeOptionfunction defined in there in other parts of the code that did basically the same thing. This helped me check that we were processing the right type of data while making the code (c)leaner.Summary by CodeRabbit
New Features
Bug Fixes
Chores