amaannawab923 commented on code in PR #45522: URL: https://github.com/apache/superset/pull/45522#discussion_r4390639419
########## superset-frontend/src/pages/ExtensionPage/index.tsx: ########## @@ -0,0 +1,105 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { useEffect, useState } from 'react'; +import { useLocation } from 'react-router-dom'; +import { t } from '@apache-superset/core/translation'; +import { Loading } from '@superset-ui/core/components'; +import { findPageByPath, resolvePage, usePages } from 'src/core/pages'; +import ExtensionsLoader from 'src/extensions/ExtensionsLoader'; +import { ErrorAlert } from 'src/components/ErrorMessage/ErrorAlert'; + +/** + * Renders whichever extension page claims the current URL. + * + * Mounted on a catch-all route that every host route outranks, so this is + * reached only for a path the host itself does not serve. Whether an + * extension serves it can only be decided once extensions have finished + * loading, so until then this must not say "not found": the page may be + * moments away. + */ +export default function ExtensionPage() { Review Comment: Fixed in 4a976dcbca, though not the way I described above — we went with the registry rather than waiting for the route change. `derivePage` now falls through to `findPageByPath` instead of to `'home'`, which makes it the third consumer of the definition `isFrontendRoute` (routes.tsx:373-376) and `ExtensionPage` already read, rather than a fourth representation of which paths a page owns. Because the registry answers live, a registration has to be able to change the answer, so there is a module-level `lastPathname` and a re-derive wired to both `onDidRegisterPage` and `onDidUnregisterPage` — the second matters, or a disposed page leaves `getPage()` naming a surface nothing serves. `'extension'` is on the `Page` union as you suggested. Nothing in the tree switches exhaustively over `getPage()` or keys a `Record` by `Page`, so it is additive here and compile-time only for a published consumer. Two things worth recording on this thread: **One contributed page to another still fires nothing**, because both derive `'extension'`. That is the same shape as the bug you reported, and neither this nor a namespaced route fixes it — the union carries a surface, not an identity. It wants an accessor that names the page, and the `pages` namespace looks like the right home since it already owns `ResolvedPage`, whereas `navigation` is payload-free by its own fileoverview. Deliberately left out of this change rather than half-done; happy to take it as a follow-up here or on the SIP thread. **`'home'` is doing two jobs**, per its own doc: the welcome surface, and the fallback for any route not enumerated. So `/roles/`, `/theme/list/` and `/tasks/list/` all report `'home'` today, and that is what made the dashboard-to-page transition fire a confident wrong value rather than nothing. Splitting it is independent of this PR but adjacent enough to mention. Seven tests; five fail against the previous code. -- 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]
