Merge pull request #2357 from spotify/eide/lighthouse-new-routing-api

lighthouse: Refactor to new routing api

packages/app/src/App.tsx

@@ -31,6 +31,7 @@ import { Router as CatalogRouter } from '@backstage/plugin-catalog';
 import { Router as DocsRouter } from '@backstage/plugin-techdocs';
 import { Router as GraphiQLRouter } from '@backstage/plugin-graphiql';
 import { Router as TechRadarRouter } from '@backstage/plugin-tech-radar';
+import { Router as LighthouseRouter } from '@backstage/plugin-lighthouse';
 import { Router as RegisterComponentRouter } from '@backstage/plugin-register-component';
 import { Route, Routes, Navigate } from 'react-router';
 
@@ -75,6 +76,7 @@ const AppRoutes = () => (
       element={<TechRadarRouter width={1500} height={800} />}
     />
     <Route path="/graphiql" element={<GraphiQLRouter />} />
+    <Route path="/lighthouse/*" element={<LighthouseRouter />} />
     <Route
       path="/register-component"
       element={<RegisterComponentRouter catalogRouteRef={catalogRouteRef} />}

plugins/lighthouse/src/Router.tsx

@@ -0,0 +1,30 @@
+/*
+ * Copyright 2020 Spotify AB
+ *
+ * Licensed 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 from 'react';
+import { Routes, Route } from 'react-router-dom';
+import { rootRouteRef, viewAuditRouteRef, createAuditRouteRef } from './plugin';
+import AuditList from './components/AuditList';
+import AuditView from './components/AuditView';
+import CreateAudit from './components/CreateAudit';
+
+export const Router = () => (
+  <Routes>
+    <Route path={`/${rootRouteRef.path}`} element={<AuditList />} />
+    <Route path={`/${viewAuditRouteRef.path}`} element={<AuditView />} />
+    <Route path={`/${createAuditRouteRef.path}`} element={<CreateAudit />} />
+  </Routes>
+);

plugins/lighthouse/src/components/AuditList/AuditListTable.test.tsx

@@ -58,10 +58,7 @@ describe('AuditListTable', () => {
     if (!website)
       throw new Error('https://anchor.fm must be present in fixture');
     expect(link).toBeInTheDocument();
-    expect(link).toHaveAttribute(
-      'href',
-      `/lighthouse/audit/${website.lastAudit.id}`,
-    );
+    expect(link).toHaveAttribute('href', `/audit/${website.lastAudit.id}`);
   });
 
   it('renders the dates that are available for a given row', () => {

plugins/lighthouse/src/components/AuditList/AuditListTable.tsx

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import React, { FC, useState } from 'react';
+import React, { FC, useState, useEffect } from 'react';
 import { Table, TableColumn, TrendLine, useApi } from '@backstage/core';
 import { Website, lighthouseApiRef } from '../../api';
 import { useInterval } from 'react-use';
@@ -23,8 +23,9 @@ import {
   CATEGORY_LABELS,
   buildSparklinesDataForItem,
 } from '../../utils';
-import { Link } from '@material-ui/core';
+import { Link, generatePath } from 'react-router-dom';
 import AuditStatusIcon from '../AuditStatusIcon';
+import { viewAuditRouteRef } from '../../plugin';
 
 const columns: TableColumn[] = [
   {
@@ -55,6 +56,10 @@ export const AuditListTable: FC<{ items: Website[] }> = ({ items }) => {
   const [websiteState, setWebsiteState] = useState(items);
   const lighthouseApi = useApi(lighthouseApiRef);
 
+  useEffect(() => {
+    setWebsiteState(items);
+  }, [items]);
+
   const runRefresh = (websites: Website[]) => {
     websites.forEach(async website => {
       const response = await lighthouseApi.getWebsiteForAuditId(
@@ -94,7 +99,11 @@ export const AuditListTable: FC<{ items: Website[] }> = ({ items }) => {
 
     return {
       websiteUrl: (
-        <Link href={`/lighthouse/audit/${website.lastAudit.id}`}>
+        <Link
+          to={generatePath(viewAuditRouteRef.path, {
+            id: website.lastAudit.id,
+          })}
+        >
           {website.url}
         </Link>
       ),

plugins/lighthouse/src/components/AuditList/index.test.tsx

@@ -63,7 +63,7 @@ describe('AuditList', () => {
     expect(element).toBeInTheDocument();
   });
 
-  it('renders a link to create a new audit', async () => {
+  it('renders a button to create a new audit', async () => {
     const rendered = render(
       wrapInTestApp(
         <ApiProvider apis={apis}>
@@ -71,12 +71,8 @@ describe('AuditList', () => {
         </ApiProvider>,
       ),
     );
-    const element = await rendered.findByText('Create Audit');
-    expect(element).toBeInTheDocument();
-    expect(element.parentElement).toHaveAttribute(
-      'href',
-      '/lighthouse/create-audit',
-    );
+    const button = await rendered.findByText('Create Audit');
+    expect(button).toBeInTheDocument();
   });
 
   describe('pagination', () => {
@@ -87,7 +83,7 @@ describe('AuditList', () => {
           <ApiProvider apis={apis}>
             <AuditList />
           </ApiProvider>,
-          { routeEntries: ['/lighthouse?page=2'] },
+          { routeEntries: ['?page=2'] },
         ),
       );
       expect(mockFetch).toHaveBeenLastCalledWith(
@@ -137,13 +133,13 @@ describe('AuditList', () => {
             <ApiProvider apis={apis}>
               <AuditList />
             </ApiProvider>,
-            { routeEntries: ['/lighthouse?page=2'] },
+            { routeEntries: ['?page=2'] },
           ),
         );
         const element = await rendered.findByLabelText(/Go to page 1/);
         fireEvent.click(element);
 
-        expect(useNavigate()).toHaveBeenCalledWith(`/lighthouse?page=1`);
+        expect(useNavigate()).toHaveBeenCalledWith(`?page=1`);
       });
     });
   });

plugins/lighthouse/src/components/AuditList/index.tsx

@@ -37,6 +37,7 @@ import { useQuery } from '../../utils';
 import LighthouseSupportButton from '../SupportButton';
 import LighthouseIntro, { LIGHTHOUSE_INTRO_LOCAL_STORAGE } from '../Intro';
 import AuditListTable from './AuditListTable';
+import { createAuditRouteRef } from '../../plugin';
 
 export const LIMIT = 10;
 
@@ -77,7 +78,7 @@ const AuditList: FC<{}> = () => {
             page={page}
             count={pageCount}
             onChange={(_event: Event, newPage: number) => {
-              navigate(`/lighthouse?page=${newPage}`);
+              navigate(`?page=${newPage}`);
             }}
           />
         )}
@@ -111,7 +112,7 @@ const AuditList: FC<{}> = () => {
           <Button
             variant="contained"
             color="primary"
-            href="/lighthouse/create-audit"
+            onClick={() => navigate(createAuditRouteRef.path)}
           >
             Create Audit
           </Button>

plugins/lighthouse/src/components/AuditView/index.test.tsx

@@ -18,15 +18,17 @@
 
 jest.mock('react-router-dom', () => {
   const actual = jest.requireActual('react-router-dom');
+  const mockNavigation = jest.fn();
   return {
     ...actual,
     useParams: jest.fn(() => ({})),
+    useNavigate: jest.fn(() => mockNavigation),
   };
 });
 
 import React from 'react';
 import mockFetch from 'jest-fetch-mock';
-import { render } from '@testing-library/react';
+import { render, fireEvent } from '@testing-library/react';
 import { wrapInTestApp } from '@backstage/test-utils';
 import { ApiRegistry, ApiProvider } from '@backstage/core';
 
@@ -34,11 +36,13 @@ import AuditView from '.';
 import { lighthouseApiRef, LighthouseRestApi, Audit, Website } from '../../api';
 import { formatTime } from '../../utils';
 import * as data from '../../__fixtures__/website-response.json';
+import { act } from 'react-dom/test-utils';
 
 const { useParams }: { useParams: jest.Mock } = jest.requireMock(
   'react-router-dom',
 );
 const websiteResponse = data as Website;
+const { useNavigate } = jest.requireMock('react-router-dom');
 
 describe('AuditView', () => {
   let apis: ApiRegistry;
@@ -70,7 +74,7 @@ describe('AuditView', () => {
     expect(iframe).toHaveAttribute('src', `https://lighthouse/v1/audits/${id}`);
   });
 
-  it('renders a link to create a new audit for this website', async () => {
+  it('renders a button to click to create a new audit for this website', async () => {
     const rendered = render(
       wrapInTestApp(
         <ApiProvider apis={apis}>
@@ -79,13 +83,15 @@ describe('AuditView', () => {
       ),
     );
 
-    const button = await rendered.findByText('Create Audit');
+    const button = await rendered.findByText('Create New Audit');
     expect(button).toBeInTheDocument();
-    expect(button.parentElement).toHaveAttribute(
-      'href',
-      `/lighthouse/create-audit?url=${encodeURIComponent(
-        'https://spotify.com',
-      )}`,
+
+    act(() => {
+      fireEvent.click(button);
+    });
+
+    expect(useNavigate()).toHaveBeenCalledWith(
+      `../../create-audit?url=${encodeURIComponent('https://spotify.com')}`,
     );
   });
 
@@ -151,7 +157,7 @@ describe('AuditView', () => {
         expect(
           rendered.getByText(formatTime(a.timeCreated)).parentElement
             ?.parentElement,
-        ).toHaveAttribute('href', `/lighthouse/audit/${a.id}`);
+        ).toHaveAttribute('href', `/audit/${a.id}`);
       });
     });
   });

plugins/lighthouse/src/components/AuditView/index.tsx

@@ -14,7 +14,13 @@
  * limitations under the License.
  */
 import React, { useState, useEffect, ReactNode, FC } from 'react';
-import { Link, useParams } from 'react-router-dom';
+import {
+  Link,
+  useParams,
+  useNavigate,
+  generatePath,
+  resolvePath,
+} from 'react-router-dom';
 import { useAsync } from 'react-use';
 import {
   makeStyles,
@@ -42,6 +48,7 @@ import { lighthouseApiRef, Website, Audit } from '../../api';
 import AuditStatusIcon from '../AuditStatusIcon';
 import LighthouseSupportButton from '../SupportButton';
 import { formatTime } from '../../utils';
+import { viewAuditRouteRef, createAuditRouteRef } from '../../plugin';
 
 const useStyles = makeStyles({
   contentGrid: {
@@ -75,7 +82,12 @@ const AuditLinkList: FC<AuditLinkListProps> = ({
         button
         component={Link}
         replace
-        to={`/lighthouse/audit/${audit.id}`}
+        to={resolvePath(
+          generatePath(viewAuditRouteRef.path, {
+            id: audit.id,
+          }),
+          '../../',
+        )}
       >
         <ListItemIcon>
           <AuditStatusIcon audit={audit} />
@@ -116,6 +128,7 @@ const ConnectedAuditView: FC<{}> = () => {
   const lighthouseApi = useApi(lighthouseApiRef);
   const params = useParams() as { id: string };
   const classes = useStyles();
+  const navigate = useNavigate();
 
   const { loading, error, value: nextValue } = useAsync(
     async () => await lighthouseApi.getWebsiteForAuditId(params.id),
@@ -154,7 +167,7 @@ const ConnectedAuditView: FC<{}> = () => {
     );
   }
 
-  let createAuditButtonUrl = '/lighthouse/create-audit';
+  let createAuditButtonUrl = createAuditRouteRef.path;
   if (value?.url) {
     createAuditButtonUrl += `?url=${encodeURIComponent(value.url)}`;
   }
@@ -176,9 +189,9 @@ const ConnectedAuditView: FC<{}> = () => {
           <Button
             variant="contained"
             color="primary"
-            href={createAuditButtonUrl}
+            onClick={() => navigate(`../../${createAuditButtonUrl}`)}
           >
-            Create Audit
+            Create New Audit
           </Button>
           <LighthouseSupportButton />
         </ContentHeader>

plugins/lighthouse/src/components/CreateAudit/index.test.tsx

@@ -78,9 +78,7 @@ describe('CreateAudit', () => {
             <CreateAudit />
           </ApiProvider>,
           {
-            routeEntries: [
-              `/lighthouse/create-audit?url=${encodeURIComponent(url)}`,
-            ],
+            routeEntries: [`/create-audit?url=${encodeURIComponent(url)}`],
           },
         ),
       );
@@ -137,7 +135,7 @@ describe('CreateAudit', () => {
 
       await wait(() => expect(rendered.getByLabelText(/URL/)).toBeEnabled());
 
-      expect(useNavigate()).toHaveBeenCalledWith('/lighthouse');
+      expect(useNavigate()).toHaveBeenCalledWith('..');
     });
   });
 

plugins/lighthouse/src/components/CreateAudit/index.tsx

@@ -78,7 +78,7 @@ const CreateAudit: FC<{}> = () => {
           },
         },
       });
-      navigate('/lighthouse');
+      navigate('..');
     } catch (err) {
       errorApi.post(err);
     } finally {
@@ -154,7 +154,7 @@ const CreateAudit: FC<{}> = () => {
                     <Button
                       variant="outlined"
                       color="primary"
-                      href="/lighthouse"
+                      onClick={() => navigate('..')}
                       disabled={submitting}
                     >
                       Cancel

plugins/lighthouse/src/index.ts

@@ -15,4 +15,5 @@
  */
 
 export { plugin } from './plugin';
+export { Router } from './Router';
 export * from './api';

plugins/lighthouse/src/plugin.ts

@@ -14,12 +14,29 @@
  * limitations under the License.
  */
 
-import { createPlugin, createApiFactory, configApiRef } from '@backstage/core';
-import AuditList from './components/AuditList';
-import AuditView from './components/AuditView';
-import CreateAudit from './components/CreateAudit';
+import {
+  createPlugin,
+  createRouteRef,
+  createApiFactory,
+  configApiRef,
+} from '@backstage/core';
 import { lighthouseApiRef, LighthouseRestApi } from './api';
 
+export const rootRouteRef = createRouteRef({
+  path: '',
+  title: 'Lighthouse',
+});
+
+export const viewAuditRouteRef = createRouteRef({
+  path: 'audit/:id',
+  title: 'View Lighthouse Audit',
+});
+
+export const createAuditRouteRef = createRouteRef({
+  path: 'create-audit',
+  title: 'Create Lighthouse Audit',
+});
+
 export const plugin = createPlugin({
   id: 'lighthouse',
   apis: [
@@ -29,9 +46,4 @@ export const plugin = createPlugin({
       factory: ({ configApi }) => LighthouseRestApi.fromConfig(configApi),
     }),
   ],
-  register({ router }) {
-    router.registerRoute('/lighthouse', AuditList);
-    router.registerRoute('/lighthouse/audit/:id', AuditView);
-    router.registerRoute('/lighthouse/create-audit', CreateAudit);
-  },
 });