Skip to content

Commit fe17a36

Browse files
skearnesclaude
skearnes
and
claude
authored
Pass datasetId into ChartView and guard its data fetch (#183)
* Pass datasetId into ChartView and guard its data fetch Closes #181, #182. ChartView previously parsed the dataset id out of `window.location.pathname`, bypassing React Router and assuming the route shape stays `/dataset/:id`. Pass `datasetId` from MainDatasetView (already in scope via `useParams`) as a prop instead, and don't mount the charts until the param is known. The `/api/${apiCall}` data-fetch in the same effect had no `response.ok` gate, so a 4xx/5xx body got handed to `response.json()` and either threw on the HTML or fed nonsense into `setInputsData` / `createChart`. Throw on non-2xx so the existing catch branch flips loading off and logs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Show an inline error in ChartView when the data fetch fails Surface the backend error in the chart panel instead of leaving the spinner up + a silent console.error. Also reset loading and clear any prior error when the effect re-runs on apiCall / datasetId change, and refresh the deps comment to mention datasetId. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * encodeURIComponent dataset_id in ChartView fetch Aligns with MainDatasetView's dataset-metadata fetch and avoids a latent issue if an ID ever contains URL-special characters. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Abort ChartView's in-flight fetch on dataset change Without this, navigating between two datasets while the first fetch is still in flight can let the stale response resolve after the new one, overwriting setInputsData with data from the wrong dataset. Cancel via AbortController and swallow the resulting AbortError. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f08d69e commit fe17a36

3 files changed

Lines changed: 69 additions & 31 deletions

File tree

‎app/src/views/dataset-view/ChartView.scss‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,17 @@
3535
transform: translate(-50%, -50%);
3636
}
3737

38+
&__error {
39+
position: absolute;
40+
top: 50%;
41+
left: 50%;
42+
transform: translate(-50%, -50%);
43+
color: #b00;
44+
font-size: 12px;
45+
text-align: center;
46+
padding: 0 12px;
47+
}
48+
3849
&__tooltip {
3950
position: fixed;
4051
background: white;

‎app/src/views/dataset-view/ChartView.tsx‎

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,13 @@ interface ChartViewProps {
3030
title: string;
3131
apiCall: string;
3232
role: string;
33+
datasetId: string;
3334
isCollapsed?: boolean;
3435
}
3536

36-
const ChartView: React.FC<ChartViewProps> = ({ uniqueId, title, apiCall, role, isCollapsed = false }) => {
37+
const ChartView: React.FC<ChartViewProps> = ({ uniqueId, title, apiCall, role, datasetId, isCollapsed = false }) => {
3738
const [loading, setLoading] = useState(true);
39+
const [fetchError, setFetchError] = useState<string | null>(null);
3840
const [inputsData, setInputsData] = useState<ChartData[]>([]);
3941
const [showTooltip, setShowTooltip] = useState<'visible' | 'hidden'>('hidden');
4042
const [currentTimesAppearing, setCurrentTimesAppearing] = useState(0);
@@ -193,22 +195,37 @@ const ChartView: React.FC<ChartViewProps> = ({ uniqueId, title, apiCall, role, i
193195
createChart(inputsData, width, height);
194196
}, [inputsData, isCollapsed, createChart]);
195197

196-
// Fetch data on mount. The resize effect below renders the chart once
197-
// inputsData populates, so isCollapsed/createChart are intentionally not
198-
// deps here — including them would re-fire the fetch every collapse toggle.
198+
// Fetch chart data when the endpoint or dataset changes. The resize effect
199+
// below redraws on isCollapsed change once inputsData is populated, so
200+
// isCollapsed/createChart are intentionally not deps here. The
201+
// AbortController guards against a stale response from the previous
202+
// datasetId racing in after the new fetch has started.
199203
useEffect(() => {
200-
const datasetId = window.location.pathname.split('/')[2];
201-
202-
fetch(`/api/${apiCall}?dataset_id=${datasetId}`, { method: 'GET' })
203-
.then(response => response.json())
204+
const controller = new AbortController();
205+
setLoading(true);
206+
setFetchError(null);
207+
fetch(`/api/${apiCall}?dataset_id=${encodeURIComponent(datasetId)}`, {
208+
method: 'GET',
209+
signal: controller.signal,
210+
})
211+
.then(response => {
212+
if (!response.ok) {
213+
throw new Error(`${apiCall} failed (HTTP ${response.status})`);
214+
}
215+
return response.json();
216+
})
204217
.then((data: ChartData[]) => {
205218
setLoading(false);
206219
setInputsData(data);
207220
})
208-
.catch(() => {
221+
.catch((error: Error) => {
222+
if (error.name === 'AbortError') return;
223+
console.error(`Error fetching ${apiCall}:`, error);
209224
setLoading(false);
225+
setFetchError(error.message);
210226
});
211-
}, [apiCall]);
227+
return () => controller.abort();
228+
}, [apiCall, datasetId]);
212229

213230
// Resize chart when isCollapsed changes
214231
useEffect(() => {
@@ -230,16 +247,20 @@ const ChartView: React.FC<ChartViewProps> = ({ uniqueId, title, apiCall, role, i
230247
<svg
231248
ref={svgRef}
232249
id={uniqueId}
233-
style={{ visibility: loading ? 'hidden' : 'visible' }}
250+
style={{ visibility: loading || fetchError ? 'hidden' : 'visible' }}
234251
/>
235252
</div>
236253

237-
<div
238-
className="chart-view__loading"
239-
style={{ visibility: loading ? 'visible' : 'hidden' }}
240-
>
241-
<LoadingSpinner />
242-
</div>
254+
{fetchError ? (
255+
<div className="chart-view__error">Failed to load chart: {fetchError}</div>
256+
) : (
257+
<div
258+
className="chart-view__loading"
259+
style={{ visibility: loading ? 'visible' : 'hidden' }}
260+
>
261+
<LoadingSpinner />
262+
</div>
263+
)}
243264

244265
{showSmiles && (
245266
<div

‎app/src/views/dataset-view/MainDatasetView.tsx‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -76,20 +76,26 @@ const MainDatasetView: React.FC = () => {
7676
id="chartsectioncharts"
7777
className={`charts-content ${isCollapsed ? '' : 'expanded'}`}
7878
>
79-
<ChartView
80-
uniqueId="reactantsFrequency"
81-
title="Frequency of Reactants"
82-
apiCall="input_stats"
83-
role="reactant"
84-
isCollapsed={isCollapsed}
85-
/>
86-
<ChartView
87-
uniqueId="productsFrequency"
88-
title="Frequency of Products"
89-
apiCall="product_stats"
90-
role="product"
91-
isCollapsed={isCollapsed}
92-
/>
79+
{datasetId && (
80+
<>
81+
<ChartView
82+
uniqueId="reactantsFrequency"
83+
title="Frequency of Reactants"
84+
apiCall="input_stats"
85+
role="reactant"
86+
datasetId={datasetId}
87+
isCollapsed={isCollapsed}
88+
/>
89+
<ChartView
90+
uniqueId="productsFrequency"
91+
title="Frequency of Products"
92+
apiCall="product_stats"
93+
role="product"
94+
datasetId={datasetId}
95+
isCollapsed={isCollapsed}
96+
/>
97+
</>
98+
)}
9399
</div>
94100
</div>
95101

0 commit comments

Comments
 (0)