EnxDev commented on code in PR #45234:
URL: https://github.com/apache/superset/pull/45234#discussion_r4255791211
##########
superset-frontend/src/components/Chart/ChartRenderer.tsx:
##########
@@ -272,14 +272,26 @@ function ChartRendererComponent({
prevQueriesResponseRef.current = queriesResponse;
}, [queriesResponse]);
+ const hasAnyErrors = queriesResponse?.some(item => item?.error);
Review Comment:
I don't think this state can happen, so I'm leaving the check as it is.
A chart only reaches `success` or `rendered` after a 200 from the chart data
API, and `ChartDataCommand` raises `ChartDataQueryFailedError` as soon as any
query in the payload carries an error
(`superset/commands/chart/data/get_data_command.py`). The whole request fails,
the chart goes to `failed`, and the store gets a single-item error response.
The one exception is `result_type=query`, which the View query modal uses and
which never feeds the chart. So there's no response where the first query has
data and the second has an error, and `some(item => item?.error)` always agrees
with `queriesResponse[0].error` here.
The check isn't new in this PR either. It came in with #37459 and I only
moved it up a few lines so the refresh guard and the clone can share it. If a
partial-error response ever did show up, not drawing a chart from it during the
refresh is the safer side.
##########
superset-frontend/src/components/Chart/ChartRenderer.tsx:
##########
@@ -272,14 +272,26 @@ function ChartRendererComponent({
prevQueriesResponseRef.current = queriesResponse;
}, [queriesResponse]);
+ const hasAnyErrors = queriesResponse?.some(item => item?.error);
+ const hasValidPreviousData =
+ (queriesResponse?.length ?? 0) > 0 && !hasAnyErrors;
Review Comment:
Confirmed and fixed in 154a25261d.
deck.gl Multiple Layers builds an empty `queries` array, so its response is
`[]`. The guard read that as missing previous data and unmounted the map on
every refresh tick, and the map then loaded all of its layers again when the
query came back. The guard has behaved this way since #37459, but it's the same
symptom this PR is about, so the fix belongs here.
Charts whose metadata sets `enableNoResults: false` now count any response
that landed as previous data. Every other chart still needs a non-empty
response, so a regular chart can't fall back to the no-results state in the
middle of a refresh.
`ChartRenderer.refresh.test.tsx` covers it with a chart that issues no
query: it has to stay mounted through `loading` and `success` without
remounting. Two more tests pin the cases that must still render nothing: a
self-fetching chart whose first query hasn't returned, and a regular chart with
an empty response.
--
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]
