diff --git a/.changeset/brave-eels-allow.md b/.changeset/brave-eels-allow.md new file mode 100644 index 0000000000..8fbe7ba8d2 --- /dev/null +++ b/.changeset/brave-eels-allow.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-app-api': minor +--- + +Updated the React Router wiring to make use of the new `basename` property of the router components in React Router v6 stable. To implement this, a new optional `basename` property has been added to the `Router` app component, which can be forwarded to the concrete router implementation in order to support this new behavior. This is done by default in any app that does not have a `Router` component override. diff --git a/.changeset/eleven-pets-sneeze.md b/.changeset/eleven-pets-sneeze.md new file mode 100644 index 0000000000..7c0d8147cc --- /dev/null +++ b/.changeset/eleven-pets-sneeze.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-plugin-api': minor +--- + +The app `Router` component now accepts an optional `basename` property. diff --git a/.changeset/shaggy-colts-watch.md b/.changeset/shaggy-colts-watch.md new file mode 100644 index 0000000000..67003bd2d3 --- /dev/null +++ b/.changeset/shaggy-colts-watch.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-components': patch +--- + +Disable base path workaround in `Link` component when React Router v6 stable is used. diff --git a/packages/core-app-api/api-report.md b/packages/core-app-api/api-report.md index 4d1db6fde1..70312a54a5 100644 --- a/packages/core-app-api/api-report.md +++ b/packages/core-app-api/api-report.md @@ -144,7 +144,9 @@ export type AppComponents = { NotFoundErrorPage: ComponentType<{}>; BootErrorPage: ComponentType; Progress: ComponentType<{}>; - Router: ComponentType<{}>; + Router: ComponentType<{ + basename?: string; + }>; ErrorBoundaryFallback: ComponentType; ThemeProvider?: ComponentType<{}>; SignInPage?: ComponentType; diff --git a/packages/core-app-api/src/app/AppManager.compat.test.tsx b/packages/core-app-api/src/app/AppManager.compat.test.tsx new file mode 100644 index 0000000000..bdebd79dc4 --- /dev/null +++ b/packages/core-app-api/src/app/AppManager.compat.test.tsx @@ -0,0 +1,107 @@ +/* + * Copyright 2020 The Backstage Authors + * + * 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 tlr, { render } from '@testing-library/react'; +import React from 'react'; + +describe.each(['beta', 'stable'])('react-router %s', rrVersion => { + beforeAll(() => { + jest.doMock('react', () => React); + // This has some side effects, so need this to be stable to avoid re-require + jest.doMock('@testing-library/react', () => tlr); + jest.doMock('react-router', () => + rrVersion === 'beta' + ? jest.requireActual('react-router-beta') + : jest.requireActual('react-router-stable'), + ); + jest.doMock('react-router-dom', () => + rrVersion === 'beta' + ? jest.requireActual('react-router-dom-beta') + : jest.requireActual('react-router-dom-stable'), + ); + }); + + afterAll(() => { + jest.resetModules(); + }); + + function requireDeps() { + return { + ...(require('./AppManager') as typeof import('./AppManager')), + ...(require('../routing') as typeof import('../routing')), + ...(require('react-router-dom') as typeof import('react-router-dom')), + ...(require('@backstage/test-utils') as typeof import('@backstage/test-utils')), + }; + } + + describe('AppManager', () => { + it('supports base path', async () => { + const { AppManager, MemoryRouter, Navigate, Route, FlatRoutes } = + requireDeps(); + const app = new AppManager({ + apis: [], + defaultApis: [], + themes: [ + { + id: 'light', + title: 'Light Theme', + variant: 'light', + Provider: ({ children }) => <>{children}, + }, + ], + icons: {} as any, + plugins: [], + components: { + NotFoundErrorPage: () => null, + BootErrorPage: () => null, + Progress: () => null, + Router: ({ children, basename }) => ( + + ), + ErrorBoundaryFallback: () => null, + ThemeProvider: ({ children }) => <>{children}, + }, + configLoader: async () => [ + { + context: 'test', + data: { app: { baseUrl: 'http://localhost/foo' } }, + }, + ], + bindRoutes: () => {}, + }); + + const AppProvider = app.getProvider(); + const AppRouter = app.getRouter(); + + const rendered = render( + + + + } /> + bar} /> + + + , + ); + + await expect(rendered.findByText('bar')).resolves.toBeInTheDocument(); + }); + }); +}); diff --git a/packages/core-app-api/src/app/AppManager.stable.test.tsx b/packages/core-app-api/src/app/AppManager.stable.test.tsx new file mode 100644 index 0000000000..a9948a6070 --- /dev/null +++ b/packages/core-app-api/src/app/AppManager.stable.test.tsx @@ -0,0 +1,118 @@ +/* + * Copyright 2020 The Backstage Authors + * + * 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 { render } from '@testing-library/react'; +import React from 'react'; +import { MemoryRouter, Navigate, Route } from 'react-router-dom'; +import { FlatRoutes } from '../routing'; +import { AppManager } from './AppManager'; +import { AppOptions } from './types'; + +jest.mock('react-router', () => jest.requireActual('react-router-stable')); +jest.mock('react-router-dom', () => + jest.requireActual('react-router-dom-stable'), +); + +const mockAppOptions: AppOptions = { + apis: [], + defaultApis: [], + themes: [ + { + id: 'light', + title: 'Light Theme', + variant: 'light', + Provider: ({ children }) => <>{children}, + }, + ], + icons: {} as any, + plugins: [], + components: { + NotFoundErrorPage: () => null, + BootErrorPage: () => null, + Progress: () => null, + Router: props => , + ErrorBoundaryFallback: () => null, + ThemeProvider: ({ children }) => <>{children}, + }, + configLoader: async () => [], + bindRoutes: () => {}, +}; + +describe('AppManager', () => { + it('supports base path', async () => { + const app = new AppManager({ + ...mockAppOptions, + components: { + ...mockAppOptions.components, + Router: props => , + }, + configLoader: async () => [ + { + context: 'test', + data: { app: { baseUrl: 'http://localhost/foo' } }, + }, + ], + }); + + const AppProvider = app.getProvider(); + const AppRouter = app.getRouter(); + + const rendered = render( + + + + } /> + bar} /> + + + , + ); + + await expect(rendered.findByText('bar')).resolves.toBeInTheDocument(); + }); + + it('supports base path with absolute navigation', async () => { + const app = new AppManager({ + ...mockAppOptions, + components: { + ...mockAppOptions.components, + Router: props => , + }, + configLoader: async () => [ + { + context: 'test', + data: { app: { baseUrl: 'http://localhost/foo' } }, + }, + ], + }); + + const AppProvider = app.getProvider(); + const AppRouter = app.getRouter(); + + const rendered = render( + + + + } /> + bar} /> + + + , + ); + + await expect(rendered.findByText('bar')).resolves.toBeInTheDocument(); + }); +}); diff --git a/packages/core-app-api/src/app/AppManager.tsx b/packages/core-app-api/src/app/AppManager.tsx index 89402476e3..0ca09c0a0c 100644 --- a/packages/core-app-api/src/app/AppManager.tsx +++ b/packages/core-app-api/src/app/AppManager.tsx @@ -99,6 +99,21 @@ const InternalAppContext = createContext<{ * The returned path does not have a trailing slash. */ function getBasePath(configApi: Config) { + if (!isReactRouterBeta()) { + // When using rr v6 stable the base path is handled through the + // basename prop on the router component instead. + return ''; + } + + return readBasePath(configApi); +} + +/** + * Read the configured base path. + * + * The returned path does not have a trailing slash. + */ +function readBasePath(configApi: ConfigApi) { let { pathname } = new URL( configApi.getOptionalString('app.baseUrl') ?? '/', 'http://dummy.dev', // baseUrl can be specified as just a path @@ -361,7 +376,7 @@ export class AppManager implements BackstageApp { const AppRouter = ({ children }: PropsWithChildren<{}>) => { const configApi = useApi(configApiRef); - const basePath = getBasePath(configApi); + const basePath = readBasePath(configApi); const mountPath = `${basePath}/*`; const { routeObjects } = useContext(InternalAppContext); @@ -390,23 +405,43 @@ export class AppManager implements BackstageApp { { signOutTargetUrl: basePath || '/' }, ); + if (isReactRouterBeta()) { + return ( + + + + {children}} /> + + + ); + } + + return ( + + + {children} + + ); + } + + if (isReactRouterBeta()) { return ( - - {children}} /> - + + + {children}} /> + + ); } return ( - + - - {children}} /> - + <>{children} ); diff --git a/packages/core-app-api/src/app/types.ts b/packages/core-app-api/src/app/types.ts index 39bf9c0554..70022534a1 100644 --- a/packages/core-app-api/src/app/types.ts +++ b/packages/core-app-api/src/app/types.ts @@ -69,7 +69,7 @@ export type AppComponents = { NotFoundErrorPage: ComponentType<{}>; BootErrorPage: ComponentType; Progress: ComponentType<{}>; - Router: ComponentType<{}>; + Router: ComponentType<{ basename?: string }>; ErrorBoundaryFallback: ComponentType; ThemeProvider?: ComponentType<{}>; diff --git a/packages/core-components/src/components/Link/Link.test.tsx b/packages/core-components/src/components/Link/Link.test.tsx index 52e687d2e4..d65351d840 100644 --- a/packages/core-components/src/components/Link/Link.test.tsx +++ b/packages/core-components/src/components/Link/Link.test.tsx @@ -107,58 +107,6 @@ describe('', () => { }); }); - describe('resolves a sub-path correctly', () => { - it('when it starts with base path', async () => { - const testString = 'This is test string'; - const linkText = 'Navigate!'; - const configApi = new ConfigReader({ - app: { baseUrl: 'http://localhost:3000/example' }, - }); - - const { getByText } = render( - wrapInTestApp( - - {linkText} - - {testString}

} /> - - , - ), - ); - - expect(() => getByText(testString)).toThrow(); - fireEvent.click(getByText(linkText)); - await waitFor(() => { - expect(getByText(testString)).toBeInTheDocument(); - }); - }); - - it('when it does not start with base path', async () => { - const testString = 'This is test string'; - const linkText = 'Navigate!'; - const configApi = new ConfigReader({ - app: { baseUrl: 'http://localhost:3000/example' }, - }); - - const { getByText } = render( - wrapInTestApp( - - {linkText} - - {testString}

} /> - - , - ), - ); - - expect(() => getByText(testString)).toThrow(); - fireEvent.click(getByText(linkText)); - await waitFor(() => { - expect(getByText(testString)).toBeInTheDocument(); - }); - }); - }); - describe('isExternalUri', () => { it.each([ [true, 'http://'], diff --git a/packages/core-components/src/components/Link/Link.tsx b/packages/core-components/src/components/Link/Link.tsx index 92ac92daad..07bf8fd78f 100644 --- a/packages/core-components/src/components/Link/Link.tsx +++ b/packages/core-components/src/components/Link/Link.tsx @@ -26,6 +26,12 @@ import { LinkProps as RouterLinkProps, } from 'react-router-dom'; import { trimEnd } from 'lodash'; +import { createRoutesFromChildren, Route } from 'react-router-dom'; + +export function isReactRouterBeta(): boolean { + const [obj] = createRoutesFromChildren(} />); + return !obj.index; +} const useStyles = makeStyles( { @@ -79,6 +85,7 @@ const useBasePath = () => { return trimEnd(pathname, '/'); }; +/** @deprecated Remove once we no longer support React Router v6 beta */ export const useResolvedPath = (uri: LinkProps['to']) => { let resolvedPath = String(uri); @@ -125,7 +132,12 @@ export const Link = React.forwardRef( ({ onClick, noTrack, ...props }, ref) => { const classes = useStyles(); const analytics = useAnalytics(); - const to = useResolvedPath(props.to); + + // Adding the base path to URLs breaks react-router v6 stable, so we only + // do it for beta. The react router version won't change at runtime so it is + // fine to ignore the rules of hooks. + // eslint-disable-next-line react-hooks/rules-of-hooks + const to = isReactRouterBeta() ? useResolvedPath(props.to) : props.to; const linkText = getNodeText(props.children) || to; const external = isExternalUri(to); const newWindow = external && !!/^https?:/.exec(to); diff --git a/packages/core-plugin-api/api-report.md b/packages/core-plugin-api/api-report.md index 0738d7bac5..cebf09e908 100644 --- a/packages/core-plugin-api/api-report.md +++ b/packages/core-plugin-api/api-report.md @@ -139,7 +139,9 @@ export type AppComponents = { NotFoundErrorPage: ComponentType<{}>; BootErrorPage: ComponentType; Progress: ComponentType<{}>; - Router: ComponentType<{}>; + Router: ComponentType<{ + basename?: string; + }>; ErrorBoundaryFallback: ComponentType; ThemeProvider?: ComponentType<{}>; SignInPage?: ComponentType;