sadpandajoe commented on code in PR #45234:
URL: https://github.com/apache/superset/pull/45234#discussion_r4281984436
##########
superset-frontend/src/components/Chart/ChartRenderer.tsx:
##########
@@ -272,14 +272,44 @@ function ChartRendererComponent({
prevQueriesResponseRef.current = queriesResponse;
}, [queriesResponse]);
+ const currentFormData =
+ chartIsStale && latestQueryFormData ? latestQueryFormData : formData;
+ const vizType = currentFormData.viz_type || propVizType;
+ // Some charts fetch their own data and issue no top-level query (e.g.
deck.gl
+ // Multiple Layers, which renders its sub-layer charts), so an empty response
+ // is expected and shouldn't trigger the no-results state. Honor the chart's
+ // own enableNoResults metadata (defaults to true).
+ const chartEnableNoResults =
+ getChartMetadataRegistry().get(vizType)?.enableNoResults ?? true;
+
+ // A failed load leaves its errored response in the store, and the next
+ // CHART_UPDATE_STARTED clears chartAlert but not queriesResponse. Checking
+ // the response itself keeps that stale error from being drawn as chart data
+ // while the retry is in flight.
+ const hasAnyErrors = queriesResponse?.some(item => item?.error);
+ // The response of a chart that fetches its own data stays empty once it has
+ // loaded, so for those any response that landed counts as previous data.
+ const hasPreviousResponse = chartEnableNoResults
+ ? (queriesResponse?.length ?? 0) > 0
+ : Array.isArray(queriesResponse);
Review Comment:
With `enableNoResults: false`, an empty `[]` response now counts as previous
data, so deck.gl Multiple Layers stays mounted through every auto-refresh tick.
Its layers are only fetched in the `useEffect` in `Multi.tsx` when
`deck_slices` or the layer-visibility filter changes, so with an unchanged
config the refresh never re-requests them. Before this change the guard
returned `null` here, which unmounted the plugin and made the remount reload
its layers. Concretely, a dashboard with a refresh interval and a Multiple
Layers chart whose underlying rows change would keep showing the first-loaded
layers while the refresh reports success. Is that intended, or does `DeckMulti`
need a refresh signal (for example reacting to the new response or
`isRefreshing`) so the layers reload?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
