You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ma...@apache.org on 2026/04/21 05:12:31 UTC
(superset) 01/01: fix(explore): skip re-fetch when navigating away from /explore (sc-104553)
This is an automated email from the ASF dual-hosted git repository.
mistercrunch pushed a commit to branch fix-save-and-go-to-dashboard
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 1b570c1d9a0e40307ef7c04d135ef0bb8bedcc6d
Author: Maxime Beauchemin <ma...@gmail.com>
AuthorDate: Tue Apr 21 05:11:57 2026 +0000
fix(explore): skip re-fetch when navigating away from /explore (sc-104553)
The history.listen in Chart/index.tsx was firing loadExploreData for every
PUSH/POP, including navigations that leave the /explore route (e.g. "Save &
go to dashboard" pushes to /superset/dashboard/:id/). That kicks off an
/api/v1/explore/ request with the destination's URL params while the Explore
page is unmounting, either racing the unmount abort or surfacing a spurious
"Failed to load chart data" toast on the destination dashboard.
Guard the listener so it only re-fetches for navigations that stay inside
the routes where ChartPage is mounted (/explore/* and /superset/explore/p).
Post-save REPLACE with a saveAction is still handled.
Also adds a regression test that navigates from /explore to
/superset/dashboard/5/ and asserts no extra /api/v1/explore/ call is made.
Co-Authored-By: Claude Opus 4.6 <no...@anthropic.com>
---
superset-frontend/src/pages/Chart/Chart.test.tsx | 46 +++++++++++++++++++++---
superset-frontend/src/pages/Chart/index.tsx | 15 +++++++-
2 files changed, 56 insertions(+), 5 deletions(-)
diff --git a/superset-frontend/src/pages/Chart/Chart.test.tsx b/superset-frontend/src/pages/Chart/Chart.test.tsx
index b80ccc32b29..a60e5e10636 100644
--- a/superset-frontend/src/pages/Chart/Chart.test.tsx
+++ b/superset-frontend/src/pages/Chart/Chart.test.tsx
@@ -256,13 +256,13 @@ describe('ChartPage', () => {
window.history.pushState(
{},
'',
- `/?${URL_PARAMS.dashboardPageId.name}=${dashboardPageId}`,
+ `/explore/?${URL_PARAMS.dashboardPageId.name}=${dashboardPageId}`,
);
const { getByTestId } = render(
<>
<Link
to={{
- pathname: '/',
+ pathname: '/explore/',
search: `?${URL_PARAMS.dashboardPageId.name}=${dashboardPageId}`,
state: { saveAction: 'overwrite' },
}}
@@ -319,7 +319,7 @@ describe('ChartPage', () => {
});
render(
<>
- <Link to="/?slice_id=99">Navigate away</Link>
+ <Link to="/explore/?slice_id=99">Navigate away</Link>
<ChartPage />
</>,
{
@@ -358,6 +358,44 @@ describe('ChartPage', () => {
JSON.stringify({ show_cell_bars: true }).slice(1, -1),
);
});
+
+ test('does not re-fetch explore data when navigating off the /explore route (e.g. "Save & go to dashboard")', async () => {
+ // Regression test for sc-104553: "Save & go to dashboard" calls
+ // history.push('/superset/dashboard/:id/'). The history.listen in
+ // ChartPage previously fired loadExploreData for that PUSH, starting a
+ // bogus /api/v1/explore/ request with the dashboard URL's params while
+ // the page was in the middle of unmounting.
+ const exploreApiRoute = 'glob:*/api/v1/explore/*';
+ const exploreFormData = getExploreFormData({
+ viz_type: VizType.Table,
+ show_cell_bars: true,
+ });
+ fetchMock.get(exploreApiRoute, {
+ result: { dataset: { id: 1 }, form_data: exploreFormData },
+ });
+ render(
+ <>
+ <Link to="/superset/dashboard/5/">Go to dashboard</Link>
+ <ChartPage />
+ </>,
+ {
+ useRouter: true,
+ useRedux: true,
+ useDnd: true,
+ },
+ );
+ // Initial mount fetches once.
+ await waitFor(() =>
+ expect(fetchMock.callHistory.calls(exploreApiRoute).length).toBe(1),
+ );
+
+ fireEvent.click(screen.getByText('Go to dashboard'));
+
+ // A PUSH to a non-/explore path must not trigger another explore fetch.
+ // Give any stray async work a chance to run before asserting.
+ await new Promise(resolve => setTimeout(resolve, 0));
+ expect(fetchMock.callHistory.calls(exploreApiRoute).length).toBe(1);
+ });
});
test('does not show error toast when request is aborted on unmount', async () => {
@@ -409,7 +447,7 @@ describe('ChartPage', () => {
render(
<>
- <Link to="/?slice_id=99">Navigate</Link>
+ <Link to="/explore/?slice_id=99">Navigate</Link>
<ChartPage />
</>,
{
diff --git a/superset-frontend/src/pages/Chart/index.tsx b/superset-frontend/src/pages/Chart/index.tsx
index d1e3f7b8f75..5546390be05 100644
--- a/superset-frontend/src/pages/Chart/index.tsx
+++ b/superset-frontend/src/pages/Chart/index.tsx
@@ -300,15 +300,28 @@ export default function ExplorePage() {
// PUSH/POP: full reload (unmount + re-fetch).
// REPLACE with saveAction state: re-fetch without unmount (keeps chart visible).
// Other REPLACE: ignored (URL sync from updateHistory).
+ // Navigations that leave the /explore route (e.g. "Save & go to dashboard"
+ // pushes /superset/dashboard/:id/) must not trigger a re-fetch here: the
+ // Explore page is about to unmount, and fetching /api/v1/explore/ with the
+ // destination's params starts a request that either races the unmount abort
+ // or surfaces a spurious error toast on the destination page.
useEffect(() => {
const unlisten = history.listen((loc: Location, action: Action) => {
+ // ChartPage is mounted at /explore/ and /superset/explore/p in
+ // routes.tsx; only re-fetch for navigations that stay inside those.
+ const isExploreRoute =
+ loc.pathname.startsWith('/explore') ||
+ loc.pathname.startsWith('/superset/explore');
const saveAction = (loc.state as Record<string, unknown>)?.saveAction as
| SaveActionType
| undefined;
if (action === 'PUSH' || action === 'POP') {
+ if (!isExploreRoute) {
+ return;
+ }
setIsLoaded(false);
loadExploreData(loc, saveAction);
- } else if (saveAction) {
+ } else if (saveAction && isExploreRoute) {
loadExploreData(loc, saveAction);
}
});