Skip to content

Commit f014f2d

Browse files
authored
fix: unified polling hook (#121)
## Description <!-- Provide a brief description of your changes --> **Note:** PR titles should follow [Conventional Commits](https://www.conventionalcommits.org/) format (e.g., `feat(devbox): add support for custom env vars` or `fix(snapshot): resolve pagination issue`) as they are used for automatic release notes generation. ## Type of Change <!-- Mark the relevant option with an 'x' --> - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation update - [ ] Code refactoring - [ ] Performance improvement - [ ] Test updates ## Related Issues <!-- Link to related issues using #issue-number --> Closes # ## Changes Made <!-- Describe the changes in detail --> ## Testing <!-- Describe how you tested your changes --> - [ ] I have tested locally - [ ] I have added/updated tests - [ ] All existing tests pass ## Checklist - [ ] My code follows the code style of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have updated the documentation accordingly - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published ## Screenshots (if applicable) <!-- Add screenshots to help explain your changes --> ## Additional Notes <!-- Any additional information that reviewers should know -->
1 parent 148a7bc commit f014f2d

11 files changed

Lines changed: 267 additions & 360 deletions

src/components/DevboxDetailPage.tsx

Lines changed: 8 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import {
1414
} from "./ResourceDetailPage.js";
1515
import { getDevboxUrl } from "../utils/url.js";
1616
import { colors } from "../utils/theme.js";
17-
import { getDevbox } from "../services/devboxService.js";
1817
import { formatTimeAgo } from "../utils/time.js";
1918
import type { Devbox } from "../store/devboxStore.js";
2019

@@ -23,15 +22,11 @@ interface DevboxDetailPageProps {
2322
onBack: () => void;
2423
}
2524

26-
export const DevboxDetailPage = ({
27-
devbox: initialDevbox,
28-
onBack,
29-
}: DevboxDetailPageProps) => {
25+
export const DevboxDetailPage = ({ devbox, onBack }: DevboxDetailPageProps) => {
3026
const [showActions, setShowActions] = React.useState(false);
3127
const [selectedOperationKey, setSelectedOperationKey] = React.useState<
3228
string | null
3329
>(null);
34-
const [currentDevbox, setCurrentDevbox] = React.useState(initialDevbox);
3530

3631
// All possible operations for devboxes
3732
const allOperations: ResourceOperation[] = [
@@ -751,60 +746,44 @@ export const DevboxDetailPage = ({
751746
};
752747

753748
// Polling function
754-
const pollDevbox = React.useCallback(async () => {
755-
const updated = await getDevbox(initialDevbox.id);
756-
setCurrentDevbox(updated);
757-
return updated;
758-
}, [initialDevbox.id]);
759-
760749
// Show DevboxActionsMenu when an action is selected
761750
if (showActions) {
762751
return (
763752
<DevboxActionsMenu
764-
devbox={currentDevbox}
753+
devbox={devbox}
765754
onBack={() => {
766755
setShowActions(false);
767756
setSelectedOperationKey(null);
768757
}}
769758
breadcrumbItems={[
770759
{ label: "Devboxes" },
771-
{ label: currentDevbox.name || currentDevbox.id },
760+
{ label: devbox.name || devbox.id },
772761
]}
773762
initialOperation={selectedOperationKey || undefined}
774763
skipOperationsMenu={true}
775764
/>
776765
);
777766
}
778767

779-
// Determine if we should poll based on status
780-
const shouldPoll =
781-
currentDevbox.status === "running" ||
782-
currentDevbox.status === "provisioning" ||
783-
currentDevbox.status === "initializing" ||
784-
currentDevbox.status === "resuming" ||
785-
currentDevbox.status === "suspending";
786-
787768
return (
788769
<ResourceDetailPage
789-
resource={currentDevbox}
770+
resource={devbox}
790771
resourceType="Devboxes"
791772
getDisplayName={(d) => d.name || d.id}
792773
getId={(d) => d.id}
793774
getStatus={(d) => d.status}
794775
getUrl={(d) => getDevboxUrl(d.id)}
795-
detailSections={buildDetailSections(currentDevbox)}
796-
operations={getFilteredOperations(currentDevbox)}
776+
detailSections={buildDetailSections(devbox)}
777+
operations={getFilteredOperations(devbox)}
797778
onOperation={handleOperation}
798779
onBack={onBack}
799780
buildDetailLines={buildDetailLines}
800781
additionalContent={
801782
<StateHistory
802-
stateTransitions={currentDevbox.state_transitions}
803-
shutdownReason={currentDevbox.shutdown_reason ?? undefined}
783+
stateTransitions={devbox.state_transitions}
784+
shutdownReason={devbox.shutdown_reason ?? undefined}
804785
/>
805786
}
806-
pollResource={shouldPoll ? pollDevbox : undefined}
807-
pollInterval={3000}
808787
/>
809788
);
810789
};

src/components/ResourceDetailPage.tsx

Lines changed: 12 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ const truncateString = (str: string, maxLength: number): string => {
4343
};
4444

4545
export function ResourceDetailPage<T>({
46-
resource: initialResource,
46+
resource,
4747
resourceType,
4848
getDisplayName,
4949
getId,
@@ -56,9 +56,6 @@ export function ResourceDetailPage<T>({
5656
onBack,
5757
buildDetailLines,
5858
additionalContent,
59-
pollResource,
60-
onPollUpdate,
61-
pollInterval = 3000,
6259
}: ResourceDetailPageProps<T>) {
6360
const isMounted = React.useRef(true);
6461
const { navigate } = useNavigation();
@@ -71,15 +68,8 @@ export function ResourceDetailPage<T>({
7168
};
7269
}, []);
7370

74-
// Local state for resource data (updated by polling)
75-
const [currentResource, setCurrentResource] = React.useState(initialResource);
7671
const [copyStatus, setCopyStatus] = React.useState<string | null>(null);
7772

78-
// Keep local resource in sync when parent provides fresher data.
79-
React.useEffect(() => {
80-
setCurrentResource(initialResource);
81-
}, [initialResource]);
82-
8373
// Copy to clipboard with status feedback
8474
const handleCopy = React.useCallback(async (text: string) => {
8575
const status = await copyToClipboard(text);
@@ -110,33 +100,12 @@ export function ResourceDetailPage<T>({
110100
}
111101
}, [totalSelectableItems, selectedIndex]);
112102

113-
// Background polling for resource details
114-
React.useEffect(() => {
115-
if (!pollResource || showDetailedInfo) return;
116-
117-
const interval = setInterval(async () => {
118-
if (isMounted.current) {
119-
try {
120-
const updatedResource = await pollResource();
121-
if (isMounted.current) {
122-
setCurrentResource(updatedResource);
123-
onPollUpdate?.(updatedResource);
124-
}
125-
} catch {
126-
// Silently ignore polling errors
127-
}
128-
}
129-
}, pollInterval);
130-
131-
return () => clearInterval(interval);
132-
}, [pollResource, pollInterval, showDetailedInfo, onPollUpdate]);
133-
134103
// Calculate viewport for detailed info view
135104
const detailViewport = useViewportHeight({ overhead: 18, minHeight: 10 });
136105

137-
const displayName = getDisplayName(currentResource);
138-
const resourceId = getId(currentResource);
139-
const status = getStatus(currentResource);
106+
const displayName = getDisplayName(resource);
107+
const resourceId = getId(resource);
108+
const status = getStatus(resource);
140109

141110
// Execute a field action
142111
const executeFieldAction = React.useCallback(
@@ -159,8 +128,8 @@ export function ResourceDetailPage<T>({
159128

160129
const handleOpenInBrowser = React.useCallback(() => {
161130
if (!getUrl) return;
162-
openUrlInBrowser(getUrl(currentResource));
163-
}, [getUrl, currentResource]);
131+
openUrlInBrowser(getUrl(resource));
132+
}, [getUrl, resource]);
164133

165134
const exitDetailedInfo = React.useCallback(() => {
166135
setShowDetailedInfo(false);
@@ -176,7 +145,7 @@ export function ResourceDetailPage<T>({
176145
} else {
177146
const op = operations[operationIndex];
178147
if (op) {
179-
onOperation(op.key, currentResource);
148+
onOperation(op.key, resource);
180149
}
181150
}
182151
}, [
@@ -185,7 +154,7 @@ export function ResourceDetailPage<T>({
185154
selectedIndex,
186155
operationIndex,
187156
operations,
188-
currentResource,
157+
resource,
189158
executeFieldAction,
190159
onOperation,
191160
]);
@@ -207,7 +176,7 @@ export function ResourceDetailPage<T>({
207176
bindings: {
208177
q: onBack,
209178
escape: onBack,
210-
c: () => handleCopy(getId(currentResource)),
179+
c: () => handleCopy(getId(resource)),
211180
...(buildDetailLines
212181
? {
213182
i: () => {
@@ -233,7 +202,7 @@ export function ResourceDetailPage<T>({
233202
);
234203
if (matchedOpIndex !== -1) {
235204
setSelectedIndex(actionableFields.length + matchedOpIndex);
236-
onOperation(operations[matchedOpIndex].key, currentResource);
205+
onOperation(operations[matchedOpIndex].key, resource);
237206
}
238207
},
239208
},
@@ -243,7 +212,7 @@ export function ResourceDetailPage<T>({
243212
detailScroll,
244213
exitDetailedInfo,
245214
onBack,
246-
currentResource,
215+
resource,
247216
buildDetailLines,
248217
selectedIndex,
249218
totalSelectableItems,
@@ -263,7 +232,7 @@ export function ResourceDetailPage<T>({
263232
if (showDetailedInfo && buildDetailLines) {
264233
return (
265234
<DetailedInfoView
266-
detailLines={buildDetailLines(currentResource)}
235+
detailLines={buildDetailLines(resource)}
267236
scrollOffset={detailScroll}
268237
viewportHeight={detailViewport.viewportHeight}
269238
displayName={displayName}

src/components/resourceDetailTypes.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,10 +111,4 @@ export interface ResourceDetailPageProps<T> {
111111
buildDetailLines?: (resource: T) => React.ReactElement[];
112112
/** Optional: Additional content to render after details section */
113113
additionalContent?: React.ReactNode;
114-
/** Optional: Polling function to refresh resource data */
115-
pollResource?: () => Promise<T>;
116-
/** Optional: Called whenever polling returns a fresh resource */
117-
onPollUpdate?: (resource: T) => void;
118-
/** Polling interval in ms (default: 3000) */
119-
pollInterval?: number;
120114
}

src/hooks/useResourceDetail.ts

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
import React from "react";
2+
3+
export interface UseResourceDetailOptions<T> {
4+
/** Resource id to fetch; when undefined, no fetch or poll runs */
5+
id: string | undefined;
6+
/** Fetch function called with id */
7+
fetch: (id: string) => Promise<T>;
8+
/** Optional initial data (e.g. from store) shown until first fetch resolves */
9+
initialData?: T | null;
10+
/** Polling interval in ms; omit or 0 to disable */
11+
pollInterval?: number;
12+
/** When provided, polling only runs while this returns true (e.g. status is in-progress) */
13+
shouldPoll?: (data: T) => boolean;
14+
}
15+
16+
export interface UseResourceDetailResult<T> {
17+
/** Current resource data (from initialData until fetch resolves, then from fetch/poll) */
18+
data: T | null;
19+
/** True while the initial fetch for this id is in progress */
20+
loading: boolean;
21+
/** Error from the last fetch attempt (poll errors are ignored) */
22+
error: Error | null;
23+
}
24+
25+
/**
26+
* Shared hook for detail view: "get state for this resource" and optional "poll it".
27+
* Single source of truth for detail data; no callbacks to parent.
28+
*/
29+
export function useResourceDetail<T>({
30+
id,
31+
fetch: fetchFn,
32+
initialData,
33+
pollInterval = 0,
34+
shouldPoll,
35+
}: UseResourceDetailOptions<T>): UseResourceDetailResult<T> {
36+
const [data, setData] = React.useState<T | null>(initialData ?? null);
37+
const [loading, setLoading] = React.useState(!!id);
38+
const [error, setError] = React.useState<Error | null>(null);
39+
const isMounted = React.useRef(true);
40+
41+
React.useEffect(() => {
42+
isMounted.current = true;
43+
return () => {
44+
isMounted.current = false;
45+
};
46+
}, []);
47+
48+
// Reset state when id changes
49+
React.useEffect(() => {
50+
if (!id) {
51+
setData(initialData ?? null);
52+
setLoading(false);
53+
setError(null);
54+
return;
55+
}
56+
setData(initialData ?? null);
57+
setError(null);
58+
setLoading(true);
59+
}, [id, initialData]);
60+
61+
// Initial fetch when id is set
62+
React.useEffect(() => {
63+
if (!id) return;
64+
65+
let cancelled = false;
66+
setLoading(true);
67+
setError(null);
68+
69+
fetchFn(id)
70+
.then((result) => {
71+
if (!cancelled && isMounted.current) {
72+
setData(result);
73+
setError(null);
74+
}
75+
})
76+
.catch((err) => {
77+
if (!cancelled && isMounted.current) {
78+
setError(err instanceof Error ? err : new Error(String(err)));
79+
// Keep initialData if we had it
80+
if (initialData != null) {
81+
setData(initialData);
82+
}
83+
}
84+
})
85+
.finally(() => {
86+
if (!cancelled && isMounted.current) {
87+
setLoading(false);
88+
}
89+
});
90+
91+
return () => {
92+
cancelled = true;
93+
};
94+
}, [id, fetchFn]);
95+
96+
// Polling: only when pollInterval > 0, we have data, and shouldPoll(data) is true
97+
React.useEffect(() => {
98+
if (!id || pollInterval <= 0 || !data) return;
99+
if (shouldPoll && !shouldPoll(data)) return;
100+
101+
const interval = setInterval(() => {
102+
if (!isMounted.current) return;
103+
fetchFn(id)
104+
.then((result) => {
105+
if (isMounted.current) {
106+
setData(result);
107+
}
108+
})
109+
.catch(() => {
110+
// Silently ignore polling errors
111+
});
112+
}, pollInterval);
113+
114+
return () => clearInterval(interval);
115+
}, [id, fetchFn, pollInterval, data, shouldPoll]);
116+
117+
return { data, loading, error };
118+
}

0 commit comments

Comments
 (0)