Skip to content

Commit bb7651a

Browse files
dannyrooseveltdannyroosevelt
committed
More PR feedback
1 parent 226c3f4 commit bb7651a

File tree

4 files changed

+168
-28
lines changed

4 files changed

+168
-28
lines changed

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

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -151,9 +151,20 @@ export function RemoteOptionsContainer({ queryEnabled }: RemoteOptionsContainerP
151151
// Track queryKey and account changes (need these before effects that use them)
152152
const queryKeyString = JSON.stringify(queryKeyInput);
153153
const accountKey = JSON.stringify(accountValue);
154-
const prevQueryKeyRef = useRef<string>();
154+
const prevResetKeyRef = useRef<string>();
155155
const prevAccountKeyRef = useRef<string>();
156156

157+
const resetKey = useMemo(() => {
158+
const {
159+
page: _page,
160+
prevContext: _prevContext,
161+
...rest
162+
} = componentConfigureInput;
163+
return JSON.stringify(rest);
164+
}, [
165+
componentConfigureInput,
166+
]);
167+
157168
// Check if there's an account prop - if so, it must be set for the query to be enabled
158169
const hasAccountProp = useMemo(() => {
159170
return configurableProps.some((p: { type: string; }) => p.type === "app");
@@ -317,20 +328,25 @@ export function RemoteOptionsContainer({ queryEnabled }: RemoteOptionsContainerP
317328

318329
// Reset pagination when queryKey changes
319330
useEffect(() => {
320-
const queryKeyChanged = prevQueryKeyRef.current && prevQueryKeyRef.current !== queryKeyString;
331+
if (!prevResetKeyRef.current) {
332+
prevResetKeyRef.current = resetKey;
333+
return;
334+
}
335+
336+
const queryParamsChanged = prevResetKeyRef.current !== resetKey;
321337

322-
if (queryKeyChanged) {
323-
// Query params changed - reset pagination state
338+
if (queryParamsChanged) {
324339
setPage(0);
325340
setContext(undefined);
326341
setNextContext(undefined);
327342
setCanLoadMore(true);
328343
setAccumulatedData([]);
329344
setAccumulatedValues(new Set());
330345
}
331-
prevQueryKeyRef.current = queryKeyString;
346+
347+
prevResetKeyRef.current = resetKey;
332348
}, [
333-
queryKeyString,
349+
resetKey,
334350
]);
335351

336352
// Separately track account changes to clear field value

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

Lines changed: 60 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import {
2-
useState, useCallback, useEffect, useRef,
2+
useState,
3+
useCallback,
4+
useEffect,
5+
useRef,
36
} from "react";
47
import {
58
useQuery, UseQueryResult,
@@ -8,12 +11,18 @@ import type {
811
AppsListRequest, App,
912
} from "@pipedream/sdk";
1013
import { useFrontendClient } from "./frontend-client-context";
14+
import {
15+
clonePaginatedPage,
16+
isPaginatedPage,
17+
PaginatedPage,
18+
} from "../utils/pagination";
1119

1220
export type UseAppsResult = Omit<UseQueryResult<unknown, Error>, "data"> & {
1321
apps: App[];
1422
isLoadingMore: boolean;
1523
hasMore: boolean;
1624
loadMore: () => Promise<void>;
25+
loadMoreError?: Error;
1726
};
1827

1928
/**
@@ -36,10 +45,24 @@ export const useApps = (input?: AppsListRequest): UseAppsResult => {
3645
const [
3746
nextPage,
3847
setNextPage,
39-
] = useState<unknown>(null);
48+
] = useState<PaginatedPage<App> | null>(null);
49+
50+
const [
51+
loadMoreError,
52+
setLoadMoreError,
53+
] = useState<Error>();
4054

4155
// Track previous input to detect when query params actually change
4256
const prevInputRef = useRef<string>();
57+
const prevQueryDataRef = useRef<unknown>();
58+
const isMountedRef = useRef(true);
59+
60+
useEffect(() => {
61+
isMountedRef.current = true;
62+
return () => {
63+
isMountedRef.current = false;
64+
};
65+
}, []);
4366

4467
const query = useQuery({
4568
queryKey: [
@@ -55,14 +78,22 @@ export const useApps = (input?: AppsListRequest): UseAppsResult => {
5578
// Reset pagination ONLY when query params change, not on refetches
5679
useEffect(() => {
5780
const inputKey = JSON.stringify(input ?? null);
58-
const isNewQuery = prevInputRef.current !== inputKey;
81+
const hasNewInput = prevInputRef.current !== inputKey;
82+
const hasNewData = prevQueryDataRef.current !== query.data;
5983

60-
if (query.data && isNewQuery) {
84+
if (!query.data || !isPaginatedPage<App>(query.data)) {
85+
return;
86+
}
87+
88+
if (query.data && (hasNewInput || hasNewData)) {
6189
prevInputRef.current = inputKey;
62-
setAllApps(query.data.data || []);
63-
setHasMore(query.data.hasNextPage());
64-
setNextPage(query.data);
90+
prevQueryDataRef.current = query.data;
91+
const pageData = clonePaginatedPage(query.data);
92+
setAllApps([...(pageData.data || [])]);
93+
setHasMore(pageData.hasNextPage());
94+
setNextPage(pageData);
6595
setIsLoadingMore(false);
96+
setLoadMoreError(undefined);
6697
}
6798
}, [
6899
query.data,
@@ -72,20 +103,37 @@ export const useApps = (input?: AppsListRequest): UseAppsResult => {
72103
const loadMore = useCallback(async () => {
73104
if (!nextPage || !hasMore || isLoadingMore) return;
74105

106+
if (!isPaginatedPage<App>(nextPage)) {
107+
setLoadMoreError(new Error("Next page response is not paginated"));
108+
return;
109+
}
110+
75111
setIsLoadingMore(true);
76112
try {
77-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
78-
const nextPageData = await (nextPage as any).getNextPage();
113+
const nextPageData = await nextPage.getNextPage();
114+
if (!isMountedRef.current) {
115+
return;
116+
}
117+
79118
setAllApps((prev) => [
80119
...prev,
81120
...(nextPageData.data || []),
82121
]);
83122
setHasMore(nextPageData.hasNextPage());
84123
setNextPage(nextPageData);
124+
setLoadMoreError(undefined);
85125
} catch (err) {
86-
console.error("Error loading more apps:", err);
126+
if (!isMountedRef.current) {
127+
return;
128+
}
129+
const error = err instanceof Error
130+
? err
131+
: new Error(String(err));
132+
setLoadMoreError(error);
87133
} finally {
88-
setIsLoadingMore(false);
134+
if (isMountedRef.current) {
135+
setIsLoadingMore(false);
136+
}
89137
}
90138
}, [
91139
nextPage,
@@ -99,5 +147,6 @@ export const useApps = (input?: AppsListRequest): UseAppsResult => {
99147
isLoadingMore,
100148
hasMore,
101149
loadMore,
150+
loadMoreError,
102151
};
103152
};

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

Lines changed: 60 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import {
2-
useState, useCallback, useEffect, useRef,
2+
useState,
3+
useCallback,
4+
useEffect,
5+
useRef,
36
} from "react";
47
import {
58
useQuery, UseQueryResult,
@@ -9,12 +12,18 @@ import type {
912
Component,
1013
} from "@pipedream/sdk";
1114
import { useFrontendClient } from "./frontend-client-context";
15+
import {
16+
clonePaginatedPage,
17+
isPaginatedPage,
18+
PaginatedPage,
19+
} from "../utils/pagination";
1220

1321
export type UseComponentsResult = Omit<UseQueryResult<unknown, Error>, "data"> & {
1422
components: Component[];
1523
isLoadingMore: boolean;
1624
hasMore: boolean;
1725
loadMore: () => Promise<void>;
26+
loadMoreError?: Error;
1827
};
1928

2029
/**
@@ -37,10 +46,24 @@ export const useComponents = (input?: ComponentsListRequest): UseComponentsResul
3746
const [
3847
nextPage,
3948
setNextPage,
40-
] = useState<unknown>(null);
49+
] = useState<PaginatedPage<Component> | null>(null);
50+
51+
const [
52+
loadMoreError,
53+
setLoadMoreError,
54+
] = useState<Error>();
4155

4256
// Track previous input to detect when query params actually change
4357
const prevInputRef = useRef<string>();
58+
const prevQueryDataRef = useRef<unknown>();
59+
const isMountedRef = useRef(true);
60+
61+
useEffect(() => {
62+
isMountedRef.current = true;
63+
return () => {
64+
isMountedRef.current = false;
65+
};
66+
}, []);
4467

4568
const query = useQuery({
4669
queryKey: [
@@ -56,14 +79,22 @@ export const useComponents = (input?: ComponentsListRequest): UseComponentsResul
5679
// Reset pagination ONLY when query params change, not on refetches
5780
useEffect(() => {
5881
const inputKey = JSON.stringify(input ?? null);
59-
const isNewQuery = prevInputRef.current !== inputKey;
82+
const hasNewInput = prevInputRef.current !== inputKey;
83+
const hasNewData = prevQueryDataRef.current !== query.data;
6084

61-
if (query.data && isNewQuery) {
85+
if (!query.data || !isPaginatedPage<Component>(query.data)) {
86+
return;
87+
}
88+
89+
if (query.data && (hasNewInput || hasNewData)) {
6290
prevInputRef.current = inputKey;
63-
setAllComponents(query.data.data || []);
64-
setHasMore(query.data.hasNextPage());
65-
setNextPage(query.data);
91+
prevQueryDataRef.current = query.data;
92+
const pageData = clonePaginatedPage(query.data);
93+
setAllComponents([...(pageData.data || [])]);
94+
setHasMore(pageData.hasNextPage());
95+
setNextPage(pageData);
6696
setIsLoadingMore(false);
97+
setLoadMoreError(undefined);
6798
}
6899
}, [
69100
query.data,
@@ -73,20 +104,37 @@ export const useComponents = (input?: ComponentsListRequest): UseComponentsResul
73104
const loadMore = useCallback(async () => {
74105
if (!nextPage || !hasMore || isLoadingMore) return;
75106

107+
if (!isPaginatedPage<Component>(nextPage)) {
108+
setLoadMoreError(new Error("Next page response is not paginated"));
109+
return;
110+
}
111+
76112
setIsLoadingMore(true);
77113
try {
78-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
79-
const nextPageData = await (nextPage as any).getNextPage();
114+
const nextPageData = await nextPage.getNextPage();
115+
if (!isMountedRef.current) {
116+
return;
117+
}
118+
80119
setAllComponents((prev) => [
81120
...prev,
82121
...(nextPageData.data || []),
83122
]);
84123
setHasMore(nextPageData.hasNextPage());
85124
setNextPage(nextPageData);
125+
setLoadMoreError(undefined);
86126
} catch (err) {
87-
console.error("Error loading more components:", err);
127+
if (!isMountedRef.current) {
128+
return;
129+
}
130+
const error = err instanceof Error
131+
? err
132+
: new Error(String(err));
133+
setLoadMoreError(error);
88134
} finally {
89-
setIsLoadingMore(false);
135+
if (isMountedRef.current) {
136+
setIsLoadingMore(false);
137+
}
90138
}
91139
}, [
92140
nextPage,
@@ -100,5 +148,6 @@ export const useComponents = (input?: ComponentsListRequest): UseComponentsResul
100148
isLoadingMore,
101149
hasMore,
102150
loadMore,
151+
loadMoreError,
103152
};
104153
};

‎packages/connect-react/src/utils/pagination.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
export type PaginatedPage<T> = {
2+
data?: T[];
3+
hasNextPage: () => boolean;
4+
getNextPage: () => Promise<PaginatedPage<T>>;
5+
};
6+
7+
export const isPaginatedPage = <T,>(value: unknown): value is PaginatedPage<T> => {
8+
if (!value || typeof value !== "object") {
9+
return false;
10+
}
11+
12+
const candidate = value as {
13+
hasNextPage?: unknown;
14+
getNextPage?: unknown;
15+
};
16+
17+
return (
18+
typeof candidate.hasNextPage === "function"
19+
&& typeof candidate.getNextPage === "function"
20+
);
21+
};
22+
23+
export const clonePaginatedPage = <T,>(page: PaginatedPage<T>): PaginatedPage<T> => {
24+
const prototype = Object.getPrototypeOf(page);
25+
return Object.assign(Object.create(prototype ?? Object.prototype), page);
26+
};

0 commit comments

Comments
 (0)