Skip to content

Commit 226c3f4

Browse files
addressing PR feedback from code rabbit
1 parent 2a6c7fd commit 226c3f4

File tree

6 files changed

+46
-42
lines changed

6 files changed

+46
-42
lines changed

packages/connect-react/src/components/ControlSelect.tsx

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -135,29 +135,32 @@ export function ControlSelect<T extends PropOptionValue>({
135135
// Memoize custom components to prevent remounting
136136
// Recompute when caller/customizer supplies new component overrides
137137
const finalComponents = useMemo(() => {
138-
const base = {
139-
...props.components,
140-
...componentsOverride,
138+
const mergedComponents = {
139+
...(props.components ?? {}),
140+
...(componentsOverride ?? {}),
141141
};
142+
const ParentMenuList = mergedComponents.MenuList ?? components.MenuList;
142143

143144
// Always set MenuList, conditionally render button inside
144145
const CustomMenuList = ({
145146
// eslint-disable-next-line react/prop-types
146147
children, ...menuProps
147148
}: MenuListProps<LabelValueOption<T>, boolean>) => (
148-
<components.MenuList {...menuProps}>
149+
<ParentMenuList {...menuProps}>
149150
{children}
150151
{showLoadMoreButtonRef.current && (
151152
<div className="pt-4">
152153
<LoadMoreButton onChange={() => onLoadMoreRef.current?.()} />
153154
</div>
154155
)}
155-
</components.MenuList>
156+
</ParentMenuList>
156157
);
157158
CustomMenuList.displayName = "CustomMenuList";
158-
base.MenuList = CustomMenuList;
159159

160-
return base;
160+
return {
161+
...mergedComponents,
162+
MenuList: CustomMenuList,
163+
};
161164
}, [
162165
props.components,
163166
componentsOverride,

packages/connect-react/src/components/RemoteOptionsContainer.tsx

Lines changed: 12 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,7 @@ import { useFrontendClient } from "../hooks/frontend-client-context";
1111
import {
1212
ConfigureComponentContext, RawPropOption,
1313
} from "../types";
14-
import {
15-
isString, sanitizeOption,
16-
} from "../utils/type-guards";
14+
import { sanitizeOption } from "../utils/type-guards";
1715
import { ControlSelect } from "./ControlSelect";
1816

1917
export type RemoteOptionsContainerProps = {
@@ -28,12 +26,8 @@ type ConfigurePropResult = {
2826

2927
// Helper to extract value from an option
3028
const extractOptionValue = (o: RawPropOption): PropOptionValue | null => {
31-
if (isString(o)) {
32-
return o;
33-
} else if (o && typeof o === "object" && "value" in o && o.value != null) {
34-
return o.value;
35-
}
36-
return null;
29+
const normalized = sanitizeOption(o);
30+
return normalized.value ?? null;
3731
};
3832

3933
export function RemoteOptionsContainer({ queryEnabled }: RemoteOptionsContainerProps) {
@@ -138,13 +132,8 @@ export function RemoteOptionsContainer({ queryEnabled }: RemoteOptionsContainerP
138132
query,
139133
]);
140134

141-
// React Query key excludes dynamicPropsId
142135
const queryKeyInput = useMemo(() => {
143-
const {
144-
// eslint-disable-next-line @typescript-eslint/no-unused-vars
145-
dynamicPropsId: _dynamicPropsId, ...rest
146-
} = componentConfigureInput;
147-
return rest;
136+
return componentConfigureInput;
148137
}, [
149138
componentConfigureInput,
150139
]);
@@ -215,16 +204,14 @@ export function RemoteOptionsContainer({ queryEnabled }: RemoteOptionsContainerP
215204
};
216205
}
217206

218-
let _options: RawPropOption[] = [];
219-
if (options?.length) {
220-
_options = options;
221-
}
222-
if (stringOptions?.length) {
223-
_options = stringOptions.map((str) => ({
224-
label: str,
225-
value: str,
226-
}));
227-
}
207+
const stringOptionObjects = stringOptions?.map((str) => ({
208+
label: str,
209+
value: str,
210+
})) ?? [];
211+
const _options: RawPropOption[] = [
212+
...(options ?? []),
213+
...stringOptionObjects,
214+
];
228215

229216
return {
230217
error: undefined,

packages/connect-react/src/components/SelectApp.tsx

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {
2-
useId, useState, useEffect, useMemo, useCallback,
2+
useId, useState, useEffect, useMemo, useCallback, useRef,
33
} from "react";
44
import Select, { components } from "react-select";
55
import type {
@@ -63,6 +63,13 @@ export function SelectApp({
6363
MenuList,
6464
} = components;
6565

66+
const isLoadingMoreRef = useRef(isLoadingMore);
67+
useEffect(() => {
68+
isLoadingMoreRef.current = isLoadingMore;
69+
}, [
70+
isLoadingMore,
71+
]);
72+
6673
// Memoize the selected value to prevent unnecessary recalculations
6774
const selectedValue = useMemo(() => {
6875
return apps?.find((o: App) => o.nameSlug === value?.nameSlug)
@@ -86,8 +93,6 @@ export function SelectApp({
8693
]);
8794

8895
// Memoize custom components to prevent remounting
89-
// Note: Don't include isLoadingMore in deps - it's read from closure
90-
// and components will re-render naturally when parent re-renders
9196
const customComponents = useMemo(() => ({
9297
Option: (optionProps: OptionProps<App>) => (
9398
<Option {...optionProps}>
@@ -135,7 +140,7 @@ export function SelectApp({
135140
MenuList: (props: MenuListProps<App>) => (
136141
<MenuList {...props}>
137142
{props.children}
138-
{isLoadingMore && (
143+
{isLoadingMoreRef.current && (
139144
<div style={{
140145
padding: "8px 12px",
141146
textAlign: "center",

packages/connect-react/src/components/SelectComponent.tsx

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,11 @@ import {
33
Component,
44
} from "@pipedream/sdk";
55
import {
6-
useId, useMemo, useCallback,
6+
useId,
7+
useMemo,
8+
useCallback,
9+
useRef,
10+
useEffect,
711
} from "react";
812
import Select, { components } from "react-select";
913
import type { MenuListProps } from "react-select";
@@ -36,6 +40,13 @@ export function SelectComponent({
3640

3741
const { MenuList } = components;
3842

43+
const isLoadingMoreRef = useRef(isLoadingMore);
44+
useEffect(() => {
45+
isLoadingMoreRef.current = isLoadingMore;
46+
}, [
47+
isLoadingMore,
48+
]);
49+
3950
// Memoize the selected value to prevent unnecessary recalculations
4051
const selectedValue = useMemo(() => {
4152
return componentsList?.find((c: Component) => c.key === value?.key) || null;
@@ -56,13 +67,11 @@ export function SelectComponent({
5667
]);
5768

5869
// Memoize custom components to prevent remounting
59-
// Note: Don't include isLoadingMore in deps - it's read from closure
60-
// and MenuList will re-render naturally when parent re-renders
6170
const customComponents = useMemo(() => ({
6271
MenuList: (props: MenuListProps<Component>) => (
6372
<MenuList {...props}>
6473
{props.children}
65-
{isLoadingMore && (
74+
{isLoadingMoreRef.current && (
6675
<div style={{
6776
padding: "8px 12px",
6877
textAlign: "center",

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ export const useApps = (input?: AppsListRequest): UseAppsResult => {
5454

5555
// Reset pagination ONLY when query params change, not on refetches
5656
useEffect(() => {
57-
const inputKey = JSON.stringify(input);
57+
const inputKey = JSON.stringify(input ?? null);
5858
const isNewQuery = prevInputRef.current !== inputKey;
5959

6060
if (query.data && isNewQuery) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ export const useComponents = (input?: ComponentsListRequest): UseComponentsResul
5555

5656
// Reset pagination ONLY when query params change, not on refetches
5757
useEffect(() => {
58-
const inputKey = JSON.stringify(input);
58+
const inputKey = JSON.stringify(input ?? null);
5959
const isNewQuery = prevInputRef.current !== inputKey;
6060

6161
if (query.data && isNewQuery) {

0 commit comments

Comments
 (0)