perf: add lazy loading along react-router routes and router links in menu (#13087)

superset-frontend/spec/javascripts/components/Menu_spec.jsx

@@ -17,11 +17,12 @@
  * under the License.
  */
 import React from 'react';
-import { shallow, mount } from 'enzyme';
+import { styledMount as mount } from 'spec/helpers/theming';
+import { shallow } from 'enzyme';
 import { Nav } from 'react-bootstrap';
 import { Menu as DropdownMenu } from 'src/common/components';
 import NavDropdown from 'src/components/NavDropdown';
-import { supersetTheme, ThemeProvider } from '@superset-ui/core';
+import { Link } from 'react-router-dom';
 
 import { Menu } from 'src/components/Menu/Menu';
 import MenuObject from 'src/components/Menu/MenuObject';
@@ -44,7 +45,7 @@ const defaultProps = {
             name: 'Datasets',
             icon: 'fa-table',
             label: 'Datasets',
-            url: '/tablemodelview/list/?_flt_1_is_sqllab_view=y',
+            url: '/tablemodelview/list/',
           },
           '-',
           {
@@ -172,10 +173,7 @@ describe('Menu', () => {
       ...overrideProps,
     };
 
-    const versionedWrapper = mount(<Menu {...props} />, {
-      wrappingComponent: ThemeProvider,
-      wrappingComponentProps: { theme: supersetTheme },
-    });
+    const versionedWrapper = mount(<Menu {...props} />);
 
     expect(versionedWrapper.find('.version-info span')).toHaveLength(2);
   });
@@ -187,4 +185,28 @@ describe('Menu', () => {
   it('renders MenuItems in NavDropdown (settings)', () => {
     expect(wrapper.find(NavDropdown).find(DropdownMenu.Item)).toHaveLength(3);
   });
+
+  it('renders a react-router Link if isFrontendRoute', () => {
+    const props = {
+      ...defaultProps,
+      isFrontendRoute: jest.fn(() => true),
+    };
+
+    const wrapper2 = mount(<Menu {...props} />);
+
+    expect(props.isFrontendRoute).toHaveBeenCalled();
+    expect(wrapper2.find(Link)).toExist();
+  });
+
+  it('does not render a react-router Link if not isFrontendRoute', () => {
+    const props = {
+      ...defaultProps,
+      isFrontendRoute: jest.fn(() => false),
+    };
+
+    const wrapper2 = mount(<Menu {...props} />);
+
+    expect(props.isFrontendRoute).toHaveBeenCalled();
+    expect(wrapper2.find(Link).exists()).toBe(false);
+  });
 });

superset-frontend/src/components/Menu/Menu.tsx

@@ -21,6 +21,7 @@ import { t, styled } from '@superset-ui/core';
 import { Nav, Navbar, NavItem } from 'react-bootstrap';
 import NavDropdown from 'src/components/NavDropdown';
 import { Menu as DropdownMenu } from 'src/common/components';
+import { Link } from 'react-router-dom';
 import MenuObject, {
   MenuObjectProps,
   MenuObjectChildProps,
@@ -57,6 +58,7 @@ export interface MenuProps {
     navbar_right: NavBarProps;
     settings: MenuObjectProps[];
   };
+  isFrontendRoute?: (path?: string) => boolean;
 }
 
 const StyledHeader = styled.header`
@@ -99,7 +101,7 @@ const StyledHeader = styled.header`
     padding-left: 12px;
   }
 
-  .navbar-inverse .navbar-nav > li > a {
+  .navbar-inverse .navbar-nav li a {
     color: ${({ theme }) => theme.colors.grayscale.dark1};
     border-bottom: none;
     transition: background-color ${({ theme }) => theme.transitionTiming}s;
@@ -153,6 +155,7 @@ const StyledHeader = styled.header`
 
 export function Menu({
   data: { menu, brand, navbar_right: navbarRight, settings },
+  isFrontendRoute = () => false,
 }: MenuProps) {
   const [dropdownOpen, setDropdownOpen] = useState(false);
 
@@ -168,9 +171,23 @@ export function Menu({
           <Navbar.Toggle />
         </Navbar.Header>
         <Nav data-test="navbar-top">
-          {menu.map((item, index) => (
-            <MenuObject {...item} key={item.label} index={index + 1} />
-          ))}
+          {menu.map((item, index) => {
+            const props = {
+              ...item,
+              isFrontendRoute: isFrontendRoute(item.url),
+              childs: item.childs?.map(c => {
+                if (typeof c === 'string') {
+                  return c;
+                }
+
+                return {
+                  ...c,
+                  isFrontendRoute: isFrontendRoute(c.url),
+                };
+              }),
+            };
+            return <MenuObject {...props} key={item.label} index={index + 1} />;
+          })}
         </Nav>
         <Nav className="navbar-right">
           {!navbarRight.user_is_anonymous && <NewMenu />}
@@ -192,7 +209,11 @@ export function Menu({
                     if (typeof child !== 'string') {
                       return (
                         <DropdownMenu.Item key={`${child.label}`}>
-                          <a href={child.url}>{child.label}</a>
+                          {isFrontendRoute(child.url) ? (
+                            <Link to={child.url || ''}>{child.label}</Link>
+                          ) : (
+                            <a href={child.url}>{child.label}</a>
+                          )}
                         </DropdownMenu.Item>
                       );
                     }
@@ -276,7 +297,7 @@ export function Menu({
 }
 
 // transform the menu data to reorganize components
-export default function MenuWrapper({ data }: MenuProps) {
+export default function MenuWrapper({ data, ...rest }: MenuProps) {
   const newMenuData = {
     ...data,
   };
@@ -322,5 +343,5 @@ export default function MenuWrapper({ data }: MenuProps) {
   newMenuData.menu = cleanedMenu;
   newMenuData.settings = settings;
 
-  return <Menu data={newMenuData} />;
+  return <Menu data={newMenuData} {...rest} />;
 }

superset-frontend/src/components/Menu/MenuObject.tsx

@@ -17,6 +17,7 @@
  * under the License.
  */
 import React, { useState } from 'react';
+import { Link } from 'react-router-dom';
 import { NavItem } from 'react-bootstrap';
 import { Menu } from 'src/common/components';
 import NavDropdown from '../NavDropdown';
@@ -27,13 +28,10 @@ export interface MenuObjectChildProps {
   icon: string;
   index: number;
   url?: string;
+  isFrontendRoute?: boolean;
 }
 
-export interface MenuObjectProps {
-  label?: string;
-  icon?: string;
-  index: number;
-  url?: string;
+export interface MenuObjectProps extends MenuObjectChildProps {
   childs?: (MenuObjectChildProps | string)[];
   isHeader?: boolean;
 }
@@ -43,9 +41,18 @@ export default function MenuObject({
   childs,
   url,
   index,
+  isFrontendRoute,
 }: MenuObjectProps) {
   const [dropdownOpen, setDropdownOpen] = useState(false);
-
+  if (url && isFrontendRoute) {
+    return (
+      <li role="presentation">
+        <Link role="button" to={url}>
+          {label}
+        </Link>
+      </li>
+    );
+  }
   if (url) {
     return (
       <NavItem eventKey={index} href={url}>
@@ -71,7 +78,11 @@ export default function MenuObject({
           if (typeof child !== 'string') {
             return (
               <Menu.Item key={`${child.label}`}>
-                <a href={child.url}>&nbsp; {child.label}</a>
+                {child.isFrontendRoute ? (
+                  <Link to={child.url || ''}>{child.label}</Link>
+                ) : (
+                  <a href={child.url}>{child.label}</a>
+                )}
               </Menu.Item>
             );
           }

superset-frontend/src/views/App.tsx

@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import React from 'react';
+import React, { Suspense } from 'react';
 import { hot } from 'react-hot-loader/root';
 import thunk from 'redux-thunk';
 import { createStore, applyMiddleware, compose, combineReducers } from 'redux';
@@ -27,27 +27,16 @@ import { initFeatureFlags } from 'src/featureFlags';
 import { ThemeProvider } from '@superset-ui/core';
 import { DynamicPluginProvider } from 'src/components/DynamicPlugins';
 import ErrorBoundary from 'src/components/ErrorBoundary';
+import Loading from 'src/components/Loading';
 import Menu from 'src/components/Menu/Menu';
 import FlashProvider from 'src/components/FlashProvider';
-import AlertList from 'src/views/CRUD/alert/AlertList';
-import ExecutionLog from 'src/views/CRUD/alert/ExecutionLog';
-import AnnotationLayersList from 'src/views/CRUD/annotationlayers/AnnotationLayersList';
-import AnnotationList from 'src/views/CRUD/annotation/AnnotationList';
-import ChartList from 'src/views/CRUD/chart/ChartList';
-import CssTemplatesList from 'src/views/CRUD/csstemplates/CssTemplatesList';
-import DashboardList from 'src/views/CRUD/dashboard/DashboardList';
-import DatabaseList from 'src/views/CRUD/data/database/DatabaseList';
-import DatasetList from 'src/views/CRUD/data/dataset/DatasetList';
-import QueryList from 'src/views/CRUD/data/query/QueryList';
-import SavedQueryList from 'src/views/CRUD/data/savedquery/SavedQueryList';
-
-import messageToastReducer from '../messageToasts/reducers';
-import { initEnhancer } from '../reduxUtils';
-import setupApp from '../setup/setupApp';
-import setupPlugins from '../setup/setupPlugins';
-import Welcome from './CRUD/welcome/Welcome';
-import ToastPresenter from '../messageToasts/containers/ToastPresenter';
-import { theme } from '../preamble';
+import { theme } from 'src/preamble';
+import ToastPresenter from 'src/messageToasts/containers/ToastPresenter';
+import setupPlugins from 'src/setup/setupPlugins';
+import setupApp from 'src/setup/setupApp';
+import messageToastReducer from 'src/messageToasts/reducers';
+import { initEnhancer } from 'src/reduxUtils';
+import { routes, isFrontendRoute } from 'src/views/routes';
 
 setupApp();
 setupPlugins();
@@ -77,78 +66,19 @@ const App = () => (
               ReactRouterRoute={Route}
               stringifyOptions={{ encode: false }}
             >
-              <Menu data={menu} />
+              <Menu data={menu} isFrontendRoute={isFrontendRoute} />
               <Switch>
-                <Route path="/superset/welcome/">
-                  <ErrorBoundary>
-                    <Welcome user={user} />
-                  </ErrorBoundary>
-                </Route>
-                <Route path="/dashboard/list/">
-                  <ErrorBoundary>
-                    <DashboardList user={user} />
-                  </ErrorBoundary>
-                </Route>
-                <Route path="/chart/list/">
-                  <ErrorBoundary>
-                    <ChartList user={user} />
-                  </ErrorBoundary>
-                </Route>
-                <Route path="/tablemodelview/list/">
-                  <ErrorBoundary>
-                    <DatasetList user={user} />
-                  </ErrorBoundary>
-                </Route>
-                <Route path="/databaseview/list/">
-                  <ErrorBoundary>
-                    <DatabaseList user={user} />
-                  </ErrorBoundary>
-                </Route>
-                <Route path="/savedqueryview/list/">
-                  <ErrorBoundary>
-                    <SavedQueryList user={user} />
-                  </ErrorBoundary>
-                </Route>
-                <Route path="/csstemplatemodelview/list/">
-                  <ErrorBoundary>
-                    <CssTemplatesList user={user} />
-                  </ErrorBoundary>
-                </Route>
-                <Route path="/annotationlayermodelview/list/">
-                  <ErrorBoundary>
-                    <AnnotationLayersList user={user} />
-                  </ErrorBoundary>
-                </Route>
-                <Route path="/annotationmodelview/:annotationLayerId/annotation/">
-                  <ErrorBoundary>
-                    <AnnotationList user={user} />
-                  </ErrorBoundary>
-                </Route>
-                <Route path="/superset/sqllab/history/">
-                  <ErrorBoundary>
-                    <QueryList user={user} />
-                  </ErrorBoundary>
-                </Route>
-                <Route path="/alert/list/">
-                  <ErrorBoundary>
-                    <AlertList user={user} />
-                  </ErrorBoundary>
-                </Route>
-                <Route path="/report/list/">
-                  <ErrorBoundary>
-                    <AlertList user={user} isReportEnabled />
-                  </ErrorBoundary>
-                </Route>
-                <Route path="/alert/:alertId/log">
-                  <ErrorBoundary>
-                    <ExecutionLog user={user} />
-                  </ErrorBoundary>
-                </Route>
-                <Route path="/report/:alertId/log">
-                  <ErrorBoundary>
-                    <ExecutionLog user={user} isReportEnabled />
-                  </ErrorBoundary>
-                </Route>
+                {routes.map(
+                  ({ path, Component, props = {}, Fallback = Loading }) => (
+                    <Route path={path} key={path}>
+                      <Suspense fallback={<Fallback />}>
+                        <ErrorBoundary>
+                          <Component user={user} {...props} />
+                        </ErrorBoundary>
+                      </Suspense>
+                    </Route>
+                  ),
+                )}
               </Switch>
               <ToastPresenter />
             </QueryParamProvider>

superset-frontend/src/views/menu.tsx

@@ -16,6 +16,9 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+
+// Menu App. Used in views that do not already include the Menu component in the layout.
+// eg, backend rendered views
 import React from 'react';
 import ReactDOM from 'react-dom';
 import { ThemeProvider } from '@superset-ui/core';

superset-frontend/src/views/routes.test.tsx

@@ -0,0 +1,32 @@
+/**
+ * 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 { isFrontendRoute, routes } from './routes';
+
+describe('isFrontendRoute', () => {
+  it('returns true if a route matches', () => {
+    routes.forEach(r => {
+      expect(isFrontendRoute(r.path)).toBe(true);
+    });
+  });
+
+  it('returns false if a route does not match', () => {
+    expect(isFrontendRoute('/non-existent/path/')).toBe(false);
+  });
+});

superset-frontend/src/views/routes.tsx

@@ -0,0 +1,179 @@
+/**
+ * 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 React, { lazy } from 'react';
+
+// not lazy loaded since this is the home page.
+import Welcome from 'src/views/CRUD/welcome/Welcome';
+
+const AnnotationLayersList = lazy(
+  () =>
+    import(
+      /* webpackChunkName: "AnnotationLayersList" */ 'src/views/CRUD/annotationlayers/AnnotationLayersList'
+    ),
+);
+const AlertList = lazy(
+  () =>
+    import(
+      /* webpackChunkName: "AlertList" */ 'src/views/CRUD/alert/AlertList'
+    ),
+);
+const AnnotationList = lazy(
+  () =>
+    import(
+      /* webpackChunkName: "AnnotationList" */ 'src/views/CRUD/annotation/AnnotationList'
+    ),
+);
+const ChartList = lazy(
+  () =>
+    import(
+      /* webpackChunkName: "ChartList" */ 'src/views/CRUD/chart/ChartList'
+    ),
+);
+const CssTemplatesList = lazy(
+  () =>
+    import(
+      /* webpackChunkName: "CssTemplatesList" */ 'src/views/CRUD/csstemplates/CssTemplatesList'
+    ),
+);
+const DashboardList = lazy(
+  () =>
+    import(
+      /* webpackChunkName: "DashboardList" */ 'src/views/CRUD/dashboard/DashboardList'
+    ),
+);
+const DatabaseList = lazy(
+  () =>
+    import(
+      /* webpackChunkName: "DatabaseList" */ 'src/views/CRUD/data/database/DatabaseList'
+    ),
+);
+const DatasetList = lazy(
+  () =>
+    import(
+      /* webpackChunkName: "DatasetList" */ 'src/views/CRUD/data/dataset/DatasetList'
+    ),
+);
+const ExecutionLog = lazy(
+  () =>
+    import(
+      /* webpackChunkName: "ExecutionLog" */ 'src/views/CRUD/alert/ExecutionLog'
+    ),
+);
+const QueryList = lazy(
+  () =>
+    import(
+      /* webpackChunkName: "QueryList" */ 'src/views/CRUD/data/query/QueryList'
+    ),
+);
+const SavedQueryList = lazy(
+  () =>
+    import(
+      /* webpackChunkName: "SavedQueryList" */ 'src/views/CRUD/data/savedquery/SavedQueryList'
+    ),
+);
+
+type Routes = {
+  path: string;
+  Component: React.ComponentType;
+  Fallback?: React.ComponentType;
+  props?: React.ComponentProps<any>;
+}[];
+
+export const routes: Routes = [
+  {
+    path: '/superset/welcome/',
+    Component: Welcome,
+  },
+  {
+    path: '/dashboard/list/',
+    Component: DashboardList,
+  },
+  {
+    path: '/chart/list/',
+    Component: ChartList,
+  },
+  {
+    path: '/tablemodelview/list/',
+    Component: DatasetList,
+  },
+  {
+    path: '/databaseview/list/',
+    Component: DatabaseList,
+  },
+  {
+    path: '/savedqueryview/list/',
+    Component: SavedQueryList,
+  },
+  {
+    path: '/csstemplatemodelview/list/',
+    Component: CssTemplatesList,
+  },
+  {
+    path: '/annotationlayermodelview/list/',
+    Component: AnnotationLayersList,
+  },
+  {
+    path: '/annotationmodelview/:annotationLayerId/annotation/',
+    Component: AnnotationList,
+  },
+  {
+    path: '/superset/sqllab/history/',
+    Component: QueryList,
+  },
+  {
+    path: '/alert/list/',
+    Component: AlertList,
+  },
+  {
+    path: '/report/list/',
+    Component: AlertList,
+    props: {
+      isReportEnabled: true,
+    },
+  },
+  {
+    path: '/alert/:alertId/log/',
+    Component: ExecutionLog,
+  },
+  {
+    path: '/report/:alertId/log/',
+    Component: ExecutionLog,
+    props: {
+      isReportEnabled: true,
+    },
+  },
+];
+
+export const frontEndRoutes = routes
+  .map(r => r.path)
+  .reduce(
+    (acc, curr) => ({
+      ...acc,
+      [curr]: true,
+    }),
+    {},
+  );
+
+export function isFrontendRoute(path?: string) {
+  if (path) {
+    const basePath = path.split(/[?#]/)[0]; // strip out query params and link bookmarks
+    return !!frontEndRoutes[basePath];
+  }
+  return false;
+}

superset/app.py

@@ -72,7 +72,7 @@ def create_app() -> Flask:
 class SupersetIndexView(IndexView):
     @expose("/")
     def index(self) -> FlaskResponse:
-        return redirect("/superset/welcome")
+        return redirect("/superset/welcome/")
 
 
 class SupersetAppInitializer:
@@ -222,7 +222,7 @@ class SupersetAppInitializer:
         #
         if appbuilder.app.config["LOGO_TARGET_PATH"]:
             appbuilder.add_link(
-                "Home", label=__("Home"), href="/superset/welcome",
+                "Home", label=__("Home"), href="/superset/welcome/",
             )
         appbuilder.add_view(
             AnnotationLayerModelView,
@@ -245,7 +245,7 @@ class SupersetAppInitializer:
         appbuilder.add_link(
             "Datasets",
             label=__("Datasets"),
-            href="/tablemodelview/list/?_flt_1_is_sqllab_view=y",
+            href="/tablemodelview/list/",
             icon="fa-table",
             category="Data",
             category_label=__("Data"),
@@ -333,7 +333,7 @@ class SupersetAppInitializer:
             appbuilder.add_link(
                 "Import Dashboards",
                 label=__("Import Dashboards"),
-                href="/superset/import_dashboards",
+                href="/superset/import_dashboards/",
                 icon="fa-cloud-upload",
                 category="Manage",
                 category_label=__("Manage"),
@@ -342,7 +342,7 @@ class SupersetAppInitializer:
         appbuilder.add_link(
             "SQL Editor",
             label=_("SQL Editor"),
-            href="/superset/sqllab",
+            href="/superset/sqllab/",
             category_icon="fa-flask",
             icon="fa-flask",
             category="SQL Lab",
@@ -350,14 +350,14 @@ class SupersetAppInitializer:
         )
         appbuilder.add_link(
             __("Saved Queries"),
-            href="/sqllab/my_queries/",
+            href="/savedqueryview/list/",
             icon="fa-save",
             category="SQL Lab",
         )
         appbuilder.add_link(
             "Query Search",
             label=_("Query History"),
-            href="/superset/sqllab/history",
+            href="/superset/sqllab/history/",
             icon="fa-search",
             category_icon="fa-flask",
             category="SQL Lab",
@@ -369,7 +369,7 @@ class SupersetAppInitializer:
             appbuilder.add_link(
                 "Upload a CSV",
                 label=__("Upload a CSV"),
-                href="/csvtodatabaseview/form",
+                href="/csvtodatabaseview/form/",
                 icon="fa-upload",
                 category="Data",
                 category_label=__("Data"),
@@ -384,7 +384,7 @@ class SupersetAppInitializer:
                 appbuilder.add_link(
                     "Upload Excel",
                     label=__("Upload Excel"),
-                    href="/exceltodatabaseview/form",
+                    href="/exceltodatabaseview/form/",
                     icon="fa-upload",
                     category="Data",
                     category_label=__("Data"),

superset/config.py

@@ -198,7 +198,7 @@ APP_ICON = "/static/assets/images/superset-logo-horiz.png"
 APP_ICON_WIDTH = 126
 
 # Uncomment to specify where clicking the logo would take the user
-# e.g. setting it to '/welcome' would take the user to '/superset/welcome'
+# e.g. setting it to '/' would take the user to '/superset/welcome/'
 LOGO_TARGET_PATH = None
 
 # Enables SWAGGER UI for superset openapi spec

superset/views/core.py

@@ -2772,7 +2772,7 @@ class Superset(BaseSupersetView):  # pylint: disable=too-many-public-methods
         )
 
     @event_logger.log_this
-    @expose("/welcome")
+    @expose("/welcome/")
     def welcome(self) -> FlaskResponse:
         """Personalized welcome page"""
         if not g.user or not g.user.get_id():