Skip to content

fix(connect-react): Fix pre-configured props handling issues #18782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

fix(connect-react): Fix pre-configured props handling issues #18782

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4d54aca
Fix pre-configured props handling in forms
rnijhara Oct 16, 2025
cde427b
fix: clear dropdown options when dependent field changes
rnijhara Oct 16, 2025
899e224
fix: prevent duplicate API calls from race condition in form state
rnijhara Oct 16, 2025
eb4315a
fix: handle integer dropdown values and error states properly
rnijhara Oct 16, 2025
f853fb6
style: fix eslint formatting errors
rnijhara Oct 16, 2025
97aa7dd
fix: add type annotations for PropOptionValue Sets
rnijhara Oct 16, 2025
0a0d9f8
fix: handle multi-select integer fields with __lv format
rnijhara Oct 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion packages/connect-react/src/components/ControlSelect.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,12 @@ export function ControlSelect<T extends PropOptionValue>({
}
} else if (rawValue && typeof rawValue === "object" && "__lv" in (rawValue as Record<string, unknown>)) {
// Extract the actual option from __lv wrapper and sanitize to LV
return sanitizeOption(((rawValue as Record<string, unknown>).__lv) as T);
// Handle both single objects and arrays wrapped in __lv
const lvContent = (rawValue as Record<string, unknown>).__lv;
if (Array.isArray(lvContent)) {
return lvContent.map((item) => sanitizeOption(item as T));
}
return sanitizeOption(lvContent as T);
} else if (!isOptionWithLabel(rawValue)) {
const lvOptions = selectOptions?.[0] && isOptionWithLabel(selectOptions[0]);
if (lvOptions) {
Expand Down
66 changes: 54 additions & 12 deletions packages/connect-react/src/components/RemoteOptionsContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ import type {
ConfigurePropOpts, PropOptionValue,
} from "@pipedream/sdk";
import { useQuery } from "@tanstack/react-query";
import { useState } from "react";
import {
useEffect, useState,
} from "react";
import { useFormContext } from "../hooks/form-context";
import { useFormFieldContext } from "../hooks/form-field-context";
import { useFrontendClient } from "../hooks/frontend-client-context";
Expand Down Expand Up @@ -95,6 +97,19 @@ export function RemoteOptionsContainer({ queryEnabled }: RemoteOptionsContainerP
setError,
] = useState<{ name: string; message: string; }>();

// Reset pagination and error when dependent fields change.
// This ensures the next query starts fresh from page 0, triggering a data replace instead of append
useEffect(() => {
setPage(0);
setCanLoadMore(true);
setError(undefined);
}, [
externalUserId,
component.key,
prop.name,
JSON.stringify(configuredPropsUpTo),
]);

const onLoadMore = () => {
setPage(pageable.page)
setContext(pageable.prevContext)
Expand All @@ -106,6 +121,7 @@ export function RemoteOptionsContainer({ queryEnabled }: RemoteOptionsContainerP

// TODO handle error!
const {
data,
isFetching, refetch,
} = useQuery({
queryKey: [
Expand All @@ -131,7 +147,13 @@ export function RemoteOptionsContainer({ queryEnabled }: RemoteOptionsContainerP
message: errors[0],
});
}
return [];
// Return proper pageable structure on error to prevent crashes
return {
page: 0,
prevContext: {},
data: [],
values: new Set<PropOptionValue>(),
};
}
let _options: RawPropOption[] = []
if (options?.length) {
Expand All @@ -148,8 +170,12 @@ export function RemoteOptionsContainer({ queryEnabled }: RemoteOptionsContainerP
_options = options;
}

// For fresh queries (page 0), start with empty set to avoid accumulating old options
// For pagination (page > 0), use existing set to dedupe across pages
const allValues = page === 0
? new Set<PropOptionValue>()
: new Set(pageable.values)
const newOptions = []
const allValues = new Set(pageable.values)
for (const o of _options || []) {
let value: PropOptionValue;
if (isString(o)) {
Expand All @@ -167,26 +193,42 @@ export function RemoteOptionsContainer({ queryEnabled }: RemoteOptionsContainerP
allValues.add(value)
newOptions.push(o)
}
let data = pageable.data
let responseData = pageable.data
if (newOptions.length) {
data = [
...pageable.data,
...newOptions,
] as RawPropOption[]
setPageable({
// Replace data on fresh queries (page 0), append on pagination (page > 0)
responseData = page === 0
? newOptions as RawPropOption[]
: [
...pageable.data,
...newOptions,
] as RawPropOption[]
const newPageable = {
page: page + 1,
prevContext: res.context,
data,
data: responseData,
values: allValues,
})
}
setPageable(newPageable)
return newPageable;
} else {
setCanLoadMore(false)
return pageable;
}
return data;
},
enabled: !!queryEnabled,
});

// Sync pageable state with query data to handle both fresh fetches and cached returns
// When React Query returns cached data, the queryFn doesn't run, so we need to sync
// the state here to ensure options populate correctly on remount
useEffect(() => {
if (data) {
setPageable(data);
}
}, [
data,
]);

const showLoadMoreButton = () => {
return !isFetching && !error && canLoadMore
}
Expand Down
69 changes: 59 additions & 10 deletions packages/connect-react/src/hooks/form-context.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ export const FormContextProvider = <T extends ConfigurableProps>({
}, [
component.key,
]);

// XXX pass this down? (in case we make it hash or set backed, but then also provide {add,remove} instead of set)
const optionalPropIsEnabled = (prop: ConfigurableProp) => enabledOptionalProps[prop.name];

Expand Down Expand Up @@ -275,6 +276,32 @@ export const FormContextProvider = <T extends ConfigurableProps>({
reloadPropIdx,
]);

// Auto-enable optional props that have values in configuredProps
// This ensures optional fields with saved values are shown when mounting with pre-configured props
useEffect(() => {
const propsToEnable: Record<string, boolean> = {};

for (const prop of configurableProps) {
if (prop.optional) {
const value = configuredProps[prop.name as keyof ConfiguredProps<T>];
if (value !== undefined) {
propsToEnable[prop.name] = true;
}
}
}

if (Object.keys(propsToEnable).length > 0) {
setEnabledOptionalProps((prev) => ({
...prev,
...propsToEnable,
}));
}
}, [
component.key,
configurableProps,
configuredProps,
]);

// these validations are necessary because they might override PropInput for number case for instance
// so can't rely on that base control form validation
const propErrors = (prop: ConfigurableProp, value: unknown): string[] => {
Expand Down Expand Up @@ -355,12 +382,21 @@ export const FormContextProvider = <T extends ConfigurableProps>({
};

useEffect(() => {
// Initialize queryDisabledIdx on load so that we don't force users
// to reconfigure a prop they've already configured whenever the page
// or component is reloaded
updateConfiguredPropsQueryDisabledIdx(_configuredProps)
// Initialize queryDisabledIdx using actual configuredProps (includes parent-passed values in controlled mode)
// instead of _configuredProps which starts empty. This ensures that when mounting with pre-configured
// values, remote options queries are not incorrectly blocked.
updateConfiguredPropsQueryDisabledIdx(configuredProps)
}, [
_configuredProps,
component.key,
]);

Comment on lines +385 to +392
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Init queryDisabledIdx from configuredProps: good. Also initialize state to undefined to avoid transient block.

The effect correctly derives the initial idx from actual configuredProps. To eliminate the brief 0 default, initialize with undefined.

Apply:

-] = useState<number | undefined>(0);
+] = useState<number | undefined>(undefined);

(lines 142–144)

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In packages/connect-react/src/hooks/form-context.tsx around lines 142–144, the
queryDisabledIdx state is initialized to 0 causing a transient block; change its
initial value to undefined so the effect that calls
updateConfiguredPropsQueryDisabledIdx(configuredProps) can set the real value on
mount. Update the state type to allow undefined if necessary and audit
downstream uses to handle undefined (use nullish checks or fallback) so no
runtime errors occur.

// Update queryDisabledIdx reactively when configuredProps changes.
// This prevents race conditions where queryDisabledIdx updates synchronously before
// configuredProps completes its state update, causing duplicate API calls with stale data.
useEffect(() => {
updateConfiguredPropsQueryDisabledIdx(configuredProps);
}, [
configuredProps,
]);

Comment on lines +393 to 401
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Reactive sync looks right; remove synchronous duplicate to prevent double work.

Now that queryDisabledIdx derives from configuredProps reactively, also calling updateConfiguredPropsQueryDisabledIdx inside updateConfiguredProps can double-trigger downstream logic.

Apply:

 const updateConfiguredProps = (configuredProps: ConfiguredProps<T>) => {
   setConfiguredProps(configuredProps);
-  updateConfiguredPropsQueryDisabledIdx(configuredProps);
   updateConfigurationErrors(configuredProps)
 };

(lines 365–369)

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In packages/connect-react/src/hooks/form-context.tsx around lines 393 to 401,
reactive syncing now updates queryDisabledIdx via useEffect so the synchronous
call inside updateConfiguredProps (lines 365–369) is redundant and can
double-trigger downstream logic; remove the direct call to
updateConfiguredPropsQueryDisabledIdx from updateConfiguredProps and rely solely
on the useEffect to update queryDisabledIdx when configuredProps changes to
prevent duplicate work.

useEffect(() => {
Expand All @@ -386,8 +422,13 @@ export const FormContextProvider = <T extends ConfigurableProps>({
if (skippablePropTypes.includes(prop.type)) {
continue;
}
// if prop.optional and not shown, we skip and do on un-collapse
// if prop.optional and not shown, we still preserve the value if it exists
// This prevents losing saved values for optional props that haven't been enabled yet
if (prop.optional && !optionalPropIsEnabled(prop)) {
const value = configuredProps[prop.name as keyof ConfiguredProps<T>];
if (value !== undefined) {
newConfiguredProps[prop.name as keyof ConfiguredProps<T>] = value;
}
continue;
}
const value = configuredProps[prop.name as keyof ConfiguredProps<T>];
Expand All @@ -398,7 +439,18 @@ export const FormContextProvider = <T extends ConfigurableProps>({
}
} else {
if (prop.type === "integer" && typeof value !== "number") {
delete newConfiguredProps[prop.name as keyof ConfiguredProps<T>];
// Preserve label-value format from remote options dropdowns
// Remote options store values as {__lv: {label: "...", value: ...}}
// For multi-select fields, this will be an array of __lv objects
const isLabelValue = value && typeof value === "object" && "__lv" in value;
const isArrayOfLabelValues = Array.isArray(value) && value.length > 0 &&
value.every((item) => item && typeof item === "object" && "__lv" in item);

if (!(isLabelValue || isArrayOfLabelValues)) {
delete newConfiguredProps[prop.name as keyof ConfiguredProps<T>];
} else {
newConfiguredProps[prop.name as keyof ConfiguredProps<T>] = value;
}
} else {
newConfiguredProps[prop.name as keyof ConfiguredProps<T>] = value;
}
Expand Down Expand Up @@ -440,9 +492,6 @@ export const FormContextProvider = <T extends ConfigurableProps>({
if (prop.reloadProps) {
setReloadPropIdx(idx);
}
if (prop.type === "app" || prop.remoteOptions) {
updateConfiguredPropsQueryDisabledIdx(newConfiguredProps);
}
const errs = propErrors(prop, value);
const newErrors = {
...errors,
Expand Down