diff --git a/.changeset/afraid-horses-lick.md b/.changeset/afraid-horses-lick.md new file mode 100644 index 0000000000..f2dfcf4ed6 --- /dev/null +++ b/.changeset/afraid-horses-lick.md @@ -0,0 +1,54 @@ +--- +'@backstage/app-defaults': patch +'@backstage/core-app-api': patch +'@backstage/core-components': patch +'@backstage/core-plugin-api': patch +'@backstage/dev-utils': patch +'@backstage/test-utils': patch +'@backstage/plugin-adr': patch +'@backstage/plugin-airbrake': patch +'@backstage/plugin-allure': patch +'@backstage/plugin-api-docs': patch +'@backstage/plugin-azure-devops': patch +'@backstage/plugin-badges': patch +'@backstage/plugin-bazaar': patch +'@backstage/plugin-catalog-graph': patch +'@backstage/plugin-catalog-import': patch +'@backstage/plugin-catalog-react': patch +'@backstage/plugin-catalog': patch +'@backstage/plugin-circleci': patch +'@backstage/plugin-cloudbuild': patch +'@backstage/plugin-code-climate': patch +'@backstage/plugin-code-coverage': patch +'@backstage/plugin-codescene': patch +'@backstage/plugin-cost-insights': patch +'@backstage/plugin-explore': patch +'@backstage/plugin-gcp-projects': patch +'@backstage/plugin-git-release-manager': patch +'@backstage/plugin-github-actions': patch +'@backstage/plugin-gitops-profiles': patch +'@backstage/plugin-graphiql': patch +'@backstage/plugin-home': patch +'@backstage/plugin-jenkins': patch +'@backstage/plugin-kafka': patch +'@backstage/plugin-kubernetes': patch +'@backstage/plugin-lighthouse': patch +'@backstage/plugin-org': patch +'@backstage/plugin-pagerduty': patch +'@backstage/plugin-permission-react': patch +'@backstage/plugin-rollbar': patch +'@backstage/plugin-scaffolder': patch +'@backstage/plugin-search-react': patch +'@backstage/plugin-search': patch +'@backstage/plugin-sentry': patch +'@backstage/plugin-shortcuts': patch +'@backstage/plugin-splunk-on-call': patch +'@backstage/plugin-tech-insights': patch +'@backstage/plugin-techdocs-addons-test-utils': patch +'@backstage/plugin-techdocs-react': patch +'@backstage/plugin-techdocs': patch +'@backstage/plugin-todo': patch +'@backstage/plugin-user-settings': patch +--- + +Updated React Router dependencies to be peer dependencies. diff --git a/.changeset/big-teachers-tell.md b/.changeset/big-teachers-tell.md new file mode 100644 index 0000000000..180a974e5e --- /dev/null +++ b/.changeset/big-teachers-tell.md @@ -0,0 +1,5 @@ +--- +'@backstage/test-utils': minor +--- + +Elements rendered in a test app are no longer wrapped in a `Routes` and `Route` element, as this is not compatible with React Router v6 stable. diff --git a/.changeset/flat-walls-kiss.md b/.changeset/flat-walls-kiss.md new file mode 100644 index 0000000000..ea2d2556ed --- /dev/null +++ b/.changeset/flat-walls-kiss.md @@ -0,0 +1,5 @@ +--- +'@backstage/create-app': patch +--- + +The Backstage packages and plugins have all been updated to support React Router v6 stable. The `create-app` template has not been migrated yet, but if you want to migrate your own app or plugins, check out the [migration guide](https://backstage.io/docs/tutorials/react-router-stable-migration). diff --git a/.changeset/good-avocados-grow.md b/.changeset/good-avocados-grow.md new file mode 100644 index 0000000000..dcc685c36d --- /dev/null +++ b/.changeset/good-avocados-grow.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-app-api': minor +--- + +Updated the routing system to be compatible with React Router v6 stable. diff --git a/.changeset/grumpy-kiwis-juggle.md b/.changeset/grumpy-kiwis-juggle.md new file mode 100644 index 0000000000..e303f8f674 --- /dev/null +++ b/.changeset/grumpy-kiwis-juggle.md @@ -0,0 +1,5 @@ +--- +'@backstage/cli': patch +--- + +Added a new `migrate react-router-deps` command to aid in the migration to React Router v6 stable. diff --git a/.changeset/mean-tomatoes-visit.md b/.changeset/mean-tomatoes-visit.md new file mode 100644 index 0000000000..111063d805 --- /dev/null +++ b/.changeset/mean-tomatoes-visit.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-techdocs': patch +--- + +Updated the `TechDocsReaderPage` to be compatible with React Router v6 stable. diff --git a/.changeset/old-lemons-switch.md b/.changeset/old-lemons-switch.md new file mode 100644 index 0000000000..62f8d099b3 --- /dev/null +++ b/.changeset/old-lemons-switch.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-components': patch +--- + +The `RoutedTabs` component has been updated to be compatible with React Router v6 stable. diff --git a/.changeset/popular-shirts-happen.md b/.changeset/popular-shirts-happen.md new file mode 100644 index 0000000000..137354fe2b --- /dev/null +++ b/.changeset/popular-shirts-happen.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-permission-react': patch +--- + +**DEPRECATION**: The `PermissionedRoute` component has been deprecated in favor of the new `RequirePermission` component. This is because the usage pattern of `PermissionedRoute` is not compatible with React Router v6 stable. + +Embed the type from `react-router` instead of exporting it directly. diff --git a/.changeset/tall-trains-remain.md b/.changeset/tall-trains-remain.md new file mode 100644 index 0000000000..9a94672656 --- /dev/null +++ b/.changeset/tall-trains-remain.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-app-api': patch +--- + +Updated `FlatRoutes` to be compatible with React Router v6 stable. diff --git a/docs/tutorials/react-router-stable-migration.md b/docs/tutorials/react-router-stable-migration.md new file mode 100644 index 0000000000..9636345116 --- /dev/null +++ b/docs/tutorials/react-router-stable-migration.md @@ -0,0 +1,205 @@ +--- +id: react-router-stable-migration +title: React Router 6.0 Migration +description: Guide for how to migrate from React Router v6 beta to React Router v6 stable +--- + +Backstage has for a long time been using `react-router` version `6.0.0-beta.0`. +We adopted this unstable version because v6 had some new features that fit +really well with Backstage, particularly relative routing. Because we jumped on +this early and unstable version, we knew that we would at some point need a +breaking migration to the stable version of `react-router` v6, which is the +point we're at now! + +This migration is required but controlled by each app, meaning that you choose +when you want to migrate your app. There will however be some point in the +future where we drop support for the beta version of `react-router`, at which +time you would be forced to migrate. + +The stable version of React Router v6 brings a number of improvements and bug +fixes. Notably, the way that paths are resolved has been improved, which fixes a +bug where paths like `/catalog` and `/catalog-import` could get confused. + +## Migration + +### Step 1 - Upgrade to Backstage 1.6 + +The first Backstage release to support `react-router` v6 is `1.6`. You should upgrade to this version first before you start migrating. If you are an early bird and want to try out migration before that release, it is also shipped in `1.6.0-next.1`. + +### Step 2 - Move `react-router` to `peerDependencies` + +It's important that only one version of `react-router` is installed in the +project at a time. Similar to how the `react` version is handled, all plugins +and packages now declare a peer dependency on the React Router dependencies, +rather than a direct dependency. The only exception to this is the app package +(in `packages/app/package.json`), which has the direct dependencies that end up +deciding what version of React Router that you are using in your project. + +Your internal packages might specify a dependency on `react-router` or `react-router-dom` in their `package.json`, and it's important that those are converted to `peerDependencies` so that we can control the version of `react-router` in the app `package.json`. + +You can automate this step by running the following command: + +```bash +yarn backstage-cli migrate react-router-deps +``` + +For those interested in doing this manually, apply the below change to all `package.json` files except the one at `packages/app/package.json` or any other app packages. Skip moving any dependencies that don't already exist, and move both `dependencies` and `devDependencies`. + +```diff + dependencies { + ... +- "react-router-dom": "^6.0.0-beta.0", +- "react-router": "^6.0.0-beta.0" + }, + peerDependencies: { + ... ++ "react-router-dom": "6.0.0-beta.0 || ^6.3.0", ++ "react-router": "6.0.0-beta.0 || ^6.3.0" + }, +``` + +### Step 3 - Ensure that your external plugins are updated + +It's important that you also update your external plugins to their latest version as these will have to perform the same `peerDependencies` update. + +During this migration there may be external plugins that need updating. If you encounter any plugins outside of the `@backstage` scope that are incompatible with your installation, make sure to check for an existing issue or raise a new one at the plugin's GitHub repository. + +### Step 4 - Bump the React Router dependencies in your app. + +Now it's time to do the actual migration to the latest version of React Router. At this time of writing that is `6.3.0`, but that is of course a moving target. + +The first step is to modify `packages/app/package.json`: + +```diff +- "react-router": "6.0.0-beta.0", +- "react-router-dom": "6.0.0-beta.0", ++ "react-router": "^6.3.0", ++ "react-router-dom": "^6.3.0", +``` + +In case you happen to have multiple app packages in your project, apply the same change to all those packages. + +Once the change has been made, run `yarn install`, and then `yarn why react-router` to validate the installation. You should see the following line in the log as the only resulting entry: + +``` +=> Found "react-router@6.3.0" +``` + +If you see multiple entries, and especially `=> Found "react-router@6.0.0-beta.0"`, then your dependencies have not yet been fully migrated to support React Router v6 stable. Double check the steps above, using the information that the Yarn `why` command logged. Repeat the same process for `yarn why react-router-dom`. + +If you end up being stuck not being able to move your entire project to stable versions cleanly, then you can use Yarn `"resolutions"` overrides in your root `package.json`. Try to avoid this option as it may lead to hidden breakages at runtime, and verify any plugins that needed the override. A better option is likely to hold off migrating for a while until plugins have had time to be updated. + +### Step 5 - Breaking Changes + +For a new app created with `npx @backstage/create-app`, the above steps are all you need to do. If you have created internal plugins and customizations then be sure to review the breaking changes in the [React Router changelog](https://reactrouter.com/docs/en/v6/upgrading/reach#breaking-updates) and validate all parts of your app. We've summarized the most important breaking changes below. + +## Breaking Changes + +See [changelog](https://reactrouter.com/docs/en/v6/upgrading/reach#breaking-updates) for a full list of breaking changes. Below we highlight a couple of the most important ones. + +### Route paths + +`Route` components must always contain a `path` or `index` prop. + +```tsx + + {/* Invalid */} + } /> + + {/* Valid */} + } /> + + {/* Valid but discouraged due to incompatibility with react-router beta */} + } /> + +``` + +Absolute route paths within each `Routes` element must now match their own location, meaning that the following is invalid: + +```tsx + + + {/* INVALID, must be "/foo/bar" or "bar" */} + + +``` + +### Routes and Route components + +The `Routes` and `Route` component both received a large related breaking changes. It is no longer possible +to have anything but `Route` elements and React fragments be a child of a `Routes` element. This means that +structures like these: + +```tsx + + + ... + +``` + +need to be migrated to this: + +```tsx + + } /> + ... + +``` + +Somewhat related to the `Routes` change, it is no longer possible to render a +`Route` element by itself, outside of a `Routes` wrapper. Previously, rendering +such a `Route` element would cause the contents of its `element` prop to be +rendered instead, but it will now throw an error. + +### `PermissionedRoute` + +Because of the above change, the `PermissionedRoute` component no longer works in all situations with React Router v6 stable. It has been deprecated in favor of the new `RequirePermission` component, which can be placed anywhere in order to perform a permissions check. + +It's crucial that you update to `RequirePermission` at the same time as you update to React Router v6 stable as the `PermissionedRoute` component will no longer function. + +```diff +- } ++ ++ ++ ++ } + /> +``` + +### `` component + +When migrating over to React Router v6 stable, you might also see browser console warnings for the `Navigate` component. This will need to be wrapped up in a `Route` component with the `Navigate` component in the `element` prop. + +```diff +- ++ } /> +``` + +### `NavLink` + +The `NavLink` component no longer has the `activeClassName` and `activeStyle` props. Instead, the `className` and `style` props accept a callback that receives a boolean indicating whether the link is active. + +## For Plugin Authors + +There are a few things to keep in mind when migrating a published plugin. You of course need to make sure that dependencies on React Router are moved to `peerDependencies` as described above. +In addition, you need to make sure that your plugin truly is compatible with both versions of React Router at runtime. To help you achieve that, you can follow these additional guidelines: + +- Bump the version of `react-router` and `react-router-dom` in your own project to use the stable version. Place them in `devDependencies` if your plugin is a single package project. The stable version is more strict, so this is the better baseline to work from. +- Make sure all `Route` elements have a `path` prop. Do not use the new `index` props, as it is not supported by the beta version. Use `path="/"` for the index routes within a `Routes`. +- If you are using `NavLink`, use both the new and old APIs simultaneously, and work around any TypeScript errors. + +## Troubleshooting + +Check the browser console for React Router related error messages. + +Check `yarn.lock` for packages depending on older versions of `react-router`: + +```bash +yarn why react-router +``` diff --git a/microsite/sidebars.json b/microsite/sidebars.json index 04f5623c86..283a87dfbc 100644 --- a/microsite/sidebars.json +++ b/microsite/sidebars.json @@ -325,6 +325,7 @@ "Tutorials": [ "tutorials/journey", "tutorials/quickstart-app-plugin", + "tutorials/react-router-stable-migration", "tutorials/package-role-migration", "tutorials/migrating-away-from-core", "tutorials/configuring-plugin-databases", diff --git a/mkdocs.yml b/mkdocs.yml index e3bd86afdb..0240744824 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -181,6 +181,7 @@ nav: - Deprecations: 'api/deprecations.md' - Tutorials: - Future developer journey: 'tutorials/journey.md' + - React Router 6.0 Migration: 'tutorials/react-router-stable-migration.md' - Package Role Migration: 'tutorials/package-role-migration.md' - Migrating away from @backstage/core: 'tutorials/migrating-away-from-core.md' - Adding Custom Plugin to Existing Monorepo App: 'tutorials/quickstart-app-plugin.md' diff --git a/packages/app-defaults/package.json b/packages/app-defaults/package.json index d115183602..75d37296d0 100644 --- a/packages/app-defaults/package.json +++ b/packages/app-defaults/package.json @@ -39,11 +39,11 @@ "@backstage/plugin-permission-react": "^0.4.5-next.0", "@backstage/theme": "^0.2.16", "@material-ui/core": "^4.12.2", - "@material-ui/icons": "^4.9.1", - "react-router-dom": "6.0.0-beta.0" + "@material-ui/icons": "^4.9.1" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/packages/app/package.json b/packages/app/package.json index c96797dac4..927dc88da2 100644 --- a/packages/app/package.json +++ b/packages/app/package.json @@ -74,8 +74,8 @@ "prop-types": "^15.7.2", "react": "^17.0.2", "react-dom": "^17.0.2", - "react-router": "6.0.0-beta.0", - "react-router-dom": "6.0.0-beta.0", + "react-router": "^6.3.0", + "react-router-dom": "^6.3.0", "react-use": "^17.2.4", "zen-observable": "^0.8.15" }, diff --git a/packages/app/src/App.tsx b/packages/app/src/App.tsx index f5152ffe2e..54eac880ec 100644 --- a/packages/app/src/App.tsx +++ b/packages/app/src/App.tsx @@ -101,7 +101,7 @@ import * as plugins from './plugins'; import { techDocsPage } from './components/techdocs/TechDocsPage'; import { ApacheAirflowPage } from '@backstage/plugin-apache-airflow'; -import { PermissionedRoute } from '@backstage/plugin-permission-react'; +import { RequirePermission } from '@backstage/plugin-permission-react'; import { catalogEntityCreatePermission } from '@backstage/plugin-catalog-common'; const app = createApp({ @@ -157,10 +157,13 @@ const routes = ( > {entityPage} - } + element={ + + + + } /> import('./migrate/packageLintConfigs').then(m => m.command)), ); + + command + .command('react-router-deps') + .description( + 'Migrates the react-router dependencies for all packages to be peer dependencies', + ) + .action( + lazy(() => import('./migrate/reactRouterDeps').then(m => m.command)), + ); } export function registerCommands(program: Command) { diff --git a/packages/cli/src/commands/migrate/reactRouterDeps.ts b/packages/cli/src/commands/migrate/reactRouterDeps.ts new file mode 100644 index 0000000000..3c1ddd293d --- /dev/null +++ b/packages/cli/src/commands/migrate/reactRouterDeps.ts @@ -0,0 +1,60 @@ +/* + * 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 fs from 'fs-extra'; +import { resolve as resolvePath } from 'path'; +import { PackageGraph } from '../../lib/monorepo'; +import { getRoleFromPackage } from '../../lib/role'; + +const REACT_ROUTER_DEPS = ['react-router', 'react-router-dom']; +const REACT_ROUTER_RANGE = '6.0.0-beta.0 || ^6.3.0'; + +export async function command() { + const packages = await PackageGraph.listTargetPackages(); + + await Promise.all( + packages.map(async ({ dir, packageJson }) => { + const role = getRoleFromPackage(packageJson); + if (role === 'frontend') { + console.log(`Skipping ${packageJson.name}`); + return; + } + + let changed = false; + for (const depName of ['dependencies', 'devDependencies'] as const) { + const depsCollection = packageJson[depName]; + if (depsCollection) { + for (const key of Object.keys(depsCollection)) { + if (REACT_ROUTER_DEPS.includes(key)) { + delete depsCollection[key]; + const peerDeps = (packageJson.peerDependencies = + packageJson.peerDependencies ?? {}); + peerDeps[key] = REACT_ROUTER_RANGE; + changed = true; + } + } + } + } + + if (changed) { + console.log(`Updating dependencies for ${packageJson.name}`); + await fs.writeJson(resolvePath(dir, 'package.json'), packageJson, { + spaces: 2, + }); + } + }), + ); +} diff --git a/packages/core-app-api/package.json b/packages/core-app-api/package.json index 1e54443473..7b5e8742fc 100644 --- a/packages/core-app-api/package.json +++ b/packages/core-app-api/package.json @@ -39,14 +39,14 @@ "@backstage/version-bridge": "^1.0.1", "@types/prop-types": "^15.7.3", "prop-types": "^15.7.2", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4", "zen-observable": "^0.8.15", "zod": "^3.11.6" }, "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", @@ -59,7 +59,11 @@ "@types/node": "^16.11.26", "@types/zen-observable": "^0.8.0", "cross-fetch": "^3.1.5", - "msw": "^0.45.0" + "msw": "^0.45.0", + "react-router-beta": "npm:react-router@6.0.0-beta.0", + "react-router-dom-beta": "npm:react-router-dom@6.0.0-beta.0", + "react-router-stable": "npm:react-router@^6.3.0", + "react-router-dom-stable": "npm:react-router-dom@^6.3.0" }, "files": [ "dist", diff --git a/packages/core-app-api/src/app/AppManager.test.tsx b/packages/core-app-api/src/app/AppManager.test.tsx index 467145acfc..c70b43581a 100644 --- a/packages/core-app-api/src/app/AppManager.test.tsx +++ b/packages/core-app-api/src/app/AppManager.test.tsx @@ -518,7 +518,7 @@ describe('Integration Test', () => { , ), ).toThrow( - 'Parameter :thing is duplicated in path /test/:thing/some/:thing', + 'Parameter :thing is duplicated in path test/:thing/some/:thing', ); }); expect(errorLogs).toEqual([ diff --git a/packages/core-app-api/src/app/AppManager.tsx b/packages/core-app-api/src/app/AppManager.tsx index f36d37f796..89402476e3 100644 --- a/packages/core-app-api/src/app/AppManager.tsx +++ b/packages/core-app-api/src/app/AppManager.tsx @@ -58,6 +58,7 @@ import { pluginCollector } from '../plugins/collectors'; import { featureFlagCollector, routingV1Collector, + routingV2Collector, } from '../routing/collectors'; import { RoutingProvider } from '../routing/RoutingProvider'; import { RouteTracker } from '../routing/RouteTracker'; @@ -80,6 +81,7 @@ import { defaultConfigLoader } from './defaultConfigLoader'; import { ApiRegistry } from '../apis/system/ApiRegistry'; import { resolveRouteBindings } from './resolveRouteBindings'; import { BackstageRouteObject } from '../routing/types'; +import { isReactRouterBeta } from './isReactRouterBeta'; type CompatiblePlugin = | BackstagePlugin @@ -223,7 +225,9 @@ export class AppManager implements BackstageApp { root: children, discoverers: [childDiscoverer, routeElementDiscoverer], collectors: { - routing: routingV1Collector, + routing: isReactRouterBeta() + ? routingV1Collector + : routingV2Collector, collectedPlugins: pluginCollector, featureFlags: featureFlagCollector, }, diff --git a/packages/core-app-api/src/app/isReactRouterBeta.test.tsx b/packages/core-app-api/src/app/isReactRouterBeta.test.tsx new file mode 100644 index 0000000000..38fd961e1e --- /dev/null +++ b/packages/core-app-api/src/app/isReactRouterBeta.test.tsx @@ -0,0 +1,38 @@ +/* + * Copyright 2022 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. + */ + +describe.each(['beta', 'stable'])('react-router %s', rrVersion => { + it('should return the correct value for the different version', () => { + jest.isolateModules(() => { + 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'), + ); + + const { isReactRouterBeta } = require('./isReactRouterBeta'); + expect(isReactRouterBeta()).toBe(rrVersion === 'beta'); + }); + }); +}); + +// eslint-disable-next-line jest/no-export +export {}; diff --git a/packages/core-app-api/src/app/isReactRouterBeta.tsx b/packages/core-app-api/src/app/isReactRouterBeta.tsx new file mode 100644 index 0000000000..bef4f8521f --- /dev/null +++ b/packages/core-app-api/src/app/isReactRouterBeta.tsx @@ -0,0 +1,23 @@ +/* + * Copyright 2022 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 React from 'react'; +import { createRoutesFromChildren, Route } from 'react-router-dom'; + +export function isReactRouterBeta(): boolean { + const [obj] = createRoutesFromChildren(} />); + return !obj.index; +} diff --git a/packages/core-app-api/src/routing/FlatRoutes.test.tsx b/packages/core-app-api/src/routing/FlatRoutes.beta.test.tsx similarity index 96% rename from packages/core-app-api/src/routing/FlatRoutes.test.tsx rename to packages/core-app-api/src/routing/FlatRoutes.beta.test.tsx index ca247d46cd..30be04d2d4 100644 --- a/packages/core-app-api/src/routing/FlatRoutes.test.tsx +++ b/packages/core-app-api/src/routing/FlatRoutes.beta.test.tsx @@ -24,6 +24,11 @@ import { AppContextProvider } from '../app/AppContext'; import { FlatRoutes } from './FlatRoutes'; import { TestApiProvider } from '@backstage/test-utils'; +jest.mock('react-router', () => jest.requireActual('react-router-beta')); +jest.mock('react-router-dom', () => + jest.requireActual('react-router-dom-beta'), +); + const mockFeatureFlagsApi = new LocalStorageFeatureFlags(); const Wrapper = ({ children }: { children?: React.ReactNode }) => ( diff --git a/packages/core-app-api/src/routing/FlatRoutes.compat.test.tsx b/packages/core-app-api/src/routing/FlatRoutes.compat.test.tsx new file mode 100644 index 0000000000..5d2ca15672 --- /dev/null +++ b/packages/core-app-api/src/routing/FlatRoutes.compat.test.tsx @@ -0,0 +1,157 @@ +/* + * 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, RenderResult } from '@testing-library/react'; +import React, { ReactNode } from 'react'; +import { LocalStorageFeatureFlags } from '../apis'; +import { featureFlagsApiRef } from '@backstage/core-plugin-api'; +import { AppContext } from '../app'; +import { AppContextProvider } from '../app/AppContext'; + +describe.each(['beta', 'stable'])('FlatRoutes %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(); + jest.resetAllMocks(); + jest.restoreAllMocks(); + jest.clearAllMocks(); + }); + + const mockFeatureFlagsApi = new LocalStorageFeatureFlags(); + + function requireDeps() { + return { + ...(require('./FlatRoutes') as typeof import('./FlatRoutes')), + ...(require('react-router-dom') as typeof import('react-router-dom')), + ...(require('@backstage/test-utils') as typeof import('@backstage/test-utils')), + }; + } + + function makeRouteRenderer(node: ReactNode) { + const { MemoryRouter, TestApiProvider } = requireDeps(); + let rendered: RenderResult | undefined = undefined; + + const Wrapper = ({ children }: { children?: React.ReactNode }) => ( + + {children} + + ); + + return (path: string) => { + const content = ( + + ({ + NotFoundErrorPage: () => <>Not Found, + }), + } as unknown as AppContext + } + > + + + + ); + if (rendered) { + rendered.unmount(); + rendered.rerender(content); + } else { + rendered = render(content); + } + return rendered; + }; + } + + it('renders some routes', () => { + const { Route, FlatRoutes } = requireDeps(); + const renderRoute = makeRouteRenderer( + + a} /> + b} /> + , + ); + expect(renderRoute('/a').getByText('a')).toBeInTheDocument(); + expect(renderRoute('/b').getByText('b')).toBeInTheDocument(); + expect(renderRoute('/c').getByText('Not Found')).toBeInTheDocument(); + expect(renderRoute('/b').queryByText('Not Found')).not.toBeInTheDocument(); + expect(renderRoute('/a').getByText('a')).toBeInTheDocument(); + }); + + it('is not sensitive to ordering and overlapping routes', () => { + const { Route, FlatRoutes } = requireDeps(); + // The '/*' suffixes here are intentional and will be ignored by FlatRoutes + const routes = ( + <> + a-1} /> + a} /> + a-2} /> + + ); + const renderRoute = makeRouteRenderer({routes}); + expect(renderRoute('/a').getByText('a')).toBeInTheDocument(); + expect(renderRoute('/a-1').getByText('a-1')).toBeInTheDocument(); + expect(renderRoute('/a-2').getByText('a-2')).toBeInTheDocument(); + }); + + it('renders children straight as outlets', () => { + const { Route, useOutlet, FlatRoutes } = requireDeps(); + const MyPage = () => { + return <>Outlet: {useOutlet()}; + }; + + const routes = ( + <> + }> + a + + }> + a-b + + }> + b + + }>c + + ); + const renderRoute = makeRouteRenderer({routes}); + expect(renderRoute('/a').getByText('Outlet: a')).toBeInTheDocument(); + expect(renderRoute('/a/b').getByText('Outlet: a-b')).toBeInTheDocument(); + expect(renderRoute('/b').getByText('Outlet: b')).toBeInTheDocument(); + expect(renderRoute('/').getByText('Outlet: c')).toBeInTheDocument(); + expect( + renderRoute('/not-found').queryByText('Outlet: c'), + ).not.toBeInTheDocument(); + expect( + renderRoute('/not-found').getByText('Not Found'), + ).toBeInTheDocument(); + }); +}); diff --git a/packages/core-app-api/src/routing/FlatRoutes.stable.test.tsx b/packages/core-app-api/src/routing/FlatRoutes.stable.test.tsx new file mode 100644 index 0000000000..93297edc4c --- /dev/null +++ b/packages/core-app-api/src/routing/FlatRoutes.stable.test.tsx @@ -0,0 +1,128 @@ +/* + * 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, RenderResult } from '@testing-library/react'; +import React, { ReactNode } from 'react'; +import { MemoryRouter, Route, useOutlet } from 'react-router-dom'; +import { LocalStorageFeatureFlags } from '../apis'; +import { featureFlagsApiRef } from '@backstage/core-plugin-api'; +import { AppContext } from '../app'; +import { AppContextProvider } from '../app/AppContext'; +import { FlatRoutes } from './FlatRoutes'; +import { TestApiProvider } from '@backstage/test-utils'; + +jest.mock('react-router', () => jest.requireActual('react-router-stable')); +jest.mock('react-router-dom', () => + jest.requireActual('react-router-dom-stable'), +); + +const mockFeatureFlagsApi = new LocalStorageFeatureFlags(); +const Wrapper = ({ children }: { children?: React.ReactNode }) => ( + + {children} + +); + +function makeRouteRenderer(node: ReactNode) { + let rendered: RenderResult | undefined = undefined; + return (path: string) => { + const content = ( + + ({ + NotFoundErrorPage: () => <>Not Found, + }), + } as unknown as AppContext + } + > + + + + ); + if (rendered) { + rendered.unmount(); + rendered.rerender(content); + } else { + rendered = render(content); + } + return rendered; + }; +} + +describe('FlatRoutes', () => { + it('renders some routes', () => { + const renderRoute = makeRouteRenderer( + + a} /> + b} /> + , + ); + expect(renderRoute('/a').getByText('a')).toBeInTheDocument(); + expect(renderRoute('/b').getByText('b')).toBeInTheDocument(); + expect(renderRoute('/c').getByText('Not Found')).toBeInTheDocument(); + expect(renderRoute('/b').queryByText('Not Found')).not.toBeInTheDocument(); + expect(renderRoute('/a').getByText('a')).toBeInTheDocument(); + }); + + it('is not sensitive to ordering and overlapping routes', () => { + // The '/*' suffixes here are intentional and will be ignored by FlatRoutes + const routes = ( + <> + a-1} /> + a} /> + a-2} /> + + ); + const renderRoute = makeRouteRenderer({routes}); + expect(renderRoute('/a').getByText('a')).toBeInTheDocument(); + expect(renderRoute('/a-1').getByText('a-1')).toBeInTheDocument(); + expect(renderRoute('/a-2').getByText('a-2')).toBeInTheDocument(); + }); + + it('renders children straight as outlets', () => { + const MyPage = () => { + return <>Outlet: {useOutlet()}; + }; + + const routes = ( + <> + }> + a + + }> + a-b + + }> + b + + }>c + + ); + const renderRoute = makeRouteRenderer({routes}); + expect(renderRoute('/a').getByText('Outlet: a')).toBeInTheDocument(); + expect(renderRoute('/a/b').getByText('Outlet: a-b')).toBeInTheDocument(); + expect(renderRoute('/b').getByText('Outlet: b')).toBeInTheDocument(); + expect(renderRoute('/').getByText('Outlet: c')).toBeInTheDocument(); + expect( + renderRoute('/not-found').queryByText('Outlet: c'), + ).not.toBeInTheDocument(); + expect( + renderRoute('/not-found').getByText('Not Found'), + ).toBeInTheDocument(); + }); +}); diff --git a/packages/core-app-api/src/routing/FlatRoutes.tsx b/packages/core-app-api/src/routing/FlatRoutes.tsx index 02dbc301fb..abb7fe9432 100644 --- a/packages/core-app-api/src/routing/FlatRoutes.tsx +++ b/packages/core-app-api/src/routing/FlatRoutes.tsx @@ -14,9 +14,12 @@ * limitations under the License. */ -import React, { ReactNode } from 'react'; +import React, { ReactNode, useMemo } from 'react'; import { useRoutes } from 'react-router-dom'; import { useApp, useElementFilter } from '@backstage/core-plugin-api'; +import { isReactRouterBeta } from '../app/isReactRouterBeta'; + +let warned = false; type RouteObject = { path: string; @@ -47,9 +50,14 @@ export type FlatRoutesProps = { export const FlatRoutes = (props: FlatRoutesProps): JSX.Element | null => { const app = useApp(); const { NotFoundErrorPage } = app.getComponents(); + const isBeta = useMemo(() => isReactRouterBeta(), []); const routes = useElementFilter(props.children, elements => elements - .getElements<{ path?: string; children: ReactNode }>() + .getElements<{ + path?: string; + element?: ReactNode; + children?: ReactNode; + }>() .flatMap(child => { let path = child.props.path; @@ -59,16 +67,30 @@ export const FlatRoutes = (props: FlatRoutesProps): JSX.Element | null => { } path = path?.replace(/\/\*$/, '') ?? '/'; + let element = isBeta ? child : child.props.element; + if (!isBeta && !element) { + element = child; + if (!warned && process.env.NODE_ENV !== 'test') { + // eslint-disable-next-line no-console + console.warn( + 'DEPRECATION WARNING: All elements within must be of type with an element prop. ' + + 'Existing usages of should be replaced with } />.', + ); + warned = true; + } + } + return [ { + // Each route matches any sub route, except for the explicit root path path, - element: child, + element, children: child.props.children ? [ // These are the children of each route, which we all add in under a catch-all // subroute in order to make them available to `useOutlet` { - path: path === '/' ? '/' : '/*', // The root path must require an exact match + path: path === '/' ? '/' : '*', // The root path must require an exact match element: child.props.children, }, ] @@ -77,19 +99,19 @@ export const FlatRoutes = (props: FlatRoutesProps): JSX.Element | null => { ]; }) // Routes are sorted to work around a bug where prefixes are unexpectedly matched + // TODO(Rugvip): This can be removed once react-router v6 beta is no longer supported .sort((a, b) => b.path.localeCompare(a.path)) - // We make sure all routes have '/*' appended, except '/' - .map(obj => { - obj.path = obj.path === '/' ? '/' : `${obj.path}/*`; - return obj; - }), + .map(obj => ({ ...obj, path: obj.path === '/' ? '/' : `${obj.path}/*` })), ); // TODO(Rugvip): Possibly add a way to skip this, like a noNotFoundPage prop - routes.push({ - element: , - path: '/*', - }); + const withNotFound = [ + ...routes, + { + path: '*', + element: , + }, + ]; - return useRoutes(routes); + return useRoutes(withNotFound); }; diff --git a/packages/core-app-api/src/routing/RouteResolver.test.ts b/packages/core-app-api/src/routing/RouteResolver.beta.test.ts similarity index 98% rename from packages/core-app-api/src/routing/RouteResolver.test.ts rename to packages/core-app-api/src/routing/RouteResolver.beta.test.ts index b068bd5e6d..f99a7b71f3 100644 --- a/packages/core-app-api/src/routing/RouteResolver.test.ts +++ b/packages/core-app-api/src/routing/RouteResolver.beta.test.ts @@ -25,6 +25,11 @@ import { import { RouteResolver } from './RouteResolver'; import { MATCH_ALL_ROUTE } from './collectors'; +jest.mock('react-router', () => jest.requireActual('react-router-beta')); +jest.mock('react-router-dom', () => + jest.requireActual('react-router-dom-beta'), +); + const element = () => null; const rest = { element, caseSensitive: false, children: [MATCH_ALL_ROUTE] }; diff --git a/packages/core-app-api/src/routing/RouteResolver.compat.test.ts b/packages/core-app-api/src/routing/RouteResolver.compat.test.ts new file mode 100644 index 0000000000..ecc854f474 --- /dev/null +++ b/packages/core-app-api/src/routing/RouteResolver.compat.test.ts @@ -0,0 +1,387 @@ +/* + * 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 { + createRouteRef, + createSubRouteRef, + createExternalRouteRef, + ExternalRouteRef, + RouteRef, + SubRouteRef, +} from '@backstage/core-plugin-api'; +import { MATCH_ALL_ROUTE } from './collectors'; + +const element = () => null; +const rest = { element, caseSensitive: false, children: [MATCH_ALL_ROUTE] }; + +const ref1 = createRouteRef({ id: 'rr1' }); +const ref2 = createRouteRef({ id: 'rr2', params: ['x'] }); +const ref3 = createRouteRef({ id: 'rr3', params: ['y'] }); +const subRef1 = createSubRouteRef({ + id: 'srr1', + parent: ref1, + path: '/foo', +}); +const subRef2 = createSubRouteRef({ + id: 'srr2', + parent: ref1, + path: '/foo/:a', +}); +const subRef3 = createSubRouteRef({ + id: 'srr3', + parent: ref2, + path: '/bar', +}); +const subRef4 = createSubRouteRef({ + id: 'srr4', + parent: ref2, + path: '/bar/:a', +}); +const externalRef1 = createExternalRouteRef({ id: 'err1' }); +const externalRef2 = createExternalRouteRef({ + id: 'err2', + optional: true, +}); +const externalRef3 = createExternalRouteRef({ id: 'err3', params: ['x'] }); +const externalRef4 = createExternalRouteRef({ + id: 'err4', + optional: true, + params: ['x'], +}); + +describe.each(['beta', 'stable'])('react-router %s', rrVersion => { + beforeAll(() => { + 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(); + jest.resetAllMocks(); + jest.restoreAllMocks(); + jest.clearAllMocks(); + }); + + it('should not resolve anything with an empty resolver', () => { + const { RouteResolver } = + require('./RouteResolver') as typeof import('./RouteResolver'); + const r = new RouteResolver(new Map(), new Map(), [], new Map(), ''); + + expect(r.resolve(ref1, '/')?.()).toBe(undefined); + expect(r.resolve(ref2, '/')?.({ x: '1x' })).toBe(undefined); + expect(r.resolve(subRef1, '/')?.()).toBe(undefined); + expect(r.resolve(subRef2, '/')?.({ a: '2a' })).toBe(undefined); + expect(r.resolve(subRef3, '/')?.({ x: '3x' })).toBe(undefined); + expect(r.resolve(subRef4, '/')?.({ x: '4x', a: '4a' })).toBe(undefined); + expect(r.resolve(externalRef1, '/')?.()).toBe(undefined); + expect(r.resolve(externalRef2, '/')?.()).toBe(undefined); + expect(r.resolve(externalRef3, '/')?.({ x: '5x' })).toBe(undefined); + expect(r.resolve(externalRef4, '/')?.({ x: '6x' })).toBe(undefined); + }); + + it('should resolve an absolute route', () => { + const { RouteResolver } = + require('./RouteResolver') as typeof import('./RouteResolver'); + const r = new RouteResolver( + new Map([[ref1, 'my-route']]), + new Map(), + [{ routeRefs: new Set([ref1]), path: 'my-route', ...rest }], + new Map(), + '', + ); + + expect(r.resolve(ref1, '/')?.()).toBe('/my-route'); + expect(r.resolve(ref2, '/')?.({ x: '1x' })).toBe(undefined); + expect(r.resolve(subRef1, '/')?.()).toBe('/my-route/foo'); + expect(r.resolve(subRef2, '/')?.({ a: '2a' })).toBe('/my-route/foo/2a'); + expect(r.resolve(subRef3, '/')?.({ x: '3x' })).toBe(undefined); + expect(r.resolve(subRef4, '/')?.({ x: '4x', a: '4a' })).toBe(undefined); + expect(r.resolve(externalRef1, '/')?.()).toBe(undefined); + expect(r.resolve(externalRef2, '/')?.()).toBe(undefined); + expect(r.resolve(externalRef3, '/')?.({ x: '5x' })).toBe(undefined); + expect(r.resolve(externalRef4, '/')?.({ x: '6x' })).toBe(undefined); + }); + + it('should resolve an absolute route and sub route with an app base path', () => { + const { RouteResolver } = + require('./RouteResolver') as typeof import('./RouteResolver'); + const r = new RouteResolver( + new Map([ + [ref2, 'my-parent/:x'], + [ref1, 'my-route'], + ]), + new Map([[ref1, ref2]]), + [ + { + routeRefs: new Set([ref2]), + path: 'my-parent/:x', + ...rest, + children: [ + MATCH_ALL_ROUTE, + { routeRefs: new Set([ref1]), path: 'my-route', ...rest }, + ], + }, + ], + new Map(), + '/base', + ); + + expect(r.resolve(ref1, '/my-parent/1x')?.()).toBe( + '/base/my-parent/1x/my-route', + ); + expect(r.resolve(ref1, '/base/my-parent/1x')?.()).toBe( + '/base/my-parent/1x/my-route', + ); + expect(r.resolve(ref2, '/')?.({ x: '1x' })).toBe('/base/my-parent/1x'); + expect(r.resolve(ref2, '/base')?.({ x: '1x' })).toBe('/base/my-parent/1x'); + expect(r.resolve(ref3, '/')?.({ y: '1y' })).toBe(undefined); + expect(r.resolve(subRef1, '/my-parent/2x')?.()).toBe( + '/base/my-parent/2x/my-route/foo', + ); + expect(r.resolve(subRef1, '/base/my-parent/2x')?.()).toBe( + '/base/my-parent/2x/my-route/foo', + ); + expect(r.resolve(subRef2, '/my-parent/3x')?.({ a: '2a' })).toBe( + '/base/my-parent/3x/my-route/foo/2a', + ); + expect(r.resolve(subRef2, '/base/my-parent/3x')?.({ a: '2a' })).toBe( + '/base/my-parent/3x/my-route/foo/2a', + ); + expect(r.resolve(subRef3, '/')?.({ x: '5x' })).toBe( + '/base/my-parent/5x/bar', + ); + expect(r.resolve(subRef4, '/')?.({ x: '6x', a: '4a' })).toBe( + '/base/my-parent/6x/bar/4a', + ); + expect(r.resolve(externalRef1, '/')?.()).toBe(undefined); + expect(r.resolve(externalRef2, '/')?.()).toBe(undefined); + expect(r.resolve(externalRef3, '/')?.({ x: '5x' })).toBe(undefined); + expect(r.resolve(externalRef4, '/')?.({ x: '6x' })).toBe(undefined); + }); + + it('should resolve an absolute route with a param and with a parent', () => { + const { RouteResolver } = + require('./RouteResolver') as typeof import('./RouteResolver'); + const r = new RouteResolver( + new Map([ + [ref1, 'my-route'], + [ref2, 'my-parent/:x'], + ]), + new Map([[ref2, ref1]]), + [ + { + routeRefs: new Set([ref2]), + path: 'my-parent/:x', + ...rest, + children: [ + MATCH_ALL_ROUTE, + { routeRefs: new Set([ref1]), path: 'my-route', ...rest }, + ], + }, + ], + new Map([ + [externalRef1, ref1], + [externalRef3, ref2], + [externalRef4, subRef3], + ]), + '', + ); + + expect(r.resolve(ref1, '/')?.()).toBe('/my-route'); + expect(r.resolve(ref2, '/')?.({ x: '1x' })).toBe('/my-route/my-parent/1x'); + expect(r.resolve(subRef1, '/')?.()).toBe('/my-route/foo'); + expect(r.resolve(subRef2, '/')?.({ a: '2a' })).toBe('/my-route/foo/2a'); + expect(r.resolve(subRef3, '/')?.({ x: '3x' })).toBe( + '/my-route/my-parent/3x/bar', + ); + expect(r.resolve(subRef4, '/')?.({ x: '4x', a: '4a' })).toBe( + '/my-route/my-parent/4x/bar/4a', + ); + expect(r.resolve(externalRef1, '/')?.()).toBe('/my-route'); + expect(r.resolve(externalRef2, '/')?.()).toBe(undefined); + expect(r.resolve(externalRef3, '/')?.({ x: '5x' })).toBe( + '/my-route/my-parent/5x', + ); + expect(r.resolve(externalRef4, '/')?.({ x: '6x' })).toBe( + '/my-route/my-parent/6x/bar', + ); + }); + + it('should resolve the most specific match', () => { + const { RouteResolver } = + require('./RouteResolver') as typeof import('./RouteResolver'); + const r = new RouteResolver( + new Map([ + [ref1, 'deep'], + [ref2, 'root/:x'], + [ref3, 'sub/:y'], + ]), + new Map([ + [ref3, ref2], + [ref1, ref3], + ]), + [ + { + routeRefs: new Set([ref2]), + path: 'root/:x', + ...rest, + children: [ + MATCH_ALL_ROUTE, + { + routeRefs: new Set([ref3]), + path: 'sub/:y', + ...rest, + children: [ + MATCH_ALL_ROUTE, + { + routeRefs: new Set([ref1]), + path: 'deep', + ...rest, + }, + ], + }, + ], + }, + ], + new Map(), + '', + ); + + expect(r.resolve(ref2, '/')?.({ x: 'x' })).toBe('/root/x'); + expect(r.resolve(ref3, '/root/x')?.({ y: 'y' })).toBe('/root/x/sub/y'); + + expect(() => r.resolve(ref1, '/')?.()).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(() => r.resolve(ref1, '/root/x')?.()).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(ref1, '/root/x/sub/y')?.()).toBe('/root/x/sub/y/deep'); + // Without the MATCH_ALL_ROUTE, we wouldn't properly match the route here + expect(r.resolve(ref1, '/root/x/sub/y/any/nested/path/here')?.()).toBe( + '/root/x/sub/y/deep', + ); + }); + + it('should resolve an absolute route with multiple parents', () => { + const { RouteResolver } = + require('./RouteResolver') as typeof import('./RouteResolver'); + const r = new RouteResolver( + new Map([ + [ref1, 'my-route'], + [ref2, 'my-parent/:x'], + [ref3, 'my-grandparent/:y'], + ]), + new Map([ + [ref1, ref2], + [ref2, ref3], + ]), + [ + { + routeRefs: new Set([ref3]), + path: 'my-grandparent/:y', + ...rest, + children: [ + MATCH_ALL_ROUTE, + { + routeRefs: new Set([ref2]), + path: 'my-parent/:x', + ...rest, + children: [ + MATCH_ALL_ROUTE, + { routeRefs: new Set([ref1]), path: 'my-route', ...rest }, + ], + }, + ], + }, + ], + new Map([ + [externalRef1, ref1], + [externalRef3, ref2], + [externalRef4, subRef3], + ]), + '', + ); + + const l = '/my-grandparent/my-y/my-parent/my-x'; + expect(r.resolve(ref1, l)?.()).toBe( + '/my-grandparent/my-y/my-parent/my-x/my-route', + ); + expect(() => r.resolve(ref1, '/')?.()).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(ref2, l)?.({ x: '1x' })).toBe( + '/my-grandparent/my-y/my-parent/1x', + ); + expect(r.resolve(ref2, '/my-grandparent/my-y')?.({ x: '1x' })).toBe( + '/my-grandparent/my-y/my-parent/1x', + ); + expect(() => r.resolve(ref2, '/')?.({ x: '1x' })).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(subRef1, l)?.()).toBe( + '/my-grandparent/my-y/my-parent/my-x/my-route/foo', + ); + expect(() => r.resolve(subRef1, '/')?.()).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(subRef2, l)?.({ a: '2a' })).toBe( + '/my-grandparent/my-y/my-parent/my-x/my-route/foo/2a', + ); + expect(() => r.resolve(subRef2, '/')?.({ a: '2a' })).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(subRef3, l)?.({ x: '3x' })).toBe( + '/my-grandparent/my-y/my-parent/3x/bar', + ); + expect(r.resolve(subRef3, '/my-grandparent/my-y')?.({ x: '3x' })).toBe( + '/my-grandparent/my-y/my-parent/3x/bar', + ); + expect(r.resolve(subRef4, l)?.({ x: '4x', a: '4a' })).toBe( + '/my-grandparent/my-y/my-parent/4x/bar/4a', + ); + expect( + r.resolve(subRef4, '/my-grandparent/my-y')?.({ x: '4x', a: '4a' }), + ).toBe('/my-grandparent/my-y/my-parent/4x/bar/4a'); + expect(r.resolve(externalRef1, l)?.()).toBe( + '/my-grandparent/my-y/my-parent/my-x/my-route', + ); + expect(() => r.resolve(externalRef1, '/')?.()).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(externalRef2, l)?.()).toBe(undefined); + expect(r.resolve(externalRef3, l)?.({ x: '5x' })).toBe( + '/my-grandparent/my-y/my-parent/5x', + ); + expect(() => r.resolve(externalRef3, '/')?.({ x: '5x' })).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(externalRef4, l)?.({ x: '6x' })).toBe( + '/my-grandparent/my-y/my-parent/6x/bar', + ); + expect(() => r.resolve(externalRef4, '/')?.({ x: '6x' })).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + }); +}); diff --git a/packages/core-app-api/src/routing/RouteResolver.stable.test.ts b/packages/core-app-api/src/routing/RouteResolver.stable.test.ts new file mode 100644 index 0000000000..9cf1fb2fb1 --- /dev/null +++ b/packages/core-app-api/src/routing/RouteResolver.stable.test.ts @@ -0,0 +1,361 @@ +/* + * 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 { + createRouteRef, + createSubRouteRef, + createExternalRouteRef, + ExternalRouteRef, + RouteRef, + SubRouteRef, +} from '@backstage/core-plugin-api'; +import { RouteResolver } from './RouteResolver'; +import { MATCH_ALL_ROUTE } from './collectors'; + +jest.mock('react-router', () => jest.requireActual('react-router-stable')); +jest.mock('react-router-dom', () => + jest.requireActual('react-router-dom-stable'), +); + +const element = () => null; +const rest = { element, caseSensitive: false, children: [MATCH_ALL_ROUTE] }; + +const ref1 = createRouteRef({ id: 'rr1' }); +const ref2 = createRouteRef({ id: 'rr2', params: ['x'] }); +const ref3 = createRouteRef({ id: 'rr3', params: ['y'] }); +const subRef1 = createSubRouteRef({ + id: 'srr1', + parent: ref1, + path: '/foo', +}); +const subRef2 = createSubRouteRef({ + id: 'srr2', + parent: ref1, + path: '/foo/:a', +}); +const subRef3 = createSubRouteRef({ + id: 'srr3', + parent: ref2, + path: '/bar', +}); +const subRef4 = createSubRouteRef({ + id: 'srr4', + parent: ref2, + path: '/bar/:a', +}); +const externalRef1 = createExternalRouteRef({ id: 'err1' }); +const externalRef2 = createExternalRouteRef({ + id: 'err2', + optional: true, +}); +const externalRef3 = createExternalRouteRef({ id: 'err3', params: ['x'] }); +const externalRef4 = createExternalRouteRef({ + id: 'err4', + optional: true, + params: ['x'], +}); + +describe('RouteResolver', () => { + it('should not resolve anything with an empty resolver', () => { + const r = new RouteResolver(new Map(), new Map(), [], new Map(), ''); + + expect(r.resolve(ref1, '/')?.()).toBe(undefined); + expect(r.resolve(ref2, '/')?.({ x: '1x' })).toBe(undefined); + expect(r.resolve(subRef1, '/')?.()).toBe(undefined); + expect(r.resolve(subRef2, '/')?.({ a: '2a' })).toBe(undefined); + expect(r.resolve(subRef3, '/')?.({ x: '3x' })).toBe(undefined); + expect(r.resolve(subRef4, '/')?.({ x: '4x', a: '4a' })).toBe(undefined); + expect(r.resolve(externalRef1, '/')?.()).toBe(undefined); + expect(r.resolve(externalRef2, '/')?.()).toBe(undefined); + expect(r.resolve(externalRef3, '/')?.({ x: '5x' })).toBe(undefined); + expect(r.resolve(externalRef4, '/')?.({ x: '6x' })).toBe(undefined); + }); + + it('should resolve an absolute route', () => { + const r = new RouteResolver( + new Map([[ref1, 'my-route']]), + new Map(), + [{ routeRefs: new Set([ref1]), path: 'my-route', ...rest }], + new Map(), + '', + ); + + expect(r.resolve(ref1, '/')?.()).toBe('/my-route'); + expect(r.resolve(ref2, '/')?.({ x: '1x' })).toBe(undefined); + expect(r.resolve(subRef1, '/')?.()).toBe('/my-route/foo'); + expect(r.resolve(subRef2, '/')?.({ a: '2a' })).toBe('/my-route/foo/2a'); + expect(r.resolve(subRef3, '/')?.({ x: '3x' })).toBe(undefined); + expect(r.resolve(subRef4, '/')?.({ x: '4x', a: '4a' })).toBe(undefined); + expect(r.resolve(externalRef1, '/')?.()).toBe(undefined); + expect(r.resolve(externalRef2, '/')?.()).toBe(undefined); + expect(r.resolve(externalRef3, '/')?.({ x: '5x' })).toBe(undefined); + expect(r.resolve(externalRef4, '/')?.({ x: '6x' })).toBe(undefined); + }); + + it('should resolve an absolute route and sub route with an app base path', () => { + const r = new RouteResolver( + new Map([ + [ref2, 'my-parent/:x'], + [ref1, 'my-route'], + ]), + new Map([[ref1, ref2]]), + [ + { + routeRefs: new Set([ref2]), + path: 'my-parent/:x', + ...rest, + children: [ + MATCH_ALL_ROUTE, + { routeRefs: new Set([ref1]), path: 'my-route', ...rest }, + ], + }, + ], + new Map(), + '/base', + ); + + expect(r.resolve(ref1, '/my-parent/1x')?.()).toBe( + '/base/my-parent/1x/my-route', + ); + expect(r.resolve(ref1, '/base/my-parent/1x')?.()).toBe( + '/base/my-parent/1x/my-route', + ); + expect(r.resolve(ref2, '/')?.({ x: '1x' })).toBe('/base/my-parent/1x'); + expect(r.resolve(ref2, '/base')?.({ x: '1x' })).toBe('/base/my-parent/1x'); + expect(r.resolve(ref3, '/')?.({ y: '1y' })).toBe(undefined); + expect(r.resolve(subRef1, '/my-parent/2x')?.()).toBe( + '/base/my-parent/2x/my-route/foo', + ); + expect(r.resolve(subRef1, '/base/my-parent/2x')?.()).toBe( + '/base/my-parent/2x/my-route/foo', + ); + expect(r.resolve(subRef2, '/my-parent/3x')?.({ a: '2a' })).toBe( + '/base/my-parent/3x/my-route/foo/2a', + ); + expect(r.resolve(subRef2, '/base/my-parent/3x')?.({ a: '2a' })).toBe( + '/base/my-parent/3x/my-route/foo/2a', + ); + expect(r.resolve(subRef3, '/')?.({ x: '5x' })).toBe( + '/base/my-parent/5x/bar', + ); + expect(r.resolve(subRef4, '/')?.({ x: '6x', a: '4a' })).toBe( + '/base/my-parent/6x/bar/4a', + ); + expect(r.resolve(externalRef1, '/')?.()).toBe(undefined); + expect(r.resolve(externalRef2, '/')?.()).toBe(undefined); + expect(r.resolve(externalRef3, '/')?.({ x: '5x' })).toBe(undefined); + expect(r.resolve(externalRef4, '/')?.({ x: '6x' })).toBe(undefined); + }); + + it('should resolve an absolute route with a param and with a parent', () => { + const r = new RouteResolver( + new Map([ + [ref1, 'my-route'], + [ref2, 'my-parent/:x'], + ]), + new Map([[ref2, ref1]]), + [ + { + routeRefs: new Set([ref2]), + path: 'my-parent/:x', + ...rest, + children: [ + MATCH_ALL_ROUTE, + { routeRefs: new Set([ref1]), path: 'my-route', ...rest }, + ], + }, + ], + new Map([ + [externalRef1, ref1], + [externalRef3, ref2], + [externalRef4, subRef3], + ]), + '', + ); + + expect(r.resolve(ref1, '/')?.()).toBe('/my-route'); + expect(r.resolve(ref2, '/')?.({ x: '1x' })).toBe('/my-route/my-parent/1x'); + expect(r.resolve(subRef1, '/')?.()).toBe('/my-route/foo'); + expect(r.resolve(subRef2, '/')?.({ a: '2a' })).toBe('/my-route/foo/2a'); + expect(r.resolve(subRef3, '/')?.({ x: '3x' })).toBe( + '/my-route/my-parent/3x/bar', + ); + expect(r.resolve(subRef4, '/')?.({ x: '4x', a: '4a' })).toBe( + '/my-route/my-parent/4x/bar/4a', + ); + expect(r.resolve(externalRef1, '/')?.()).toBe('/my-route'); + expect(r.resolve(externalRef2, '/')?.()).toBe(undefined); + expect(r.resolve(externalRef3, '/')?.({ x: '5x' })).toBe( + '/my-route/my-parent/5x', + ); + expect(r.resolve(externalRef4, '/')?.({ x: '6x' })).toBe( + '/my-route/my-parent/6x/bar', + ); + }); + + it('should resolve the most specific match', () => { + const r = new RouteResolver( + new Map([ + [ref1, 'deep'], + [ref2, 'root/:x'], + [ref3, 'sub/:y'], + ]), + new Map([ + [ref3, ref2], + [ref1, ref3], + ]), + [ + { + routeRefs: new Set([ref2]), + path: 'root/:x', + ...rest, + children: [ + MATCH_ALL_ROUTE, + { + routeRefs: new Set([ref3]), + path: 'sub/:y', + ...rest, + children: [ + MATCH_ALL_ROUTE, + { + routeRefs: new Set([ref1]), + path: 'deep', + ...rest, + }, + ], + }, + ], + }, + ], + new Map(), + '', + ); + + expect(r.resolve(ref2, '/')?.({ x: 'x' })).toBe('/root/x'); + expect(r.resolve(ref3, '/root/x')?.({ y: 'y' })).toBe('/root/x/sub/y'); + + expect(() => r.resolve(ref1, '/')?.()).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(() => r.resolve(ref1, '/root/x')?.()).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(ref1, '/root/x/sub/y')?.()).toBe('/root/x/sub/y/deep'); + // Without the MATCH_ALL_ROUTE, we wouldn't properly match the route here + expect(r.resolve(ref1, '/root/x/sub/y/any/nested/path/here')?.()).toBe( + '/root/x/sub/y/deep', + ); + }); + + it('should resolve an absolute route with multiple parents', () => { + const r = new RouteResolver( + new Map([ + [ref1, 'my-route'], + [ref2, 'my-parent/:x'], + [ref3, 'my-grandparent/:y'], + ]), + new Map([ + [ref1, ref2], + [ref2, ref3], + ]), + [ + { + routeRefs: new Set([ref3]), + path: 'my-grandparent/:y', + ...rest, + children: [ + MATCH_ALL_ROUTE, + { + routeRefs: new Set([ref2]), + path: 'my-parent/:x', + ...rest, + children: [ + MATCH_ALL_ROUTE, + { routeRefs: new Set([ref1]), path: 'my-route', ...rest }, + ], + }, + ], + }, + ], + new Map([ + [externalRef1, ref1], + [externalRef3, ref2], + [externalRef4, subRef3], + ]), + '', + ); + + const l = '/my-grandparent/my-y/my-parent/my-x'; + expect(r.resolve(ref1, l)?.()).toBe( + '/my-grandparent/my-y/my-parent/my-x/my-route', + ); + expect(() => r.resolve(ref1, '/')?.()).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(ref2, l)?.({ x: '1x' })).toBe( + '/my-grandparent/my-y/my-parent/1x', + ); + expect(r.resolve(ref2, '/my-grandparent/my-y')?.({ x: '1x' })).toBe( + '/my-grandparent/my-y/my-parent/1x', + ); + expect(() => r.resolve(ref2, '/')?.({ x: '1x' })).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(subRef1, l)?.()).toBe( + '/my-grandparent/my-y/my-parent/my-x/my-route/foo', + ); + expect(() => r.resolve(subRef1, '/')?.()).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(subRef2, l)?.({ a: '2a' })).toBe( + '/my-grandparent/my-y/my-parent/my-x/my-route/foo/2a', + ); + expect(() => r.resolve(subRef2, '/')?.({ a: '2a' })).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(subRef3, l)?.({ x: '3x' })).toBe( + '/my-grandparent/my-y/my-parent/3x/bar', + ); + expect(r.resolve(subRef3, '/my-grandparent/my-y')?.({ x: '3x' })).toBe( + '/my-grandparent/my-y/my-parent/3x/bar', + ); + expect(r.resolve(subRef4, l)?.({ x: '4x', a: '4a' })).toBe( + '/my-grandparent/my-y/my-parent/4x/bar/4a', + ); + expect( + r.resolve(subRef4, '/my-grandparent/my-y')?.({ x: '4x', a: '4a' }), + ).toBe('/my-grandparent/my-y/my-parent/4x/bar/4a'); + expect(r.resolve(externalRef1, l)?.()).toBe( + '/my-grandparent/my-y/my-parent/my-x/my-route', + ); + expect(() => r.resolve(externalRef1, '/')?.()).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(externalRef2, l)?.()).toBe(undefined); + expect(r.resolve(externalRef3, l)?.({ x: '5x' })).toBe( + '/my-grandparent/my-y/my-parent/5x', + ); + expect(() => r.resolve(externalRef3, '/')?.({ x: '5x' })).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(externalRef4, l)?.({ x: '6x' })).toBe( + '/my-grandparent/my-y/my-parent/6x/bar', + ); + expect(() => r.resolve(externalRef4, '/')?.({ x: '6x' })).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + }); +}); diff --git a/packages/core-app-api/src/routing/RouteResolver.ts b/packages/core-app-api/src/routing/RouteResolver.ts index f186b28586..f17c549bc6 100644 --- a/packages/core-app-api/src/routing/RouteResolver.ts +++ b/packages/core-app-api/src/routing/RouteResolver.ts @@ -30,15 +30,7 @@ import { ExternalRouteRef, SubRouteRef, } from '@backstage/core-plugin-api'; - -// Joins a list of paths together, avoiding trailing and duplicate slashes -function joinPaths(...paths: string[]): string { - const normalized = paths.join('/').replace(/\/\/+/g, '/'); - if (normalized !== '/' && normalized.endsWith('/')) { - return normalized.slice(0, -1); - } - return normalized; -} +import { joinPaths } from './helpers'; /** * Resolves the absolute route ref that our target route ref is pointing pointing to, as well @@ -90,7 +82,7 @@ function resolveTargetRef( // Find the path that our target route is bound to const resolvedPath = routePaths.get(targetRef); - if (!resolvedPath) { + if (resolvedPath === undefined) { return [undefined, '']; } @@ -160,22 +152,20 @@ function resolveBasePath( // we need to traverse to reach our target except for the very last one. None of these // paths are allowed to require any parameters, as the caller would have no way of knowing // what parameters those are. - const diffPath = joinPaths( - ...refDiffList.slice(0, -1).map(ref => { - const path = routePaths.get(ref); - if (!path) { - throw new Error(`No path for ${ref}`); - } - if (path.includes(':')) { - throw new Error( - `Cannot route to ${targetRef} with parent ${ref} as it has parameters`, - ); - } - return path; - }), - ); + const diffPaths = refDiffList.slice(0, -1).map(ref => { + const path = routePaths.get(ref); + if (path === undefined) { + throw new Error(`No path for ${ref}`); + } + if (path.includes(':')) { + throw new Error( + `Cannot route to ${targetRef} with parent ${ref} as it has parameters`, + ); + } + return path; + }); - return parentPath + diffPath; + return `${joinPaths(parentPath, ...diffPaths)}/`; } export class RouteResolver { @@ -235,7 +225,7 @@ export class RouteResolver { ); const routeFunc: RouteFunc = (...[params]) => { - return basePath + generatePath(targetPath, params); + return joinPaths(basePath, generatePath(targetPath, params)); }; return routeFunc; } diff --git a/packages/core-app-api/src/routing/RoutingProvider.test.tsx b/packages/core-app-api/src/routing/RoutingProvider.beta.test.tsx similarity index 98% rename from packages/core-app-api/src/routing/RoutingProvider.test.tsx rename to packages/core-app-api/src/routing/RoutingProvider.beta.test.tsx index c559c0fff0..532be8c09d 100644 --- a/packages/core-app-api/src/routing/RoutingProvider.test.tsx +++ b/packages/core-app-api/src/routing/RoutingProvider.beta.test.tsx @@ -40,6 +40,11 @@ import { RouteResolver } from './RouteResolver'; import { AnyRouteRef, RouteFunc } from './types'; import { AppContextProvider } from '../app/AppContext'; +jest.mock('react-router', () => jest.requireActual('react-router-beta')); +jest.mock('react-router-dom', () => + jest.requireActual('react-router-dom-beta'), +); + const MockComponent = ({ children }: PropsWithChildren<{ path?: string }>) => ( <>{children} ); diff --git a/packages/core-app-api/src/routing/RoutingProvider.compat.test.tsx b/packages/core-app-api/src/routing/RoutingProvider.compat.test.tsx new file mode 100644 index 0000000000..1ff80fcacc --- /dev/null +++ b/packages/core-app-api/src/routing/RoutingProvider.compat.test.tsx @@ -0,0 +1,422 @@ +/* + * 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 React, { PropsWithChildren, ReactElement } from 'react'; +import { render } from '@testing-library/react'; +import type { + BackstagePlugin, + RouteRef, + ExternalRouteRef, +} from '@backstage/core-plugin-api'; +import type { AnyRouteRef, RouteFunc } from './types'; + +const mockContext = { + getComponents: () => ({ Progress: () => null } as any), + getSystemIcon: jest.fn(), + getSystemIcons: jest.fn(), + getPlugins: jest.fn(), +}; + +describe.each(['beta', 'stable'])('react-router %s', rrVersion => { + function requireDeps() { + return { + ...(require('./FlatRoutes') as typeof import('./FlatRoutes')), + ...(require('react-router-dom') as typeof import('react-router-dom')), + ...(require('./RoutingProvider') as typeof import('./RoutingProvider')), + ...(require('../extensions/traversal') as typeof import('../extensions/traversal')), + ...(require('./collectors') as typeof import('./collectors')), + ...(require('./validation') as typeof import('./validation')), + ...(require('./types') as typeof import('./types')), + ...(require('../app/AppContext') as typeof import('../app/AppContext')), + ...(require('@backstage/core-plugin-api') as typeof import('@backstage/core-plugin-api')), + }; + } + + const MockComponent = ({ children }: PropsWithChildren<{}>) => { + const { useOutlet } = requireDeps(); + return ( + <> + {children} +
{useOutlet()}
+ + ); + }; + + const MockRouteSource = (props: { + path?: string; + name: string; + routeRef: AnyRouteRef; + params?: T; + }) => { + const { useRouteRef } = requireDeps(); + try { + const routeFunc = useRouteRef(props.routeRef as any) as + | RouteFunc + | undefined; + return ( +
+ Path at {props.name}: {routeFunc?.(props.params) ?? ''} +
+ ); + } catch (ex) { + return ( +
+ Error at {props.name}, {String(ex)} +
+ ); + } + }; + + let plugin: BackstagePlugin; + let refPage1: RouteRef; + let refSource1: RouteRef; + let refPage2: RouteRef; + let refSource2: RouteRef; + let refPage3: RouteRef<{ x: string }>; + let eRefA: ExternalRouteRef; + let eRefB: ExternalRouteRef; + let eRefC: ExternalRouteRef; + let eRefD: ExternalRouteRef; + let eRefE: ExternalRouteRef; + + let ExtensionPage1: typeof MockComponent; + let ExtensionPage2: typeof MockComponent; + let ExtensionPage3: typeof MockComponent; + let ExtensionSource1: typeof MockRouteSource; + let ExtensionSource2: typeof MockRouteSource; + + beforeAll(() => { + jest.doMock('react', () => React); + 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'), + ); + + const { + createRoutableExtension, + createExternalRouteRef, + createRouteRef, + createPlugin, + } = requireDeps(); + + plugin = createPlugin({ id: 'my-plugin' }); + refPage1 = createRouteRef({ id: 'refPage1' }); + refSource1 = createRouteRef({ id: 'refSource1' }); + refPage2 = createRouteRef({ id: 'refPage2' }); + refSource2 = createRouteRef({ id: 'refSource2' }); + refPage3 = createRouteRef({ id: 'refPage3', params: ['x'] }); + eRefA = createExternalRouteRef({ id: '1' }); + eRefB = createExternalRouteRef({ id: '2' }); + eRefC = createExternalRouteRef({ id: '3', params: ['y'] }); + eRefD = createExternalRouteRef({ id: '4', optional: true }); + eRefE = createExternalRouteRef({ id: '5', optional: true, params: ['z'] }); + + ExtensionPage1 = plugin.provide( + createRoutableExtension({ + name: 'ExtensionPage1', + component: () => Promise.resolve(MockComponent), + mountPoint: refPage1, + }), + ); + ExtensionPage2 = plugin.provide( + createRoutableExtension({ + name: 'ExtensionPage2', + component: () => Promise.resolve(MockComponent), + mountPoint: refPage2, + }), + ); + ExtensionPage3 = plugin.provide( + createRoutableExtension({ + name: 'ExtensionPage3', + component: () => Promise.resolve(MockComponent), + mountPoint: refPage3, + }), + ); + ExtensionSource1 = plugin.provide( + createRoutableExtension({ + name: 'ExtensionSource1', + component: () => Promise.resolve(MockRouteSource), + mountPoint: refSource1, + }), + ); + ExtensionSource2 = plugin.provide( + createRoutableExtension({ + name: 'ExtensionSource2', + component: () => Promise.resolve(MockRouteSource), + mountPoint: refSource2, + }), + ); + }); + + afterAll(() => { + jest.resetModules(); + jest.resetAllMocks(); + jest.restoreAllMocks(); + jest.clearAllMocks(); + }); + + function withRoutingProvider( + root: ReactElement, + routeBindings: [ExternalRouteRef, RouteRef][] = [], + ) { + const { + traverseElementTree, + childDiscoverer, + routeElementDiscoverer, + routingV2Collector, + RoutingProvider, + } = requireDeps(); + const { routing } = traverseElementTree({ + root, + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routing: routingV2Collector, + }, + }); + + return ( + + {root} + + ); + } + + it('should handle simple routeRef path creation for routeRefs used in other parts of the app', async () => { + const { MemoryRouter, Routes, Route, AppContextProvider } = requireDeps(); + const root = ( + + + + + + + + } + > + + } + /> + + } /> + + + + + + + + + + ); + + const rendered = render( + withRoutingProvider(root, [ + [eRefA, refPage2], + [eRefB, refPage1], + [eRefC, refSource1], + [eRefD, refPage1], + ]), + ); + + await new Promise(r => setTimeout(r, 500)); + + rendered.debug(); + await expect( + rendered.findByText('Path at inside: /foo/bar'), + ).resolves.toBeInTheDocument(); + expect( + rendered.getByText('Path at insideExternal: /baz'), + ).toBeInTheDocument(); + expect(rendered.getByText('Path at outside: /foo/bar')).toBeInTheDocument(); + expect( + rendered.getByText('Path at outsideExternal1: /foo'), + ).toBeInTheDocument(); + expect( + rendered.getByText('Path at outsideExternal2: /foo/bar'), + ).toBeInTheDocument(); + expect( + rendered.getByText('Path at outsideExternal3: /foo'), + ).toBeInTheDocument(); + expect( + rendered.getByText('Path at outsideExternal4: '), + ).toBeInTheDocument(); + }); + + it('should handle routeRefs with parameters', async () => { + const { MemoryRouter, Routes, Route, AppContextProvider } = requireDeps(); + const root = ( + + + + }> + + } + /> + + + + + + ); + + const rendered = render(withRoutingProvider(root)); + + await expect( + rendered.findByText('Path at inside: /foo/bar/bleb'), + ).resolves.toBeInTheDocument(); + expect( + rendered.getByText('Path at outside: /foo/bar/blob'), + ).toBeInTheDocument(); + }); + + it('should handle relative routing within parameterized routePaths', async () => { + const { MemoryRouter, Routes, Route, AppContextProvider } = requireDeps(); + const root = ( + + + + + }> + + } + /> + } /> + + + + + + + + ); + + const rendered = render(withRoutingProvider(root)); + + await expect( + rendered.findByText('Path at inside: /foo/blob/baz'), + ).resolves.toBeInTheDocument(); + rendered.debug(); + }); + + it('should throw errors for routing to other routeRefs with unsupported parameters', () => { + const { MemoryRouter, Routes, Route } = requireDeps(); + const root = ( + + + }> + } + /> + } /> + + + + + + ); + + const rendered = render(withRoutingProvider(root)); + + expect( + rendered.getByText( + `Error at outsideWithParams, Error: Cannot route to ${refPage2} with parent ${refPage3} as it has parameters`, + ), + ).toBeInTheDocument(); + expect( + rendered.getByText( + `Error at outsideNoParams, Error: Cannot route to ${refPage2} with parent ${refPage3} as it has parameters`, + ), + ).toBeInTheDocument(); + }); + + it('should handle relative routing of parameterized routePaths with duplicate param names', () => { + const { + MemoryRouter, + Routes, + Route, + traverseElementTree, + childDiscoverer, + routeElementDiscoverer, + routingV2Collector, + validateRouteParameters, + } = requireDeps(); + const root = ( + + + }> + } + /> + + + + ); + + const { routing } = traverseElementTree({ + root, + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routing: routingV2Collector, + }, + }); + + expect(() => + validateRouteParameters(routing.paths, routing.parents), + ).toThrow('Parameter :id is duplicated in path foo/:id/bar/:id'); + }); +}); diff --git a/packages/core-app-api/src/routing/RoutingProvider.stable.test.tsx b/packages/core-app-api/src/routing/RoutingProvider.stable.test.tsx new file mode 100644 index 0000000000..c5ea3ed9e0 --- /dev/null +++ b/packages/core-app-api/src/routing/RoutingProvider.stable.test.tsx @@ -0,0 +1,412 @@ +/* + * 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 React, { PropsWithChildren, ReactElement } from 'react'; +import { MemoryRouter, Routes, Route, useOutlet } from 'react-router-dom'; +import { render } from '@testing-library/react'; +import { renderHook } from '@testing-library/react-hooks'; +import { useVersionedContext } from '@backstage/version-bridge'; +import { + childDiscoverer, + routeElementDiscoverer, + traverseElementTree, +} from '../extensions/traversal'; +import { + createPlugin, + useRouteRef, + createRoutableExtension, + createRouteRef, + createExternalRouteRef, + RouteRef, + ExternalRouteRef, +} from '@backstage/core-plugin-api'; +import { RoutingProvider } from './RoutingProvider'; +import { routingV2Collector } from './collectors'; +import { validateRouteParameters } from './validation'; +import { RouteResolver } from './RouteResolver'; +import { AnyRouteRef, RouteFunc } from './types'; +import { AppContextProvider } from '../app/AppContext'; + +jest.mock('react-router', () => jest.requireActual('react-router-stable')); +jest.mock('react-router-dom', () => + jest.requireActual('react-router-dom-stable'), +); + +const MockComponent = ({ children }: PropsWithChildren<{}>) => ( + <> + {children} +
{useOutlet()}
+ +); + +const plugin = createPlugin({ id: 'my-plugin' }); + +const refPage1 = createRouteRef({ id: 'refPage1' }); +const refSource1 = createRouteRef({ id: 'refSource1' }); +const refPage2 = createRouteRef({ id: 'refPage2' }); +const refSource2 = createRouteRef({ id: 'refSource2' }); +const refPage3 = createRouteRef({ id: 'refPage3', params: ['x'] }); +const eRefA = createExternalRouteRef({ id: '1' }); +const eRefB = createExternalRouteRef({ id: '2' }); +const eRefC = createExternalRouteRef({ id: '3', params: ['y'] }); +const eRefD = createExternalRouteRef({ id: '4', optional: true }); +const eRefE = createExternalRouteRef({ + id: '5', + optional: true, + params: ['z'], +}); + +const MockRouteSource = (props: { + path?: string; + name: string; + routeRef: AnyRouteRef; + params?: T; +}) => { + try { + const routeFunc = useRouteRef(props.routeRef as any) as + | RouteFunc + | undefined; + return ( +
+ Path at {props.name}: {routeFunc?.(props.params) ?? ''} +
+ ); + } catch (ex) { + return ( +
+ Error at {props.name}, {String(ex)} +
+ ); + } +}; + +const ExtensionPage1 = plugin.provide( + createRoutableExtension({ + name: 'ExtensionPage1', + component: () => Promise.resolve(MockComponent), + mountPoint: refPage1, + }), +); +const ExtensionPage2 = plugin.provide( + createRoutableExtension({ + name: 'ExtensionPage2', + component: () => Promise.resolve(MockComponent), + mountPoint: refPage2, + }), +); +const ExtensionPage3 = plugin.provide( + createRoutableExtension({ + name: 'ExtensionPage3', + component: () => Promise.resolve(MockComponent), + mountPoint: refPage3, + }), +); +const ExtensionSource1 = plugin.provide( + createRoutableExtension({ + name: 'ExtensionSource1', + component: () => Promise.resolve(MockRouteSource), + mountPoint: refSource1, + }), +); +const ExtensionSource2 = plugin.provide( + createRoutableExtension({ + name: 'ExtensionSource2', + component: () => Promise.resolve(MockRouteSource), + mountPoint: refSource2, + }), +); + +const mockContext = { + getComponents: () => ({ Progress: () => null } as any), + getSystemIcon: jest.fn(), + getSystemIcons: jest.fn(), + getPlugins: jest.fn(), +}; + +function withRoutingProvider( + root: ReactElement, + routeBindings: [ExternalRouteRef, RouteRef][] = [], +) { + const { routing } = traverseElementTree({ + root, + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routing: routingV2Collector, + }, + }); + + return ( + + {root} + + ); +} + +describe('discovery', () => { + it('should handle simple routeRef path creation for routeRefs used in other parts of the app', async () => { + const root = ( + + + + + + + + } + > + + } + /> + + } /> + + + + + + + + + + ); + + const rendered = render( + withRoutingProvider(root, [ + [eRefA, refPage2], + [eRefB, refPage1], + [eRefC, refSource1], + [eRefD, refPage1], + ]), + ); + + await expect( + rendered.findByText('Path at inside: /foo/bar'), + ).resolves.toBeInTheDocument(); + expect( + rendered.getByText('Path at insideExternal: /baz'), + ).toBeInTheDocument(); + expect(rendered.getByText('Path at outside: /foo/bar')).toBeInTheDocument(); + expect( + rendered.getByText('Path at outsideExternal1: /foo'), + ).toBeInTheDocument(); + expect( + rendered.getByText('Path at outsideExternal2: /foo/bar'), + ).toBeInTheDocument(); + expect( + rendered.getByText('Path at outsideExternal3: /foo'), + ).toBeInTheDocument(); + expect( + rendered.getByText('Path at outsideExternal4: '), + ).toBeInTheDocument(); + }); + + it('should handle routeRefs with parameters', async () => { + const root = ( + + + + }> + + } + /> + + + + + + ); + + const rendered = render(withRoutingProvider(root)); + + await expect( + rendered.findByText('Path at inside: /foo/bar/bleb'), + ).resolves.toBeInTheDocument(); + expect( + rendered.getByText('Path at outside: /foo/bar/blob'), + ).toBeInTheDocument(); + }); + + it('should handle relative routing within parameterized routePaths', async () => { + const root = ( + + + + + }> + + } + /> + } /> + + + + + + + + ); + + const rendered = render(withRoutingProvider(root)); + + await expect( + rendered.findByText('Path at inside: /foo/blob/baz'), + ).resolves.toBeInTheDocument(); + rendered.debug(); + }); + + it('should throw errors for routing to other routeRefs with unsupported parameters', () => { + const root = ( + + + }> + } + /> + } /> + + + + + + ); + + const rendered = render(withRoutingProvider(root)); + + expect( + rendered.getByText( + `Error at outsideWithParams, Error: Cannot route to ${refPage2} with parent ${refPage3} as it has parameters`, + ), + ).toBeInTheDocument(); + expect( + rendered.getByText( + `Error at outsideNoParams, Error: Cannot route to ${refPage2} with parent ${refPage3} as it has parameters`, + ), + ).toBeInTheDocument(); + }); + + it('should handle relative routing of parameterized routePaths with duplicate param names', () => { + const root = ( + + + }> + } + /> + + + + ); + + const { routing } = traverseElementTree({ + root, + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routing: routingV2Collector, + }, + }); + + expect(() => + validateRouteParameters(routing.paths, routing.parents), + ).toThrow('Parameter :id is duplicated in path foo/:id/bar/:id'); + }); +}); + +describe('v1 consumer', () => { + function useMockRouteRefV1( + routeRef: AnyRouteRef, + location: string, + ): RouteFunc | undefined { + const resolver = useVersionedContext<{ + 1: RouteResolver; + }>('routing-context')?.atVersion(1); + if (!resolver) { + throw new Error('no impl'); + } + return resolver.resolve(routeRef, location); + } + + it('should resolve routes', () => { + const routeRef1 = createRouteRef({ id: 'refPage1' }); + const routeRef2 = createRouteRef({ id: 'refSource1' }); + const routeRef3 = createRouteRef({ id: 'refPage2', params: ['x'] }); + + const renderedHook = renderHook( + ({ routeRef }) => useMockRouteRefV1(routeRef, '/'), + { + initialProps: { + routeRef: routeRef1 as AnyRouteRef, + }, + wrapper: ({ children }) => ( + , string>([ + [routeRef2, '/foo'], + [routeRef3, '/bar/:x'], + ]) + } + routeParents={new Map()} + routeObjects={[]} + routeBindings={new Map()} + basePath="/base" + children={children} + /> + ), + }, + ); + + expect(renderedHook.result.current).toBe(undefined); + renderedHook.rerender({ routeRef: routeRef2 }); + expect(renderedHook.result.current?.()).toBe('/base/foo'); + renderedHook.rerender({ routeRef: routeRef3 }); + expect(renderedHook.result.current?.({ x: 'my-x' })).toBe('/base/bar/my-x'); + }); +}); diff --git a/packages/core-app-api/src/routing/collectors.test.tsx b/packages/core-app-api/src/routing/collectors.beta.test.tsx similarity index 98% rename from packages/core-app-api/src/routing/collectors.test.tsx rename to packages/core-app-api/src/routing/collectors.beta.test.tsx index 1c5bc8c639..74d8bbbe53 100644 --- a/packages/core-app-api/src/routing/collectors.test.tsx +++ b/packages/core-app-api/src/routing/collectors.beta.test.tsx @@ -32,6 +32,11 @@ import { } from '@backstage/core-plugin-api'; import { MemoryRouter, Routes, Route } from 'react-router-dom'; +jest.mock('react-router', () => jest.requireActual('react-router-beta')); +jest.mock('react-router-dom', () => + jest.requireActual('react-router-dom-beta'), +); + const MockComponent = ({ children }: PropsWithChildren<{ path?: string }>) => ( <>{children} ); @@ -109,7 +114,7 @@ function routeObj( routeRefs: new Set(refs), children: [ { - path: '/*', + path: '*', caseSensitive: false, element: 'match-all', routeRefs: new Set(), diff --git a/packages/core-app-api/src/routing/collectors.compat.test.tsx b/packages/core-app-api/src/routing/collectors.compat.test.tsx new file mode 100644 index 0000000000..e1333a070a --- /dev/null +++ b/packages/core-app-api/src/routing/collectors.compat.test.tsx @@ -0,0 +1,297 @@ +/* + * 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 React from 'react'; +import type { PropsWithChildren } from 'react'; +import { + RouteRef, + createPlugin, + createRouteRef, + createRoutableExtension, + attachComponentData, +} from '@backstage/core-plugin-api'; +import { Collector, Discoverer } from '../extensions/traversal'; + +describe.each(['beta', 'stable'])('react-router %s', rrVersion => { + beforeAll(() => { + 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'), + ); + }); + + function requireDeps() { + return { + ...(require('./collectors') as typeof import('./collectors')), + ...(require('../extensions/traversal') as typeof import('../extensions/traversal')), + ...(require('@backstage/core-plugin-api') as typeof import('@backstage/core-plugin-api')), + ...(require('react-router-dom') as typeof import('react-router-dom')), + }; + } + + afterAll(() => { + jest.resetModules(); + jest.resetAllMocks(); + jest.restoreAllMocks(); + jest.clearAllMocks(); + }); + + const MockComponent = ({ + children, + }: PropsWithChildren<{ path?: string }>) => <>{children}; + + const plugin = createPlugin({ id: 'my-plugin' }); + + const ref1 = createRouteRef({ id: 'ref1' }); + const ref2 = createRouteRef({ id: 'ref2' }); + const ref3 = createRouteRef({ id: 'ref3' }); + const ref4 = createRouteRef({ id: 'ref4' }); + const ref5 = createRouteRef({ id: 'ref5' }); + const refOrder = [ref1, ref2, ref3, ref4, ref5]; + + const Extension1 = plugin.provide( + createRoutableExtension({ + name: 'Extension1', + component: () => Promise.resolve(MockComponent), + mountPoint: ref1, + }), + ); + const Extension2 = plugin.provide( + createRoutableExtension({ + name: 'Extension2', + component: () => Promise.resolve(MockComponent), + mountPoint: ref2, + }), + ); + const Extension3 = plugin.provide( + createRoutableExtension({ + name: 'Extension3', + component: () => Promise.resolve(MockComponent), + mountPoint: ref3, + }), + ); + const Extension4 = plugin.provide( + createRoutableExtension({ + name: 'Extension4', + component: () => Promise.resolve(MockComponent), + mountPoint: ref4, + }), + ); + const Extension5 = plugin.provide( + createRoutableExtension({ + name: 'Extension5', + component: () => Promise.resolve(MockComponent), + mountPoint: ref5, + }), + ); + + const AggregationComponent = ({ + children, + }: PropsWithChildren<{ + path: string; + }>) => <>{children}; + + attachComponentData(AggregationComponent, 'core.gatherMountPoints', true); + + function sortedEntries(map: Map): [RouteRef, T][] { + return Array.from(map).sort( + ([a], [b]) => refOrder.indexOf(a) - refOrder.indexOf(b), + ); + } + + function routeObj( + path: string, + refs: RouteRef[], + children: any[] = [], + type: 'mounted' | 'gathered' = 'mounted', + ) { + return expect.objectContaining({ + path: path, + caseSensitive: false, + element: type, + routeRefs: new Set(refs), + children: [ + { + path: '*', + caseSensitive: false, + element: 'match-all', + routeRefs: new Set(), + }, + ...children, + ], + }); + } + + describe(`routing with ${rrVersion}`, () => { + let traversalOptions: { + discoverers: Discoverer[]; + collectors: { routing: Collector }; + }; + beforeEach(() => { + const { + routingV1Collector, + routingV2Collector, + childDiscoverer, + routeElementDiscoverer, + } = requireDeps(); + + traversalOptions = { + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routing: + rrVersion === 'beta' ? routingV1Collector : routingV2Collector, + }, + }; + }); + + it('should collect routes', () => { + const { MemoryRouter, Routes, Route, traverseElementTree } = + requireDeps(); + const list = [ +
, +
, +
+ } /> +
, + ]; + + const root = ( + + + } /> + }> +
+ }> +
+
+ Some text here shouldn't be a problem +
+ {null} +
+ + } /> + +
+ + {false} + {list} + {true} + {0} +
+ +
+ } /> +
+ + + ); + + const { routing } = traverseElementTree({ + root, + ...traversalOptions, + }); + + expect(sortedEntries(routing.paths)).toEqual([ + [ref1, 'foo'], + [ref2, 'bar/:id'], + [ref3, rrVersion === 'beta' ? '/baz' : 'baz'], + [ref4, 'divsoup'], + [ref5, 'blop'], + ]); + expect(sortedEntries(routing.parents)).toEqual([ + [ref1, undefined], + [ref2, ref1], + [ref3, ref2], + [ref4, undefined], + [ref5, ref1], + ]); + expect(routing.objects).toEqual([ + routeObj( + 'foo', + [ref1], + [ + routeObj( + 'bar/:id', + [ref2], + [routeObj(rrVersion === 'beta' ? '/baz' : 'baz', [ref3])], + ), + routeObj('blop', [ref5]), + ], + ), + routeObj('divsoup', [ref4]), + ]); + }); + + it('should collect routes with aggregators', () => { + const { MemoryRouter, Routes, Route, traverseElementTree } = + requireDeps(); + const root = ( + + + + +
+ +
+ HELLO +
+ }> + + + + + + +
+
+ ); + + const { routing } = traverseElementTree({ + root, + ...traversalOptions, + }); + + expect(sortedEntries(routing.paths)).toEqual([ + [ref1, 'foo'], + [ref2, 'foo'], + [ref3, 'bar'], + [ref4, 'baz'], + [ref5, 'baz'], + ]); + expect(sortedEntries(routing.parents)).toEqual([ + [ref1, undefined], + [ref2, undefined], + [ref3, undefined], + [ref4, ref3], + [ref5, ref3], + ]); + expect(routing.objects).toEqual([ + routeObj('foo', [ref1, ref2], [], 'gathered'), + routeObj( + 'bar', + [ref3], + [routeObj('baz', [ref4, ref5], [], 'gathered')], + ), + ]); + }); + }); +}); diff --git a/packages/core-app-api/src/routing/collectors.stable.test.tsx b/packages/core-app-api/src/routing/collectors.stable.test.tsx new file mode 100644 index 0000000000..2324ea5b69 --- /dev/null +++ b/packages/core-app-api/src/routing/collectors.stable.test.tsx @@ -0,0 +1,507 @@ +/* + * 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 React, { ComponentType, PropsWithChildren } from 'react'; +import { routingV2Collector } from './collectors'; + +import { + traverseElementTree, + childDiscoverer, + routeElementDiscoverer, +} from '../extensions/traversal'; +import { + createRoutableExtension, + createRouteRef, + createPlugin, + RouteRef, + attachComponentData, + BackstagePlugin, +} from '@backstage/core-plugin-api'; +import { MemoryRouter, Routes, Route } from 'react-router-dom'; + +jest.mock('react-router', () => jest.requireActual('react-router-stable')); +jest.mock('react-router-dom', () => + jest.requireActual('react-router-dom-stable'), +); + +const MockComponent = ({ children }: PropsWithChildren<{ path?: string }>) => ( + <>{children} +); + +const plugin = createPlugin({ id: 'my-plugin' }); + +const ref1 = createRouteRef({ id: 'ref1' }); +const ref2 = createRouteRef({ id: 'ref2' }); +const ref3 = createRouteRef({ id: 'ref3' }); +const ref4 = createRouteRef({ id: 'ref4' }); +const ref5 = createRouteRef({ id: 'ref5' }); +const refOrder = [ref1, ref2, ref3, ref4, ref5]; + +const Extension1 = plugin.provide( + createRoutableExtension({ + name: 'Extension1', + component: () => Promise.resolve(MockComponent), + mountPoint: ref1, + }), +); +const Extension2 = plugin.provide( + createRoutableExtension({ + name: 'Extension2', + component: () => Promise.resolve(MockComponent), + mountPoint: ref2, + }), +); +const Extension3 = plugin.provide( + createRoutableExtension({ + name: 'Extension3', + component: () => Promise.resolve(MockComponent), + mountPoint: ref3, + }), +); +const Extension4 = plugin.provide( + createRoutableExtension({ + name: 'Extension4', + component: () => Promise.resolve(MockComponent), + mountPoint: ref4, + }), +); +const Extension5 = plugin.provide( + createRoutableExtension({ + name: 'Extension5', + component: () => Promise.resolve(MockComponent), + mountPoint: ref5, + }), +); + +const AggregationComponent = ({ + children, +}: PropsWithChildren<{ + path: string; +}>) => <>{children}; + +attachComponentData(AggregationComponent, 'core.gatherMountPoints', true); + +function sortedEntries(map: Map): [RouteRef, T][] { + return Array.from(map).sort( + ([a], [b]) => refOrder.indexOf(a) - refOrder.indexOf(b), + ); +} + +function routeObj( + path: string, + refs: RouteRef[], + children: any[] = [], + type: 'mounted' | 'gathered' = 'mounted', + backstagePlugin?: BackstagePlugin, +) { + return { + path: path, + caseSensitive: false, + element: type, + routeRefs: new Set(refs), + children: [ + { + path: '*', + caseSensitive: false, + element: 'match-all', + routeRefs: new Set(), + }, + ...children, + ], + plugin: backstagePlugin, + }; +} + +describe('discovery', () => { + it('should collect routes', () => { + const list = [ +
, +
, +
+ } /> +
, + ]; + + const root = ( + + + } /> + }> +
+ {[, 'a string']}} + > +
+
+ Some text here shouldn't be a problem +
+ {null} +
+ } /> +
+ + {false} + {list} + {true} + {0} +
+ +
+ } /> +
+ + + ); + + const { routing } = traverseElementTree({ + root, + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routing: routingV2Collector, + }, + }); + expect(sortedEntries(routing.paths)).toEqual([ + [ref1, 'foo'], + [ref2, 'bar/:id'], + [ref3, 'baz'], + [ref4, 'divsoup'], + [ref5, 'blop'], + ]); + expect(sortedEntries(routing.parents)).toEqual([ + [ref1, undefined], + [ref2, ref1], + [ref3, ref2], + [ref4, undefined], + [ref5, ref1], + ]); + expect(routing.objects).toEqual([ + routeObj( + 'foo', + [ref1], + [ + routeObj( + 'bar/:id', + [ref2], + [routeObj('baz', [ref3], undefined, undefined, plugin)], + undefined, + plugin, + ), + routeObj('blop', [ref5], undefined, undefined, plugin), + ], + undefined, + plugin, + ), + routeObj('divsoup', [ref4], undefined, undefined, plugin), + ]); + }); + + it('should handle all react router Route patterns', () => { + const root = ( + + + }> + + } /> + + + }> + } /> + } /> + + + + ); + + const { routing } = traverseElementTree({ + root, + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routing: routingV2Collector, + }, + }); + expect(sortedEntries(routing.paths)).toEqual([ + [ref1, 'foo'], + [ref2, 'bar/:id'], + [ref3, 'baz'], + [ref4, 'divsoup'], + [ref5, 'blop'], + ]); + expect(sortedEntries(routing.parents)).toEqual([ + [ref1, undefined], + [ref2, ref1], + [ref3, undefined], + [ref4, ref3], + [ref5, ref3], + ]); + }); + + it('should handle absolute route paths', () => { + const root = ( + + + }> + + } /> + + + }> + } /> + } /> + + + + ); + + const { routing } = traverseElementTree({ + root, + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routing: routingV2Collector, + }, + }); + expect(sortedEntries(routing.paths)).toEqual([ + [ref1, 'foo'], + [ref2, 'bar/:id'], + [ref3, 'baz'], + [ref4, 'divsoup'], + [ref5, 'blop'], + ]); + expect(sortedEntries(routing.parents)).toEqual([ + [ref1, undefined], + [ref2, ref1], + [ref3, undefined], + [ref4, ref3], + [ref5, ref3], + ]); + }); + + it('should use the route aggregator key to bind child routes to the same path', () => { + const root = ( + + + + +
+ +
+ HELLO +
+ }> + + + + + + +
+
+ ); + + const { routing } = traverseElementTree({ + root, + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routing: routingV2Collector, + }, + }); + expect(sortedEntries(routing.paths)).toEqual([ + [ref1, 'foo'], + [ref2, 'foo'], + [ref3, 'bar'], + [ref4, 'baz'], + [ref5, 'baz'], + ]); + expect(sortedEntries(routing.parents)).toEqual([ + [ref1, undefined], + [ref2, undefined], + [ref3, undefined], + [ref4, ref3], + [ref5, ref3], + ]); + expect(routing.objects).toEqual([ + routeObj('foo', [ref1, ref2], [], 'gathered'), + routeObj( + 'bar', + [ref3], + [routeObj('baz', [ref4, ref5], [], 'gathered')], + undefined, + plugin, + ), + ]); + }); + + it('should use the route aggregator but stop when encountering explicit path', () => { + const root = ( + + + }> + + + + }> + } /> + + + + + + + + + ); + + const { routing } = traverseElementTree({ + root, + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routing: routingV2Collector, + }, + }); + expect(sortedEntries(routing.paths)).toEqual([ + [ref1, 'foo'], + [ref2, 'bar'], + [ref3, 'baz'], + [ref4, 'blop'], + [ref5, 'bar'], + ]); + expect(sortedEntries(routing.parents)).toEqual([ + [ref1, undefined], + [ref2, ref1], + [ref3, ref2], + [ref4, ref3], + [ref5, ref1], + ]); + expect(routing.objects).toEqual([ + routeObj( + 'foo', + [ref1], + [ + routeObj( + 'bar', + [ref2, ref5], + [ + routeObj( + 'baz', + [ref3], + [routeObj('blop', [ref4], undefined, undefined, plugin)], + undefined, + plugin, + ), + ], + 'gathered', + ), + ], + undefined, + plugin, + ), + ]); + }); + + it('should throw when you provide path property on an extension', () => { + expect(() => { + traverseElementTree({ + root: , + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routing: routingV2Collector, + }, + }); + }).toThrow( + 'Path property may not be set directly on a routable extension "Extension(Extension1)"', + ); + }); + + it('should throw when element prop is not a string', () => { + const Div = 'div' as unknown as ComponentType<{ path: boolean }>; + expect(() => { + traverseElementTree({ + root:
, + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routing: routingV2Collector, + }, + }); + }).toThrow('Element path must be a string at "div"'); + }); + + it('should throw when the mount point gatherers have an element prop', () => { + const AnyAggregationComponent = AggregationComponent as any; + expect(() => { + traverseElementTree({ + root: } />, + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routing: routingV2Collector, + }, + }); + }).toThrow( + 'Mount point gatherers may not have an element prop "AggregationComponent"', + ); + }); + + it('should throw elements within element prop contains a path', () => { + expect(() => { + traverseElementTree({ + root: } />, + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routing: routingV2Collector, + }, + }); + }).toThrow( + 'Elements within the element prop tree may not have paths, found "bar"', + ); + }); + + it('should throw when a routable extension does not have a path set', () => { + expect(() => { + traverseElementTree({ + root: , + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routing: routingV2Collector, + }, + }); + }).toThrow( + 'Routable extension "Extension(Extension3)" with mount point "routeRef{type=absolute,id=ref3}" must be assigned a path', + ); + }); + + it('should throw when Route elements contain multiple routable extensions', () => { + expect(() => { + traverseElementTree({ + root: ( + + + + + } + /> + ), + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routing: routingV2Collector, + }, + }); + }).toThrow( + 'Route element with path "foo" may not contain multiple routable extensions', + ); + }); +}); diff --git a/packages/core-app-api/src/routing/collectors.tsx b/packages/core-app-api/src/routing/collectors.tsx index fcd954201a..e5d71f5117 100644 --- a/packages/core-app-api/src/routing/collectors.tsx +++ b/packages/core-app-api/src/routing/collectors.tsx @@ -19,7 +19,7 @@ import { getComponentData, BackstagePlugin, } from '@backstage/core-plugin-api'; -import { isValidElement } from 'react'; +import { isValidElement, ReactNode, Children } from 'react'; import { BackstageRouteObject } from './types'; import { createCollector } from '../extensions/traversal'; import { FeatureFlagged, FeatureFlaggedProps } from './FeatureFlagged'; @@ -30,11 +30,179 @@ import { FeatureFlagged, FeatureFlaggedProps } from './FeatureFlagged'; // mount points that are as deep in the routing tree as possible. export const MATCH_ALL_ROUTE: BackstageRouteObject = { caseSensitive: false, - path: '/*', + path: '*', element: 'match-all', // These elements aren't used, so we add in a bit of debug information routeRefs: new Set(), }; +function stringifyNode(node: ReactNode): string { + const anyNode = node as { type?: { displayName?: string; name?: string } }; + if (anyNode?.type) { + return ( + anyNode.type.displayName ?? anyNode.type.name ?? String(anyNode.type) + ); + } + return String(anyNode); +} + +interface RoutingV2CollectorContext { + routeRef?: RouteRef; + gatherPath?: string; + gatherRouteRef?: RouteRef; + obj?: BackstageRouteObject; + isElementAncestor?: boolean; +} + +function collectSubTree( + node: ReactNode, + entries = new Array<{ routeRef: RouteRef; plugin?: BackstagePlugin }>(), +) { + Children.forEach(node, element => { + if (!isValidElement(element)) { + return; + } + + if (element.props.path) { + throw new Error( + `Elements within the element prop tree may not have paths, found "${element.props.path}"`, + ); + } + + const routeRef = getComponentData(element, 'core.mountPoint'); + if (routeRef) { + const plugin = getComponentData(element, 'core.plugin'); + entries.push({ routeRef, plugin }); + } + + collectSubTree(element.props.children, entries); + }); + + return entries; +} + +export const routingV2Collector = createCollector( + () => ({ + paths: new Map(), + parents: new Map(), + objects: new Array(), + }), + (acc, node, parent, ctx?: RoutingV2CollectorContext) => { + const pathProp: unknown = node.props?.path; + + const mountPoint = getComponentData(node, 'core.mountPoint'); + if (mountPoint && pathProp) { + throw new Error( + `Path property may not be set directly on a routable extension "${stringifyNode( + node, + )}"`, + ); + } + + // If we're in an element prop, ignore everything + if (ctx?.isElementAncestor) { + return ctx; + } + // Start ignoring everything if we enter an element prop + if (parent?.props.element === node) { + return { ...ctx, isElementAncestor: true }; + } + + const parentChildren = ctx?.obj?.children ?? acc.objects; + + if (pathProp !== undefined) { + if (typeof pathProp !== 'string') { + throw new Error( + `Element path must be a string at "${stringifyNode(node)}"`, + ); + } + + const path = pathProp.startsWith('/') ? pathProp.slice(1) : pathProp; + + const elementProp = node.props.element; + + if (getComponentData(node, 'core.gatherMountPoints')) { + if (elementProp) { + throw new Error( + `Mount point gatherers may not have an element prop "${stringifyNode( + node, + )}"`, + ); + } + + const newObj = { + path, + element: 'gathered', + routeRefs: new Set(), + caseSensitive: Boolean(node.props?.caseSensitive), + children: [MATCH_ALL_ROUTE], + plugin: undefined, + }; + parentChildren.push(newObj); + + return { + obj: newObj, + gatherPath: path, + routeRef: ctx?.routeRef, + gatherRouteRef: ctx?.routeRef, + }; + } + + if (elementProp) { + const [extension, ...others] = collectSubTree(elementProp); + if (others.length > 0) { + throw new Error( + `Route element with path "${pathProp}" may not contain multiple routable extensions`, + ); + } + if (!extension) { + return ctx; + } + const { routeRef, plugin } = extension; + + const newObj = { + path, + element: 'mounted', + routeRefs: new Set([routeRef]), + caseSensitive: Boolean(node.props?.caseSensitive), + children: [MATCH_ALL_ROUTE], + plugin, + }; + parentChildren.push(newObj); + acc.paths.set(routeRef, path); + acc.parents.set(routeRef, ctx?.routeRef); + + return { + obj: newObj, + routeRef: routeRef ?? ctx?.routeRef, + gatherPath: path, + gatherRouteRef: ctx?.gatherRouteRef, + }; + } + } + + if (mountPoint) { + if (!ctx?.gatherPath) { + throw new Error( + `Routable extension "${stringifyNode( + node, + )}" with mount point "${mountPoint}" must be assigned a path`, + ); + } + + ctx?.obj?.routeRefs.add(mountPoint); + acc.paths.set(mountPoint, ctx.gatherPath); + acc.parents.set(mountPoint, ctx?.gatherRouteRef); + + return { + ...ctx, + routeRef: mountPoint, + }; + } + + return ctx; + }, +); + interface RoutingV1CollectorContext { path?: string; routeRef?: RouteRef; diff --git a/packages/core-app-api/src/routing/helpers.ts b/packages/core-app-api/src/routing/helpers.ts new file mode 100644 index 0000000000..b25f602d52 --- /dev/null +++ b/packages/core-app-api/src/routing/helpers.ts @@ -0,0 +1,24 @@ +/* + * Copyright 2022 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. + */ + +// Joins a list of paths together, avoiding trailing and duplicate slashes +export function joinPaths(...paths: string[]): string { + const normalized = paths.join('/').replace(/\/\/+/g, '/'); + if (normalized !== '/' && normalized.endsWith('/')) { + return normalized.slice(0, -1); + } + return normalized; +} diff --git a/packages/core-app-api/src/routing/validation.ts b/packages/core-app-api/src/routing/validation.ts index c5d3e4aca9..63950b9fdc 100644 --- a/packages/core-app-api/src/routing/validation.ts +++ b/packages/core-app-api/src/routing/validation.ts @@ -20,6 +20,7 @@ import { RouteRef, SubRouteRef, } from '@backstage/core-plugin-api'; +import { joinPaths } from './helpers'; import { AnyRouteRef } from './types'; // Validates that there is no duplication of route parameter names @@ -40,10 +41,10 @@ export function validateRouteParameters( let fullPath = ''; while (currentRouteRef) { const path = routePaths.get(currentRouteRef); - if (!path) { + if (path === undefined) { throw new Error(`No path for ${currentRouteRef}`); } - fullPath = `${path}${fullPath}`; + fullPath = joinPaths(path, fullPath); currentRouteRef = routeParents.get(currentRouteRef); } diff --git a/packages/core-components/package.json b/packages/core-components/package.json index 74dc89bfc0..ffa256bd48 100644 --- a/packages/core-components/package.json +++ b/packages/core-components/package.json @@ -60,8 +60,6 @@ "react-helmet": "6.1.0", "react-hook-form": "^7.12.2", "react-markdown": "^8.0.0", - "react-router": "6.0.0-beta.0", - "react-router-dom": "6.0.0-beta.0", "react-sparklines": "^1.7.0", "react-syntax-highlighter": "^15.4.5", "react-text-truncate": "^0.19.0", @@ -76,7 +74,9 @@ "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", "react": "^16.13.1 || ^17.0.0", - "react-dom": "^16.13.1 || ^17.0.0" + "react-dom": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/core-app-api": "^1.0.6-next.0", diff --git a/packages/core-components/src/components/Button/Button.test.tsx b/packages/core-components/src/components/Button/Button.test.tsx index e3a1074e6f..805b3b3f10 100644 --- a/packages/core-components/src/components/Button/Button.test.tsx +++ b/packages/core-components/src/components/Button/Button.test.tsx @@ -26,10 +26,12 @@ describe(' - , + + {testString}

} /> +
+ , ), ); diff --git a/packages/core-components/src/components/Button/Button.tsx b/packages/core-components/src/components/Button/Button.tsx index d7d3c192a4..575f42b03c 100644 --- a/packages/core-components/src/components/Button/Button.tsx +++ b/packages/core-components/src/components/Button/Button.tsx @@ -44,7 +44,7 @@ const LinkWrapper = React.forwardRef((props, ref) => ( * @public * @remarks * - * Makes the Button to utilise react-router + * Makes the Button to utilize react-router */ export const Button = React.forwardRef((props, ref) => ( diff --git a/packages/core-components/src/components/Link/Link.stories.tsx b/packages/core-components/src/components/Link/Link.stories.tsx index d9944c1c51..6049ec68b1 100644 --- a/packages/core-components/src/components/Link/Link.stories.tsx +++ b/packages/core-components/src/components/Link/Link.stories.tsx @@ -15,7 +15,12 @@ */ import React, { ComponentType } from 'react'; import { Link } from './Link'; -import { Route, useLocation, NavLink as RouterNavLink } from 'react-router-dom'; +import { + Route, + useLocation, + NavLink as RouterNavLink, + Routes, +} from 'react-router-dom'; import { createRouteRef, useRouteRef } from '@backstage/core-plugin-api'; import { wrapInTestApp } from '@backstage/test-utils'; @@ -50,11 +55,11 @@ export const Default = () => { return ( <> - This link will utilise the react-router + This link will utilize the react-router MemoryRouter's navigation - -

Hi there!

-
+ + Hi there!} /> + ); }; @@ -75,9 +80,9 @@ export const PassProps = () => {  has props for both material-ui's component as well as for react-router-dom's - -

Hi there!

-
+ + Hi there!} /> + ); }; diff --git a/packages/core-components/src/components/Link/Link.test.tsx b/packages/core-components/src/components/Link/Link.test.tsx index 5cfeaa4869..52e687d2e4 100644 --- a/packages/core-components/src/components/Link/Link.test.tsx +++ b/packages/core-components/src/components/Link/Link.test.tsx @@ -33,10 +33,12 @@ describe('', () => { const linkText = 'Navigate!'; const { getByText } = render( wrapInTestApp( - + <> {linkText} - {testString}

} /> -
, + + {testString}

} /> +
+ , ), ); expect(() => getByText(testString)).toThrow(); @@ -116,8 +118,8 @@ describe('', () => { const { getByText } = render( wrapInTestApp( + {linkText} - {linkText} {testString}

} />
, @@ -141,8 +143,8 @@ describe('', () => { const { getByText } = render( wrapInTestApp( + {linkText} - {linkText} {testString}

} />
, diff --git a/packages/core-components/src/components/TabbedLayout/RoutedTabs.test.tsx b/packages/core-components/src/components/TabbedLayout/RoutedTabs.test.tsx index f76524fe50..15b00937a0 100644 --- a/packages/core-components/src/components/TabbedLayout/RoutedTabs.test.tsx +++ b/packages/core-components/src/components/TabbedLayout/RoutedTabs.test.tsx @@ -32,7 +32,7 @@ const testRoute2 = { const testRoute3 = { title: 'tabbed-test-title-3', - path: '/some-other-path-similar', + path: 'some-other-path-similar', children:
tabbed-test-content-3
, }; diff --git a/packages/core-components/src/components/TabbedLayout/RoutedTabs.tsx b/packages/core-components/src/components/TabbedLayout/RoutedTabs.tsx index 5debafd3f8..197cdb2239 100644 --- a/packages/core-components/src/components/TabbedLayout/RoutedTabs.tsx +++ b/packages/core-components/src/components/TabbedLayout/RoutedTabs.tsx @@ -41,7 +41,15 @@ export function useSelectedSubRoute(subRoutes: SubRoute[]): { const element = useRoutes(sortedRoutes) ?? subRoutes[0].children; - const [matchedRoute] = matchRoutes(sortedRoutes, `/${params['*']}`) ?? []; + // TODO(Rugvip): Once we only support v6 stable we can always prefix + // This avoids having a double / prefix for react-router v6 beta, which in turn breaks + // the tab highlighting when using relative paths for the tabs. + let currentRoute = params['*'] ?? ''; + if (!currentRoute.startsWith('/')) { + currentRoute = `/${currentRoute}`; + } + + const [matchedRoute] = matchRoutes(sortedRoutes, currentRoute) ?? []; const foundIndex = matchedRoute ? subRoutes.findIndex(t => `${t.path}/*` === matchedRoute.route.path) : 0; @@ -67,13 +75,16 @@ export function RoutedTabs(props: { routes: SubRoute[] }) { [routes], ); - const onTabChange = (tabIndex: number) => + const onTabChange = (tabIndex: number) => { + let { path } = routes[tabIndex]; // Remove trailing /* + path = path.replace(/\/\*$/, ''); // And remove leading / for relative navigation + path = path.replace(/^\//, ''); // Note! route resolves relative to the position in the React tree, // not relative to current location - navigate(routes[tabIndex].path.replace(/\/\*$/, '').replace(/^\//, '')); - + navigate(path); + }; return ( <> { @@ -59,24 +58,14 @@ describe('TabbedLayout', () => { it('navigates when user clicks different tab', async () => { const { getByText, queryByText, queryAllByRole } = await renderInTestApp( - - - -
tabbed-test-content
-
- -
tabbed-test-content-2
-
- - } - /> -
, + + +
tabbed-test-content
+
+ +
tabbed-test-content-2
+
+
, ); const secondTab = queryAllByRole('tab')[1]; diff --git a/packages/core-components/src/layout/Sidebar/Items.tsx b/packages/core-components/src/layout/Sidebar/Items.tsx index 0413864084..5a0b92f558 100644 --- a/packages/core-components/src/layout/Sidebar/Items.tsx +++ b/packages/core-components/src/layout/Sidebar/Items.tsx @@ -33,6 +33,7 @@ import classnames from 'classnames'; import React, { ComponentProps, ComponentType, + CSSProperties, forwardRef, KeyboardEventHandler, ReactNode, @@ -303,7 +304,7 @@ const sidebarSubmenuType = React.createElement(SidebarSubmenu).type; // properly yet, matching for example /foobar with /foo. export const WorkaroundNavLink = React.forwardRef< HTMLAnchorElement, - NavLinkProps + NavLinkProps & { activeStyle?: CSSProperties; activeClassName?: string } >(function WorkaroundNavLinkWithRef( { to, diff --git a/packages/core-plugin-api/package.json b/packages/core-plugin-api/package.json index 20e06593eb..26c490e894 100644 --- a/packages/core-plugin-api/package.json +++ b/packages/core-plugin-api/package.json @@ -39,12 +39,12 @@ "@backstage/version-bridge": "^1.0.1", "history": "^5.0.0", "prop-types": "^15.7.2", - "react-router-dom": "6.0.0-beta.0", "zen-observable": "^0.8.15" }, "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/packages/core-plugin-api/src/routing/useRouteRefParams.test.tsx b/packages/core-plugin-api/src/routing/useRouteRefParams.test.tsx index 430dc746a5..40ee7219c2 100644 --- a/packages/core-plugin-api/src/routing/useRouteRefParams.test.tsx +++ b/packages/core-plugin-api/src/routing/useRouteRefParams.test.tsx @@ -41,9 +41,7 @@ describe('useRouteRefParams', () => { const { getByText } = render( - - - + } /> , ); diff --git a/packages/dev-utils/package.json b/packages/dev-utils/package.json index 062ad64585..90cef137c1 100644 --- a/packages/dev-utils/package.json +++ b/packages/dev-utils/package.json @@ -48,14 +48,14 @@ "@testing-library/react": "^12.1.3", "@testing-library/user-event": "^14.0.0", "react-use": "^17.2.4", - "react-router": "6.0.0-beta.0", - "react-router-dom": "6.0.0-beta.0", "zen-observable": "^0.8.15" }, "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", "react": "^16.13.1 || ^17.0.0", - "react-dom": "^16.13.1 || ^17.0.0" + "react-dom": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/packages/techdocs-cli-embedded-app/package.json b/packages/techdocs-cli-embedded-app/package.json index e9263da8b5..d80536c02c 100644 --- a/packages/techdocs-cli-embedded-app/package.json +++ b/packages/techdocs-cli-embedded-app/package.json @@ -25,8 +25,8 @@ "history": "^5.0.0", "react": "^17.0.2", "react-dom": "^17.0.2", - "react-router": "6.0.0-beta.0", - "react-router-dom": "6.0.0-beta.0", + "react-router": "^6.3.0", + "react-router-dom": "^6.3.0", "react-use": "^17.2.4" }, "devDependencies": { diff --git a/packages/test-utils/package.json b/packages/test-utils/package.json index d2a22d2e05..a93c6e3268 100644 --- a/packages/test-utils/package.json +++ b/packages/test-utils/package.json @@ -46,13 +46,13 @@ "@testing-library/react": "^12.1.3", "@testing-library/user-event": "^14.0.0", "cross-fetch": "^3.1.5", - "react-router": "6.0.0-beta.0", - "react-router-dom": "6.0.0-beta.0", "zen-observable": "^0.8.15" }, "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/packages/test-utils/src/testUtils/appWrappers.tsx b/packages/test-utils/src/testUtils/appWrappers.tsx index fd22c1a150..3d2b5a0212 100644 --- a/packages/test-utils/src/testUtils/appWrappers.tsx +++ b/packages/test-utils/src/testUtils/appWrappers.tsx @@ -15,7 +15,7 @@ */ import React, { ComponentType, ReactNode, ReactElement } from 'react'; -import { MemoryRouter, Routes } from 'react-router'; +import { MemoryRouter } from 'react-router'; import { Route } from 'react-router-dom'; import { lightTheme } from '@backstage/theme'; import { ThemeProvider } from '@material-ui/core/styles'; @@ -184,11 +184,7 @@ export function createTestAppWrapper( {routeElements} - {/* The path of * here is needed to be set as a catch all, so it will render the wrapper element - * and work with nested routes if they exist too */} - - {children}} /> - + {children} ); diff --git a/plugins/adr/package.json b/plugins/adr/package.json index 3adae6db71..da659ced48 100644 --- a/plugins/adr/package.json +++ b/plugins/adr/package.json @@ -37,12 +37,12 @@ "git-url-parse": "^12.0.0", "octokit": "^2.0.0", "react-markdown": "^8.0.0", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4", "remark-gfm": "^3.0.1" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/airbrake/package.json b/plugins/airbrake/package.json index d75b4df303..7a7d4fd018 100644 --- a/plugins/airbrake/package.json +++ b/plugins/airbrake/package.json @@ -37,7 +37,8 @@ "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/app-defaults": "^1.0.6-next.0", @@ -51,8 +52,7 @@ "@types/node": "^16.11.26", "@types/object-hash": "^2.2.1", "cross-fetch": "^3.1.5", - "msw": "^0.45.0", - "react-router": "6.0.0-beta.0" + "msw": "^0.45.0" }, "files": [ "dist" diff --git a/plugins/airbrake/src/extensions.test.tsx b/plugins/airbrake/src/extensions.test.tsx index 05afe8f1e3..fd41f36930 100644 --- a/plugins/airbrake/src/extensions.test.tsx +++ b/plugins/airbrake/src/extensions.test.tsx @@ -15,7 +15,7 @@ */ import React from 'react'; import { EntityAirbrakeContent } from './extensions'; -import { Route } from 'react-router'; +import { Route, Routes } from 'react-router'; import { renderInTestApp, TestApiProvider } from '@backstage/test-utils'; import { airbrakeApiRef, MockAirbrakeApi } from './api'; import { createEntity } from './api'; @@ -26,7 +26,9 @@ describe('The Airbrake entity', () => { const rendered = await renderInTestApp( - } /> + + } /> + , ); diff --git a/plugins/allure/package.json b/plugins/allure/package.json index a336c3deff..f3ffa403b8 100644 --- a/plugins/allure/package.json +++ b/plugins/allure/package.json @@ -33,11 +33,11 @@ "@material-ui/core": "^4.12.2", "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.57", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/api-docs/package.json b/plugins/api-docs/package.json index ddf87f3ff7..897eded1a6 100644 --- a/plugins/api-docs/package.json +++ b/plugins/api-docs/package.json @@ -47,14 +47,14 @@ "graphql": "^16.0.0", "graphql-ws": "^5.4.1", "isomorphic-form-data": "^2.0.0", - "react-router": "6.0.0-beta.0", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4", "swagger-ui-react": "^4.11.1" }, "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/azure-devops/package.json b/plugins/azure-devops/package.json index d80e01fbe9..84e3d19348 100644 --- a/plugins/azure-devops/package.json +++ b/plugins/azure-devops/package.json @@ -42,11 +42,11 @@ "@material-ui/lab": "4.0.0-alpha.57", "humanize-duration": "^3.27.0", "luxon": "^3.0.0", - "react-router": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/badges/package.json b/plugins/badges/package.json index 8a850e321e..7e136aac72 100644 --- a/plugins/badges/package.json +++ b/plugins/badges/package.json @@ -39,11 +39,11 @@ "@material-ui/core": "^4.12.2", "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.57", - "react-router": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/bazaar/package.json b/plugins/bazaar/package.json index 4d39f92a09..90b244e9dd 100644 --- a/plugins/bazaar/package.json +++ b/plugins/bazaar/package.json @@ -40,11 +40,11 @@ "luxon": "^3.0.0", "material-ui-search-bar": "^1.0.0", "react-hook-form": "^7.13.0", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/catalog-graph/package.json b/plugins/catalog-graph/package.json index d74c10b50a..ad13d4bd63 100644 --- a/plugins/catalog-graph/package.json +++ b/plugins/catalog-graph/package.json @@ -37,12 +37,12 @@ "lodash": "^4.17.15", "p-limit": "^3.1.0", "qs": "^6.9.4", - "react-router": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/catalog-import/package.json b/plugins/catalog-import/package.json index 3e02888b94..6e99914ee8 100644 --- a/plugins/catalog-import/package.json +++ b/plugins/catalog-import/package.json @@ -51,13 +51,13 @@ "js-base64": "^3.6.0", "lodash": "^4.17.21", "react-hook-form": "^7.12.2", - "react-router": "6.0.0-beta.0", "react-use": "^17.2.4", "yaml": "^2.0.0" }, "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/catalog-react/package.json b/plugins/catalog-react/package.json index 291def38c6..fafac47a8c 100644 --- a/plugins/catalog-react/package.json +++ b/plugins/catalog-react/package.json @@ -53,14 +53,14 @@ "jwt-decode": "^3.1.0", "lodash": "^4.17.21", "qs": "^6.9.4", - "react-router": "6.0.0-beta.0", "react-use": "^17.2.4", "yaml": "^2.0.0", "zen-observable": "^0.8.15" }, "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/catalog/package.json b/plugins/catalog/package.json index 5465814ef9..edd69f2e2e 100644 --- a/plugins/catalog/package.json +++ b/plugins/catalog/package.json @@ -52,13 +52,13 @@ "history": "^5.0.0", "lodash": "^4.17.21", "react-helmet": "6.1.0", - "react-router": "6.0.0-beta.0", "react-use": "^17.2.4", "zen-observable": "^0.8.15" }, "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/catalog/src/components/EntityLayout/EntityLayout.test.tsx b/plugins/catalog/src/components/EntityLayout/EntityLayout.test.tsx index 0a3f443d3e..548fff3ff1 100644 --- a/plugins/catalog/src/components/EntityLayout/EntityLayout.test.tsx +++ b/plugins/catalog/src/components/EntityLayout/EntityLayout.test.tsx @@ -34,7 +34,6 @@ import { } from '@backstage/test-utils'; import { act, fireEvent } from '@testing-library/react'; import React from 'react'; -import { Route, Routes } from 'react-router'; import { EntityLayout } from './EntityLayout'; const mockEntity = { @@ -160,28 +159,21 @@ describe('EntityLayout', () => { it('navigates when user clicks different tab', async () => { const rendered = await renderInTestApp( - - - - - -
tabbed-test-content
-
- -
tabbed-test-content-2
-
-
-
- - } - /> -
, + + + + +
tabbed-test-content
+
+ +
tabbed-test-content-2
+
+
+
+
, { mountedRoutes: { '/catalog/:namespace/:kind/:name': entityRouteRef, diff --git a/plugins/circleci/package.json b/plugins/circleci/package.json index 23334570a2..4b0a7ca3e0 100644 --- a/plugins/circleci/package.json +++ b/plugins/circleci/package.json @@ -47,12 +47,12 @@ "humanize-duration": "^3.27.0", "lodash": "^4.17.21", "luxon": "^3.0.0", - "react-router": "6.0.0-beta.0", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/cloudbuild/package.json b/plugins/cloudbuild/package.json index ac65694c74..75ca4421b2 100644 --- a/plugins/cloudbuild/package.json +++ b/plugins/cloudbuild/package.json @@ -44,12 +44,12 @@ "@material-ui/lab": "4.0.0-alpha.57", "luxon": "^3.0.0", "qs": "^6.9.4", - "react-router": "6.0.0-beta.0", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/code-climate/package.json b/plugins/code-climate/package.json index 3631017c94..77586ecf52 100644 --- a/plugins/code-climate/package.json +++ b/plugins/code-climate/package.json @@ -33,11 +33,11 @@ "@material-ui/lab": "4.0.0-alpha.57", "humanize-duration": "^3.27.1", "luxon": "^3.0.0", - "react-router": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/code-coverage/package.json b/plugins/code-coverage/package.json index 62aaf4c5ba..2159ae56d0 100644 --- a/plugins/code-coverage/package.json +++ b/plugins/code-coverage/package.json @@ -37,13 +37,13 @@ "@material-ui/styles": "^4.11.0", "highlight.js": "^10.6.0", "luxon": "^3.0.0", - "react-router": "6.0.0-beta.0", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4", "recharts": "^1.8.5" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/codescene/package.json b/plugins/codescene/package.json index af2f380a26..4e304eb221 100644 --- a/plugins/codescene/package.json +++ b/plugins/codescene/package.json @@ -31,11 +31,11 @@ "@material-ui/icons": "^4.9.1", "@material-ui/lab": "^4.0.0-alpha.57", "rc-progress": "3.4.0", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/cost-insights/package.json b/plugins/cost-insights/package.json index a737688d74..ffde84d26c 100644 --- a/plugins/cost-insights/package.json +++ b/plugins/cost-insights/package.json @@ -50,7 +50,6 @@ "luxon": "^3.0.0", "pluralize": "^8.0.0", "qs": "^6.9.4", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4", "recharts": "^1.8.5", "regression": "^2.0.1", @@ -58,7 +57,8 @@ }, "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/explore/package.json b/plugins/explore/package.json index db91907e25..10ee68c95f 100644 --- a/plugins/explore/package.json +++ b/plugins/explore/package.json @@ -44,13 +44,13 @@ "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.57", "classnames": "^2.2.6", - "react-router": "6.0.0-beta.0", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/gcp-projects/package.json b/plugins/gcp-projects/package.json index 56c68effee..8b8d1d4859 100644 --- a/plugins/gcp-projects/package.json +++ b/plugins/gcp-projects/package.json @@ -40,11 +40,11 @@ "@material-ui/core": "^4.12.2", "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.57", - "@react-hookz/web": "^15.0.0", - "react-router-dom": "6.0.0-beta.0" + "@react-hookz/web": "^15.0.0" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/git-release-manager/package.json b/plugins/git-release-manager/package.json index b6c95e3f79..3d95f05211 100644 --- a/plugins/git-release-manager/package.json +++ b/plugins/git-release-manager/package.json @@ -34,13 +34,13 @@ "@octokit/rest": "^19.0.3", "luxon": "^3.0.0", "qs": "^6.10.1", - "react-router": "6.0.0-beta.0", "react-use": "^17.2.4", "recharts": "^1.8.5" }, "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/github-actions/package.json b/plugins/github-actions/package.json index 18adf36085..a85c7e3a4c 100644 --- a/plugins/github-actions/package.json +++ b/plugins/github-actions/package.json @@ -47,12 +47,12 @@ "@material-ui/lab": "4.0.0-alpha.57", "@octokit/rest": "^19.0.3", "luxon": "^3.0.0", - "react-router": "6.0.0-beta.0", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/gitops-profiles/package.json b/plugins/gitops-profiles/package.json index 23796ad3e9..7f92b6e702 100644 --- a/plugins/gitops-profiles/package.json +++ b/plugins/gitops-profiles/package.json @@ -41,11 +41,11 @@ "@material-ui/core": "^4.12.2", "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.57", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/graphiql/package.json b/plugins/graphiql/package.json index a78ba4d5d7..417694fbd5 100644 --- a/plugins/graphiql/package.json +++ b/plugins/graphiql/package.json @@ -46,7 +46,8 @@ "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", @@ -60,8 +61,7 @@ "@types/jest": "^26.0.7", "@types/node": "^16.11.26", "cross-fetch": "^3.1.5", - "msw": "^0.45.0", - "react-router-dom": "6.0.0-beta.0" + "msw": "^0.45.0" }, "files": [ "dist" diff --git a/plugins/home/package.json b/plugins/home/package.json index e02d6d3e34..2a56904e7c 100644 --- a/plugins/home/package.json +++ b/plugins/home/package.json @@ -45,12 +45,12 @@ "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.57", "lodash": "^4.17.21", - "react-router": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/jenkins/package.json b/plugins/jenkins/package.json index 6d53c7fc78..e9afa65665 100644 --- a/plugins/jenkins/package.json +++ b/plugins/jenkins/package.json @@ -46,12 +46,12 @@ "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.57", "luxon": "^3.0.0", - "react-router": "6.0.0-beta.0", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/kafka/package.json b/plugins/kafka/package.json index 0dfb4a2134..a290eca265 100644 --- a/plugins/kafka/package.json +++ b/plugins/kafka/package.json @@ -34,11 +34,11 @@ "@material-ui/core": "^4.12.2", "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.57", - "react-router": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/kubernetes/package.json b/plugins/kubernetes/package.json index 41be1bdf77..34ecb7d3e4 100644 --- a/plugins/kubernetes/package.json +++ b/plugins/kubernetes/package.json @@ -50,11 +50,11 @@ "js-yaml": "^4.0.0", "lodash": "^4.17.21", "luxon": "^3.0.0", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/lighthouse/package.json b/plugins/lighthouse/package.json index 720c8a3a09..994109398c 100644 --- a/plugins/lighthouse/package.json +++ b/plugins/lighthouse/package.json @@ -44,11 +44,11 @@ "@material-ui/core": "^4.12.2", "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.57", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/org/package.json b/plugins/org/package.json index bc871ac480..33668fbee3 100644 --- a/plugins/org/package.json +++ b/plugins/org/package.json @@ -40,12 +40,12 @@ "p-limit": "^3.1.0", "pluralize": "^8.0.0", "qs": "^6.10.1", - "react-router": "6.0.0-beta.0", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/catalog-client": "^1.0.5-next.0", diff --git a/plugins/pagerduty/package.json b/plugins/pagerduty/package.json index 714b67d879..7567424e5e 100644 --- a/plugins/pagerduty/package.json +++ b/plugins/pagerduty/package.json @@ -45,12 +45,12 @@ "@material-ui/lab": "4.0.0-alpha.57", "classnames": "^2.2.6", "luxon": "^3.0.0", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/permission-react/api-report.md b/plugins/permission-react/api-report.md index 12f1300c70..293343b7ac 100644 --- a/plugins/permission-react/api-report.md +++ b/plugins/permission-react/api-report.md @@ -6,7 +6,6 @@ import { ApiRef } from '@backstage/core-plugin-api'; import { AuthorizePermissionRequest } from '@backstage/plugin-permission-common'; import { AuthorizePermissionResponse } from '@backstage/plugin-permission-common'; -import { ComponentProps } from 'react'; import { Config } from '@backstage/config'; import { DiscoveryApi } from '@backstage/core-plugin-api'; import { EvaluatePermissionRequest } from '@backstage/plugin-permission-common'; @@ -14,8 +13,8 @@ import { EvaluatePermissionResponse } from '@backstage/plugin-permission-common' import { IdentityApi } from '@backstage/core-plugin-api'; import { Permission } from '@backstage/plugin-permission-common'; import { ReactElement } from 'react'; +import { ReactNode } from 'react'; import { ResourcePermission } from '@backstage/plugin-permission-common'; -import { Route } from 'react-router'; // @public (undocumented) export type AsyncPermissionResult = { @@ -48,22 +47,46 @@ export type PermissionApi = { // @public export const permissionApiRef: ApiRef; -// @public +// @public @deprecated export const PermissionedRoute: ( - props: ComponentProps & { + props: { + caseSensitive?: boolean; + children?: ReactNode; + element?: ReactElement | null; + path?: string; errorComponent?: ReactElement | null; } & ( - | { - permission: Exclude; - resourceRef?: never; - } - | { - permission: ResourcePermission; - resourceRef: string | undefined; - } - ), + | { + permission: Exclude; + resourceRef?: never; + } + | { + permission: ResourcePermission; + resourceRef: string | undefined; + } + ), ) => JSX.Element; +// @public +export function RequirePermission( + props: RequirePermissionProps, +): JSX.Element | null; + +// @public +export type RequirePermissionProps = ( + | { + permission: Exclude; + resourceRef?: never; + } + | { + permission: ResourcePermission; + resourceRef: string | undefined; + } +) & { + errorPage?: ReactNode; + children: ReactNode; +}; + // @public export function usePermission( input: diff --git a/plugins/permission-react/package.json b/plugins/permission-react/package.json index 86b7f5297d..7b13ecdf73 100644 --- a/plugins/permission-react/package.json +++ b/plugins/permission-react/package.json @@ -35,13 +35,13 @@ "@backstage/core-plugin-api": "^1.0.6-next.0", "@backstage/plugin-permission-common": "^0.6.4-next.0", "cross-fetch": "^3.1.5", - "react-router": "6.0.0-beta.0", "react-use": "^17.2.4", "swr": "^1.1.2" }, "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/permission-react/src/components/PermissionedRoute.tsx b/plugins/permission-react/src/components/PermissionedRoute.tsx index dd39c5bb4f..004a623031 100644 --- a/plugins/permission-react/src/components/PermissionedRoute.tsx +++ b/plugins/permission-react/src/components/PermissionedRoute.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { ComponentProps, ReactElement } from 'react'; +import React, { ReactElement, ReactNode } from 'react'; import { Route } from 'react-router'; import { useApp } from '@backstage/core-plugin-api'; import { usePermission } from '../hooks'; @@ -29,20 +29,25 @@ import { * NotFoundErrorPage (see {@link @backstage/core-app-api#AppComponents}). * * @public + * @deprecated This component no longer works with the most recent version of `@backstage/core-app-api` and react-router v6, use {@link RequirePermission} instead. */ export const PermissionedRoute = ( - props: ComponentProps & { + props: { + caseSensitive?: boolean; + children?: ReactNode; + element?: ReactElement | null; + path?: string; errorComponent?: ReactElement | null; } & ( - | { - permission: Exclude; - resourceRef?: never; - } - | { - permission: ResourcePermission; - resourceRef: string | undefined; - } - ), + | { + permission: Exclude; + resourceRef?: never; + } + | { + permission: ResourcePermission; + resourceRef: string | undefined; + } + ), ) => { const { permission, resourceRef, errorComponent, ...otherProps } = props; diff --git a/plugins/permission-react/src/components/PermissionedRoute.test.tsx b/plugins/permission-react/src/components/RequirePermission.test.tsx similarity index 85% rename from plugins/permission-react/src/components/PermissionedRoute.test.tsx rename to plugins/permission-react/src/components/RequirePermission.test.tsx index 9159227925..262a602ead 100644 --- a/plugins/permission-react/src/components/PermissionedRoute.test.tsx +++ b/plugins/permission-react/src/components/RequirePermission.test.tsx @@ -15,7 +15,7 @@ */ import React from 'react'; -import { PermissionedRoute } from '.'; +import { RequirePermission } from './RequirePermission'; import { usePermission } from '../hooks'; import { renderInTestApp } from '@backstage/test-utils'; import { createPermission } from '@backstage/plugin-permission-common'; @@ -32,14 +32,14 @@ const permission = createPermission({ attributes: { action: 'read' }, }); -describe('PermissionedRoute', () => { +describe('RequirePermission', () => { it('Does not render when loading', async () => { mockUsePermission.mockReturnValue({ loading: true, allowed: false }); const { queryByText } = await renderInTestApp( - content
} + children={
content
} />, ); @@ -50,9 +50,9 @@ describe('PermissionedRoute', () => { mockUsePermission.mockReturnValue({ loading: false, allowed: true }); const { getByText } = await renderInTestApp( - content
} + children={
content
} />, ); @@ -64,9 +64,9 @@ describe('PermissionedRoute', () => { await expect( renderInTestApp( - content
} + children={
content
} />, ), ).rejects.toThrowError('Reached NotFound Page'); @@ -76,10 +76,10 @@ describe('PermissionedRoute', () => { mockUsePermission.mockReturnValue({ loading: false, allowed: false }); const { getByText } = await renderInTestApp( - content
} - errorComponent={

Custom Error

} + children={
content
} + errorPage={

Custom Error

} />, ); diff --git a/plugins/permission-react/src/components/RequirePermission.tsx b/plugins/permission-react/src/components/RequirePermission.tsx new file mode 100644 index 0000000000..801eb0fe24 --- /dev/null +++ b/plugins/permission-react/src/components/RequirePermission.tsx @@ -0,0 +1,82 @@ +/* + * Copyright 2021 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 React, { ReactNode } from 'react'; +import { useApp } from '@backstage/core-plugin-api'; +import { usePermission } from '../hooks'; +import { + isResourcePermission, + Permission, + ResourcePermission, +} from '@backstage/plugin-permission-common'; + +/** + * Properties for {@link RequirePermission} + * + * @public + */ +export type RequirePermissionProps = ( + | { + permission: Exclude; + resourceRef?: never; + } + | { + permission: ResourcePermission; + resourceRef: string | undefined; + } +) & { + /** + * The error page to be displayed if the user is not allowed access. + * + * Defaults to the `NotFoundErrorPage` app component. + */ + errorPage?: ReactNode; + children: ReactNode; +}; + +/** + * A boundary that only renders its child elements if the user has the specified permission. + * + * While loading, nothing will be rendered. If the user does not have + * permission, the `errorPage` element will be rendered, falling back + * to the `NotFoundErrorPage` app component if no `errorPage` is provider. + * + * @public + */ +export function RequirePermission( + props: RequirePermissionProps, +): JSX.Element | null { + const { permission, resourceRef } = props; + const permissionResult = usePermission( + isResourcePermission(permission) + ? { permission, resourceRef } + : { permission }, + ); + const app = useApp(); + + if (permissionResult.loading) { + return null; + } else if (permissionResult.allowed) { + return <>{props.children}; + } + + if (props.errorPage) { + return <>{props.errorPage}; + } + // If no explicit error element is provided, the not found page is used as fallback. + const { NotFoundErrorPage } = app.getComponents(); + return ; +} diff --git a/plugins/permission-react/src/components/index.ts b/plugins/permission-react/src/components/index.ts index 05f13bca81..e443ad0b5e 100644 --- a/plugins/permission-react/src/components/index.ts +++ b/plugins/permission-react/src/components/index.ts @@ -15,3 +15,5 @@ */ export { PermissionedRoute } from './PermissionedRoute'; +export { RequirePermission } from './RequirePermission'; +export type { RequirePermissionProps } from './RequirePermission'; diff --git a/plugins/rollbar/package.json b/plugins/rollbar/package.json index d31dd16bcb..3dad5d4db3 100644 --- a/plugins/rollbar/package.json +++ b/plugins/rollbar/package.json @@ -44,13 +44,13 @@ "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.57", "lodash": "^4.17.21", - "react-router": "6.0.0-beta.0", - "react-router-dom": "6.0.0-beta.0", "react-sparklines": "^1.7.0", "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/scaffolder/package.json b/plugins/scaffolder/package.json index 4200e2d3ff..3c948c6612 100644 --- a/plugins/scaffolder/package.json +++ b/plugins/scaffolder/package.json @@ -69,8 +69,6 @@ "lodash": "^4.17.21", "luxon": "^3.0.0", "qs": "^6.9.4", - "react-router": "6.0.0-beta.0", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4", "use-immer": "^0.7.0", "yaml": "^2.0.0", @@ -78,7 +76,9 @@ }, "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/scaffolder/src/components/Router.tsx b/plugins/scaffolder/src/components/Router.tsx index 0aa04b93d0..05633787b5 100644 --- a/plugins/scaffolder/src/components/Router.tsx +++ b/plugins/scaffolder/src/components/Router.tsx @@ -125,6 +125,7 @@ export const Router = (props: RouterProps) => { return ( { /> } /> - - - + } + /> { undefined as any, ); - const rendered = await renderWithEffects( + const rendered = await renderInTestApp( - - - - - - This is root} /> - - + + } /> + This is root} /> + , + { + routeEntries: ['/create'], + mountedRoutes: { '/create': rootRouteRef }, + }, ); expect( diff --git a/plugins/scaffolder/src/next/Router/Router.tsx b/plugins/scaffolder/src/next/Router/Router.tsx index 91efe850e0..758e268cb3 100644 --- a/plugins/scaffolder/src/next/Router/Router.tsx +++ b/plugins/scaffolder/src/next/Router/Router.tsx @@ -78,6 +78,7 @@ export const Router = (props: PropsWithChildren) => { return ( { await act(async () => { await render( - wrapInTestApp( - - } /> - , - { - mountedRoutes: { - '/search': rootRouteRef, - }, + wrapInTestApp(, { + mountedRoutes: { + '/search': rootRouteRef, }, - ), + }), ); expect(navigate).toHaveBeenCalledTimes(1); diff --git a/plugins/sentry/package.json b/plugins/sentry/package.json index c0d3df778f..ee48f7e090 100644 --- a/plugins/sentry/package.json +++ b/plugins/sentry/package.json @@ -45,12 +45,12 @@ "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.57", "luxon": "^3.0.0", - "react-router": "6.0.0-beta.0", "react-sparklines": "^1.7.0", "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/shortcuts/package.json b/plugins/shortcuts/package.json index f02a7ba68a..8476197c03 100644 --- a/plugins/shortcuts/package.json +++ b/plugins/shortcuts/package.json @@ -33,13 +33,13 @@ "@material-ui/lab": "4.0.0-alpha.57", "@types/zen-observable": "^0.8.2", "react-hook-form": "^7.12.2", - "react-router": "6.0.0-beta.0", "react-use": "^17.2.4", "uuid": "^8.3.2", "zen-observable": "^0.8.15" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/splunk-on-call/package.json b/plugins/splunk-on-call/package.json index 0bf2d7daed..5ae1da0823 100644 --- a/plugins/splunk-on-call/package.json +++ b/plugins/splunk-on-call/package.json @@ -44,11 +44,11 @@ "@material-ui/lab": "4.0.0-alpha.57", "classnames": "^2.2.6", "luxon": "^3.0.0", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/tech-insights/package.json b/plugins/tech-insights/package.json index ab4477acf1..5cc08c9164 100644 --- a/plugins/tech-insights/package.json +++ b/plugins/tech-insights/package.json @@ -39,12 +39,12 @@ "@material-ui/core": "^4.12.2", "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.57", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/techdocs-addons-test-utils/package.json b/plugins/techdocs-addons-test-utils/package.json index 7d87ca2e52..04935376fa 100644 --- a/plugins/techdocs-addons-test-utils/package.json +++ b/plugins/techdocs-addons-test-utils/package.json @@ -46,14 +46,14 @@ "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.57", "react-use": "^17.2.4", - "react-router-dom": "6.0.0-beta.0", "@testing-library/react": "^12.1.3", "testing-library__dom": "^7.29.4-beta.1" }, "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", "react": "^16.13.1 || ^17.0.0", - "react-dom": "^16.13.1 || ^17.0.0" + "react-dom": "^16.13.1 || ^17.0.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/techdocs-react/package.json b/plugins/techdocs-react/package.json index 452838284e..0ed196cf5e 100644 --- a/plugins/techdocs-react/package.json +++ b/plugins/techdocs-react/package.json @@ -46,12 +46,12 @@ "jss": "~10.8.2", "lodash": "^4.17.21", "react-helmet": "6.1.0", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@testing-library/jest-dom": "^5.10.1", diff --git a/plugins/techdocs/package.json b/plugins/techdocs/package.json index 604fec15ed..4385818c95 100644 --- a/plugins/techdocs/package.json +++ b/plugins/techdocs/package.json @@ -57,14 +57,14 @@ "jss": "~10.8.2", "lodash": "^4.17.21", "react-helmet": "6.1.0", - "react-router": "6.0.0-beta.0", - "react-router-dom": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { "@types/react": "^16.13.1 || ^17.0.0", "react": "^16.13.1 || ^17.0.0", - "react-dom": "^16.13.1 || ^17.0.0" + "react-dom": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0", + "react-router-dom": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/plugins/techdocs/src/reader/components/TechDocsReaderPage/TechDocsReaderPage.tsx b/plugins/techdocs/src/reader/components/TechDocsReaderPage/TechDocsReaderPage.tsx index cbeb667417..1c276b87a1 100644 --- a/plugins/techdocs/src/reader/components/TechDocsReaderPage/TechDocsReaderPage.tsx +++ b/plugins/techdocs/src/reader/components/TechDocsReaderPage/TechDocsReaderPage.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { ReactNode, ReactChild, Children } from 'react'; +import React, { ReactNode, Children, ReactElement } from 'react'; import { useOutlet } from 'react-router-dom'; import { Page } from '@backstage/core-components'; @@ -30,15 +30,10 @@ import { TechDocsReaderPageContent } from '../TechDocsReaderPageContent'; import { TechDocsReaderPageHeader } from '../TechDocsReaderPageHeader'; import { TechDocsReaderPageSubheader } from '../TechDocsReaderPageSubheader'; import { rootDocsRouteRef } from '../../../routes'; -import { useRouteRefParams } from '@backstage/core-plugin-api'; - -type Extension = ReactChild & { - type: { - __backstage_data: { - map: Map; - }; - }; -}; +import { + getComponentData, + useRouteRefParams, +} from '@backstage/core-plugin-api'; /** * Props for {@link TechDocsReaderLayout} @@ -94,8 +89,18 @@ export const TechDocsReaderPage = (props: TechDocsReaderPageProps) => { const childrenList = outlet ? Children.toArray(outlet.props.children) : []; const page = childrenList.find(child => { - const { type } = child as Extension; - return !type?.__backstage_data?.map?.get(TECHDOCS_ADDONS_WRAPPER_KEY); + if (getComponentData(child, TECHDOCS_ADDONS_WRAPPER_KEY)) { + return false; + } + + // react-router 6 stable wraps children in a routing context provider, so check one level deeper + const nestedChildren = (child as ReactElement)?.props?.children; + if (nestedChildren) { + return !Children.toArray(nestedChildren).some(nested => + getComponentData(nested, TECHDOCS_ADDONS_WRAPPER_KEY), + ); + } + return true; }); return ( diff --git a/plugins/todo/package.json b/plugins/todo/package.json index 66f367f189..9677a3eb30 100644 --- a/plugins/todo/package.json +++ b/plugins/todo/package.json @@ -42,7 +42,8 @@ "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", @@ -55,8 +56,7 @@ "@types/jest": "^26.0.7", "@types/node": "^16.11.26", "cross-fetch": "^3.1.5", - "msw": "^0.45.0", - "react-router": "6.0.0-beta.0" + "msw": "^0.45.0" }, "files": [ "dist" diff --git a/plugins/todo/src/plugin.test.tsx b/plugins/todo/src/plugin.test.tsx index af4704c16f..40dfdfbff2 100644 --- a/plugins/todo/src/plugin.test.tsx +++ b/plugins/todo/src/plugin.test.tsx @@ -15,7 +15,7 @@ */ import React from 'react'; -import { Route } from 'react-router'; +import { Route, Routes } from 'react-router'; import { renderInTestApp, TestApiProvider } from '@backstage/test-utils'; import { todoPlugin, EntityTodoContent } from './plugin'; import { todoApiRef } from './api'; @@ -55,7 +55,9 @@ describe('todo', () => { metadata: { name: 'Test TODO' }, }} > - } /> + + } /> + , ); diff --git a/plugins/user-settings/package.json b/plugins/user-settings/package.json index 6e73ba6499..e23c9d2ed4 100644 --- a/plugins/user-settings/package.json +++ b/plugins/user-settings/package.json @@ -41,11 +41,11 @@ "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.57", "@types/react": "^16.13.1 || ^17.0.0", - "react-router": "6.0.0-beta.0", "react-use": "^17.2.4" }, "peerDependencies": { - "react": "^16.13.1 || ^17.0.0" + "react": "^16.13.1 || ^17.0.0", + "react-router": "6.0.0-beta.0 || ^6.3.0" }, "devDependencies": { "@backstage/cli": "^0.18.2-next.0", diff --git a/yarn.lock b/yarn.lock index 975bd1b33d..c87e137b3b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -13505,8 +13505,8 @@ evp_bytestokey@^1.0.0, evp_bytestokey@^1.0.3: prop-types "^15.7.2" react "^17.0.2" react-dom "^17.0.2" - react-router "6.0.0-beta.0" - react-router-dom "6.0.0-beta.0" + react-router "^6.3.0" + react-router-dom "^6.3.0" react-use "^17.2.4" zen-observable "^0.8.15" @@ -22535,7 +22535,14 @@ react-resize-detector@^6.6.3: lodash "^4.17.21" resize-observer-polyfill "^1.5.1" -react-router-dom@6.0.0-beta.0: +"react-router-beta@npm:react-router@6.0.0-beta.0", react-router@6.0.0-beta.0: + version "6.0.0-beta.0" + resolved "https://registry.npmjs.org/react-router/-/react-router-6.0.0-beta.0.tgz#3e11f39b6ded4412c2fed9e4f989dd4c8156724d" + integrity sha512-VgMdfpVcmFQki/LZuLh8E/MNACekDetz4xqft+a6fBZvvJnVqKbLqebF7hyoawGrV1HcO5tVaUang2Og4W2j1Q== + dependencies: + prop-types "^15.7.2" + +"react-router-dom-beta@npm:react-router-dom@6.0.0-beta.0", react-router-dom@6.0.0-beta.0: version "6.0.0-beta.0" resolved "https://registry.npmjs.org/react-router-dom/-/react-router-dom-6.0.0-beta.0.tgz#9dcc8555365f22f7fbd09f26b6b82543f3eb97d6" integrity sha512-36yNNGMT8RB9FRPL9nKJi6HKDkgOakU+o/2hHpSzR6e37gN70MpOU6QQlmif4oAWWBwjyGc3ZNOMFCsFuHUY5w== @@ -22543,17 +22550,18 @@ react-router-dom@6.0.0-beta.0: prop-types "^15.7.2" react-router "6.0.0-beta.0" -react-router@6.0.0-beta.0: - version "6.0.0-beta.0" - resolved "https://registry.npmjs.org/react-router/-/react-router-6.0.0-beta.0.tgz#3e11f39b6ded4412c2fed9e4f989dd4c8156724d" - integrity sha512-VgMdfpVcmFQki/LZuLh8E/MNACekDetz4xqft+a6fBZvvJnVqKbLqebF7hyoawGrV1HcO5tVaUang2Og4W2j1Q== +"react-router-dom-stable@npm:react-router-dom@^6.3.0", react-router-dom@^6.3.0: + version "6.3.0" + resolved "https://registry.npmjs.org/react-router-dom/-/react-router-dom-6.3.0.tgz#a0216da813454e521905b5fa55e0e5176123f43d" + integrity sha512-uaJj7LKytRxZNQV8+RbzJWnJ8K2nPsOOEuX7aQstlMZKQT0164C+X2w6bnkqU3sjtLvpd5ojrezAyfZ1+0sStw== dependencies: - prop-types "^15.7.2" + history "^5.2.0" + react-router "6.3.0" -react-router@^6.0.0-beta.0: - version "6.2.1" - resolved "https://registry.npmjs.org/react-router/-/react-router-6.2.1.tgz#be2a97a6006ce1d9123c28934e604faef51448a3" - integrity sha512-2fG0udBtxou9lXtK97eJeET2ki5//UWfQSl1rlJ7quwe6jrktK9FCCc8dQb5QY6jAv3jua8bBQRhhDOM/kVRsg== +"react-router-stable@npm:react-router@^6.3.0", react-router@6.3.0, react-router@^6.0.0-beta.0, react-router@^6.3.0: + version "6.3.0" + resolved "https://registry.npmjs.org/react-router/-/react-router-6.3.0.tgz#3970cc64b4cb4eae0c1ea5203a80334fdd175557" + integrity sha512-7Wh1DzVQ+tlFjkeo+ujvjSqSJmkt1+8JO+T5xklPlgrh70y7ogx75ODRW0ThWhY7S+6yEDks8TYrtQe/aoboBQ== dependencies: history "^5.2.0" @@ -25198,8 +25206,8 @@ tdigest@^0.1.1: history "^5.0.0" react "^17.0.2" react-dom "^17.0.2" - react-router "6.0.0-beta.0" - react-router-dom "6.0.0-beta.0" + react-router "^6.3.0" + react-router-dom "^6.3.0" react-use "^17.2.4" teeny-request@^8.0.0: