sadpandajoe commented on code in PR #39506:
URL: https://github.com/apache/superset/pull/39506#discussion_r3834990047
##########
superset-frontend/src/pages/Chart/index.tsx:
##########
@@ -312,8 +312,18 @@ export default function ExplorePage() {
// Other REPLACE: ignored (URL sync from updateHistory).
// Entries holding a chart state of the loaded chart are skipped: Explore
// pushed them itself, and ExploreViewContainer restores a popped one in
place.
+ // 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');
Review Comment:
This still treats `/explore-anything` and stale `/superset/explore...` paths
as Explore even though the router only mounts Chart at `/explore/` and
`/explore/p`. Navigating to one of those non-Chart paths will reintroduce the
unwanted request during unmount; could this use the route constants or matcher
instead?
--
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]
