Merge pull request #5802 from thornbill/router-cleanup

Use lazy routes everywhere

package-lock.json

@@ -19,7 +19,6 @@
         "@fontsource/noto-sans-tc": "5.0.19",
         "@jellyfin/libass-wasm": "4.2.1",
         "@jellyfin/sdk": "0.0.0-unstable.202407020501",
-        "@loadable/component": "5.16.4",
         "@mui/icons-material": "5.15.19",
         "@mui/material": "5.15.19",
         "@mui/x-data-grid": "7.6.1",
@@ -4124,27 +4123,6 @@
       "integrity": "sha512-Hcv+nVC0kZnQ3tD9GVu5xSMR4VVYOteQIr/hwFPVEvPdlXqgGEuRjiheChHgdM+JyqdgNcmzZOX/tnl0JOiI7A==",
       "dev": true
     },
-    "node_modules/@loadable/component": {
-      "version": "5.16.4",
-      "resolved": "https://registry.npmjs.org/@loadable/component/-/component-5.16.4.tgz",
-      "integrity": "sha512-fJWxx9b5WHX90QKmizo9B+es2so8DnBthI1mbflwCoOyvzEwxiZ/SVDCTtXEnHG72/kGBdzr297SSIekYtzSOQ==",
-      "license": "MIT",
-      "dependencies": {
-        "@babel/runtime": "^7.12.18",
-        "hoist-non-react-statics": "^3.3.1",
-        "react-is": "^16.12.0"
-      },
-      "engines": {
-        "node": ">=8"
-      },
-      "funding": {
-        "type": "github",
-        "url": "https://github.com/sponsors/gregberge"
-      },
-      "peerDependencies": {
-        "react": "^16.3.0 || ^17.0.0 || ^18.0.0"
-      }
-    },
     "node_modules/@mapbox/node-pre-gyp": {
       "version": "1.0.10",
       "resolved": "https://registry.npmjs.org/@mapbox/node-pre-gyp/-/node-pre-gyp-1.0.10.tgz",
@@ -26986,16 +26964,6 @@
       "integrity": "sha512-Hcv+nVC0kZnQ3tD9GVu5xSMR4VVYOteQIr/hwFPVEvPdlXqgGEuRjiheChHgdM+JyqdgNcmzZOX/tnl0JOiI7A==",
       "dev": true
     },
-    "@loadable/component": {
-      "version": "5.16.4",
-      "resolved": "https://registry.npmjs.org/@loadable/component/-/component-5.16.4.tgz",
-      "integrity": "sha512-fJWxx9b5WHX90QKmizo9B+es2so8DnBthI1mbflwCoOyvzEwxiZ/SVDCTtXEnHG72/kGBdzr297SSIekYtzSOQ==",
-      "requires": {
-        "@babel/runtime": "^7.12.18",
-        "hoist-non-react-statics": "^3.3.1",
-        "react-is": "^16.12.0"
-      }
-    },
     "@mapbox/node-pre-gyp": {
       "version": "1.0.10",
       "resolved": "https://registry.npmjs.org/@mapbox/node-pre-gyp/-/node-pre-gyp-1.0.10.tgz",

package.json

@@ -81,7 +81,6 @@
     "@fontsource/noto-sans-tc": "5.0.19",
     "@jellyfin/libass-wasm": "4.2.1",
     "@jellyfin/sdk": "0.0.0-unstable.202407020501",
-    "@loadable/component": "5.16.4",
     "@mui/icons-material": "5.15.19",
     "@mui/material": "5.15.19",
     "@mui/x-data-grid": "7.6.1",

src/RootApp.tsx

@@ -1,4 +1,3 @@
-import loadable from '@loadable/component';
 import { History } from '@remix-run/router';
 import { QueryClientProvider } from '@tanstack/react-query';
 import { ReactQueryDevtools } from '@tanstack/react-query-devtools';
@@ -8,26 +7,17 @@ import { ApiProvider } from 'hooks/useApi';
 import { WebConfigProvider } from 'hooks/useWebConfig';
 import { queryClient } from 'utils/query/queryClient';
 
-const StableAppRouter = loadable(() => import('./apps/stable/AppRouter'));
-const RootAppRouter = loadable(() => import('./RootAppRouter'));
+import RootAppRouter from 'RootAppRouter';
 
-const RootApp = ({ history }: Readonly<{ history: History }>) => {
-    const layoutMode = localStorage.getItem('layout');
-    const isExperimentalLayout = layoutMode === 'experimental';
-
-    return (
-        <QueryClientProvider client={queryClient}>
-            <ApiProvider>
-                <WebConfigProvider>
-                    {isExperimentalLayout ?
-                        <RootAppRouter history={history} /> :
-                        <StableAppRouter history={history} />
-                    }
-                </WebConfigProvider>
-            </ApiProvider>
-            <ReactQueryDevtools initialIsOpen={false} />
-        </QueryClientProvider>
-    );
-};
+const RootApp = ({ history }: Readonly<{ history: History }>) => (
+    <QueryClientProvider client={queryClient}>
+        <ApiProvider>
+            <WebConfigProvider>
+                <RootAppRouter history={history} />
+            </WebConfigProvider>
+        </ApiProvider>
+        <ReactQueryDevtools initialIsOpen={false} />
+    </QueryClientProvider>
+);
 
 export default RootApp;

src/RootAppRouter.tsx

@@ -4,21 +4,26 @@ import React from 'react';
 import {
     RouterProvider,
     createHashRouter,
-    Outlet
+    Outlet,
+    useLocation
 } from 'react-router-dom';
 
 import { EXPERIMENTAL_APP_ROUTES } from 'apps/experimental/routes/routes';
 import AppHeader from 'components/AppHeader';
 import Backdrop from 'components/Backdrop';
 import { useLegacyRouterSync } from 'hooks/useLegacyRouterSync';
-import { DASHBOARD_APP_ROUTES } from 'apps/dashboard/routes/routes';
+import { DASHBOARD_APP_PATHS, DASHBOARD_APP_ROUTES } from 'apps/dashboard/routes/routes';
 import UserThemeProvider from 'themes/UserThemeProvider';
+import { STABLE_APP_ROUTES } from 'apps/stable/routes/routes';
+
+const layoutMode = localStorage.getItem('layout');
+const isExperimentalLayout = layoutMode === 'experimental';
 
 const router = createHashRouter([
     {
         element: <RootAppLayout />,
         children: [
-            ...EXPERIMENTAL_APP_ROUTES,
+            ...(isExperimentalLayout ? EXPERIMENTAL_APP_ROUTES : STABLE_APP_ROUTES),
             ...DASHBOARD_APP_ROUTES
         ]
     }
@@ -35,10 +40,14 @@ export default function RootAppRouter({ history }: Readonly<{ history: History}>
  * NOTE: The app will crash if these get removed from the DOM.
  */
 function RootAppLayout() {
+    const location = useLocation();
+    const isNewLayoutPath = Object.values(DASHBOARD_APP_PATHS)
+        .some(path => location.pathname.startsWith(`/${path}`));
+
     return (
         <UserThemeProvider>
             <Backdrop />
-            <AppHeader isHidden />
+            <AppHeader isHidden={isExperimentalLayout || isNewLayoutPath} />
 
             <Outlet />
         </UserThemeProvider>

src/apps/dashboard/AppLayout.tsx

@@ -13,23 +13,20 @@ import { useApi } from 'hooks/useApi';
 
 import AppTabs from './components/AppTabs';
 import AppDrawer from './components/drawer/AppDrawer';
+import { DASHBOARD_APP_PATHS } from './routes/routes';
 
 import './AppOverrides.scss';
 
-interface AppLayoutProps {
-    drawerlessPaths: string[]
-}
+const DRAWERLESS_PATHS = [ DASHBOARD_APP_PATHS.MetadataManager ];
 
-const AppLayout: FC<AppLayoutProps> = ({
-    drawerlessPaths
-}) => {
+export const Component: FC = () => {
     const [ isDrawerActive, setIsDrawerActive ] = useState(false);
     const location = useLocation();
     const { user } = useApi();
 
     const isMediumScreen = useMediaQuery((t: Theme) => t.breakpoints.up('md'));
     const isDrawerAvailable = Boolean(user)
-        && !drawerlessPaths.some(path => location.pathname.startsWith(`/${path}`));
+        && !DRAWERLESS_PATHS.some(path => location.pathname.startsWith(`/${path}`));
     const isDrawerOpen = isDrawerActive && isDrawerAvailable;
 
     const onToggleDrawer = useCallback(() => {
@@ -86,5 +83,3 @@ const AppLayout: FC<AppLayoutProps> = ({
         </Box>
     );
 };
-
-export default AppLayout;

src/apps/dashboard/routes/routes.tsx

@@ -1,13 +1,12 @@
 import React from 'react';
 import { RouteObject } from 'react-router-dom';
-import AppLayout from '../AppLayout';
 import ConnectionRequired from 'components/ConnectionRequired';
 import { ASYNC_ADMIN_ROUTES } from './_asyncRoutes';
 import { toAsyncPageRoute } from 'components/router/AsyncRoute';
 import { toViewManagerPageRoute } from 'components/router/LegacyRoute';
 import { LEGACY_ADMIN_ROUTES } from './_legacyRoutes';
 import ServerContentPage from 'components/ServerContentPage';
-import ErrorBoundary from '../../../components/router/ErrorBoundary';
+import ErrorBoundary from 'components/router/ErrorBoundary';
 
 export const DASHBOARD_APP_PATHS = {
     Dashboard: 'dashboard',
@@ -20,7 +19,7 @@ export const DASHBOARD_APP_ROUTES: RouteObject[] = [
         element: <ConnectionRequired isAdminRequired />,
         children: [
             {
-                element: <AppLayout drawerlessPaths={[ DASHBOARD_APP_PATHS.MetadataManager ]} />,
+                lazy: () => import('../AppLayout'),
                 children: [
                     {
                         path: DASHBOARD_APP_PATHS.Dashboard,

src/apps/experimental/AppLayout.tsx

@@ -15,7 +15,7 @@ import AppDrawer, { isDrawerPath } from './components/drawers/AppDrawer';
 
 import './AppOverrides.scss';
 
-const AppLayout = () => {
+export const Component = () => {
     const [ isDrawerActive, setIsDrawerActive ] = useState(false);
     const { user } = useApi();
     const location = useLocation();
@@ -76,5 +76,3 @@ const AppLayout = () => {
         </Box>
     );
 };
-
-export default AppLayout;

src/apps/experimental/routes/asyncRoutes/user.ts

@@ -1,12 +1,7 @@
-import loadable from '@loadable/component';
 import { AsyncRoute, AsyncRouteType } from '../../../../components/router/AsyncRoute';
 
 export const ASYNC_USER_ROUTES: AsyncRoute[] = [
-    {
-        path: 'home.html',
-        // FIXME: The use of tab manager for the home screen breaks when using lazy route handling
-        Component: loadable(() => import('../home'))
-    },
+    { path: 'home.html', page: 'home', type: AsyncRouteType.Experimental },
     { path: 'quickconnect', page: 'quickConnect' },
     { path: 'search.html', page: 'search' },
     { path: 'userprofile.html', page: 'user/userprofile' },

src/apps/experimental/routes/home.tsx

@@ -2,14 +2,13 @@ import React, { useCallback, useEffect, useMemo, useRef } from 'react';
 import { useSearchParams } from 'react-router-dom';
 
 import globalize from '../../../scripts/globalize';
-import LibraryMenu from '../../../scripts/libraryMenu';
 import { clearBackdrop } from '../../../components/backdrop/backdrop';
 import layoutManager from '../../../components/layoutManager';
-import * as mainTabsManager from '../../../components/maintabsmanager';
+import Page from '../../../components/Page';
+
 import '../../../elements/emby-tabs/emby-tabs';
 import '../../../elements/emby-button/emby-button';
 import '../../../elements/emby-scroller/emby-scroller';
-import Page from '../../../components/Page';
 
 type OnResumeOptions = {
     autoFocus?: boolean;
@@ -29,12 +28,14 @@ const Home = () => {
     const [ searchParams ] = useSearchParams();
     const initialTabIndex = parseInt(searchParams.get('tab') ?? '0', 10);
 
+    const libraryMenu = useMemo(async () => ((await import('../../../scripts/libraryMenu')).default), []);
+    const mainTabsManager = useMemo(() => import('../../../components/maintabsmanager'), []);
     const tabController = useRef<ControllerProps | null>();
     const tabControllers = useMemo<ControllerProps[]>(() => [], []);
     const element = useRef<HTMLDivElement>(null);
 
-    const setTitle = () => {
-        LibraryMenu.setTitle(null);
+    const setTitle = async () => {
+        (await libraryMenu).setTitle(null);
     };
 
     const getTabs = () => {
@@ -78,18 +79,6 @@ const Home = () => {
         });
     }, [ tabControllers ]);
 
-    const onViewDestroy = useCallback(() => {
-        if (tabControllers) {
-            tabControllers.forEach(function (t) {
-                if (t.destroy) {
-                    t.destroy();
-                }
-            });
-        }
-
-        tabController.current = null;
-    }, [ tabControllers ]);
-
     const loadTab = useCallback((index: number, previousIndex: number | null) => {
         getTabController(index).then((controller) => {
             const refresh = !controller.refreshed;
@@ -118,19 +107,23 @@ const Home = () => {
         loadTab(newIndex, previousIndex);
     }, [ loadTab, tabControllers ]);
 
-    const onResume = useCallback(() => {
-        setTitle();
+    const onSetTabs = useCallback(async () => {
+        (await mainTabsManager).setTabs(element.current, initialTabIndex, getTabs, getTabContainers, null, onTabChange, false);
+    }, [ initialTabIndex, mainTabsManager, onTabChange ]);
+
+    const onResume = useCallback(async () => {
+        void setTitle();
         clearBackdrop();
 
         const currentTabController = tabController.current;
 
         if (!currentTabController) {
-            mainTabsManager.selectedTabIndex(initialTabIndex);
+            (await mainTabsManager).selectedTabIndex(initialTabIndex);
         } else if (currentTabController?.onResume) {
             currentTabController.onResume({});
         }
         (document.querySelector('.skinHeader') as HTMLDivElement).classList.add('noHomeButtonHeader');
-    }, [ initialTabIndex ]);
+    }, [ initialTabIndex, mainTabsManager ]);
 
     const onPause = useCallback(() => {
         const currentTabController = tabController.current;
@@ -141,13 +134,13 @@ const Home = () => {
     }, []);
 
     useEffect(() => {
-        mainTabsManager.setTabs(element.current, initialTabIndex, getTabs, getTabContainers, null, onTabChange, false);
+        void onSetTabs();
 
-        onResume();
+        void onResume();
         return () => {
             onPause();
         };
-    }, [ initialTabIndex, onPause, onResume, onTabChange, onViewDestroy ]);
+    }, [ onPause, onResume, onSetTabs ]);
 
     return (
         <div ref={element}>

src/apps/experimental/routes/routes.tsx

@@ -8,8 +8,6 @@ import { toViewManagerPageRoute } from 'components/router/LegacyRoute';
 import { toRedirectRoute } from 'components/router/Redirect';
 import ErrorBoundary from 'components/router/ErrorBoundary';
 
-import AppLayout from '../AppLayout';
-
 import { ASYNC_USER_ROUTES } from './asyncRoutes';
 import { LEGACY_PUBLIC_ROUTES, LEGACY_USER_ROUTES } from './legacyRoutes';
 import VideoPage from './video';
@@ -17,7 +15,7 @@ import VideoPage from './video';
 export const EXPERIMENTAL_APP_ROUTES: RouteObject[] = [
     {
         path: '/*',
-        element: <AppLayout />,
+        lazy: () => import('../AppLayout'),
         children: [
             {
                 /* User routes: Any child route of this layout is authenticated */

src/apps/stable/AppRouter.tsx

@@ -1,43 +0,0 @@
-import { History } from '@remix-run/router';
-import React from 'react';
-import { Outlet, RouterProvider, createHashRouter, useLocation } from 'react-router-dom';
-
-import { useLegacyRouterSync } from 'hooks/useLegacyRouterSync';
-import { STABLE_APP_ROUTES } from './routes/routes';
-import Backdrop from 'components/Backdrop';
-import AppHeader from 'components/AppHeader';
-import { DASHBOARD_APP_PATHS, DASHBOARD_APP_ROUTES } from 'apps/dashboard/routes/routes';
-import UserThemeProvider from 'themes/UserThemeProvider';
-
-const router = createHashRouter([{
-    element: <StableAppLayout />,
-    children: [
-        ...STABLE_APP_ROUTES,
-        ...DASHBOARD_APP_ROUTES
-    ]
-}]);
-
-export default function StableAppRouter({ history }: Readonly<{ history: History }>) {
-    useLegacyRouterSync({ router, history });
-
-    return <RouterProvider router={router} />;
-}
-
-/**
- * Layout component that renders legacy components required on all pages.
- * NOTE: The app will crash if these get removed from the DOM.
- */
-function StableAppLayout() {
-    const location = useLocation();
-    const isNewLayoutPath = Object.values(DASHBOARD_APP_PATHS)
-        .some(path => location.pathname.startsWith(`/${path}`));
-
-    return (
-        <UserThemeProvider>
-            <Backdrop />
-            <AppHeader isHidden={isNewLayoutPath} />
-
-            <Outlet />
-        </UserThemeProvider>
-    );
-}

src/apps/stable/routes/routes.tsx

@@ -16,7 +16,7 @@ import { LEGACY_PUBLIC_ROUTES, LEGACY_USER_ROUTES } from './legacyRoutes';
 export const STABLE_APP_ROUTES: RouteObject[] = [
     {
         path: '/*',
-        element: <AppLayout />,
+        Component: AppLayout,
         children: [
             {
                 /* User routes */

src/components/router/AsyncRoute.tsx

@@ -16,8 +16,6 @@ export interface AsyncRoute {
     page?: string
     /** The page type used to load the correct page element. */
     type?: AsyncRouteType
-    /** Override for pages that don't support lazy importing */
-    Component?: React.ComponentType | null;
 }
 
 const importPage = (page: string, type: AsyncRouteType) => {
@@ -34,13 +32,8 @@ const importPage = (page: string, type: AsyncRouteType) => {
 export const toAsyncPageRoute = ({
     path,
     page,
-    Component,
     type = AsyncRouteType.Stable
 }: AsyncRoute): RouteObject => {
-    if (Component) {
-        return { path, Component };
-    }
-
     return {
         path,
         lazy: async () => {