Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core-*-api: backport support for external route ref default targets + config route binding #24698

Merged
merged 10 commits into from
May 15, 2024
5 changes: 5 additions & 0 deletions .changeset/chilled-planes-sniff.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@backstage/plugin-api-docs': patch
---

The `registerComponent` external route will now by default bind to the catalog import page if it is available.
5 changes: 5 additions & 0 deletions .changeset/cuddly-jobs-kick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@backstage/core-compat-api': patch
---

Add support for forwarding default target from legacy external route refs.
5 changes: 5 additions & 0 deletions .changeset/green-apricots-invite.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@backstage/plugin-org': patch
---

The `catalogIndex` external route is now optional and will by default bind to the catalog index page if it is available.
5 changes: 5 additions & 0 deletions .changeset/honest-pandas-chew.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@backstage/core-plugin-api': patch
---

Added a new `defaultTarget` option to `createExternalRouteRef`. I lets you specify a default target of the route by name, for example `'catalog.catalogIndex'`, which will be used if the target route is present in the app and there is no explicit route binding.
5 changes: 5 additions & 0 deletions .changeset/proud-readers-move.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@backstage/plugin-catalog-graph': patch
---

The `catalogEntity` external route will now by default bind to the catalog entity page if it is available.
9 changes: 9 additions & 0 deletions .changeset/rich-teachers-agree.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@backstage/plugin-catalog': minor
---

Added the following default targets for external routes:

- `createComponent` binds to the Scaffolder page.
- `viewTechDoc` binds to the TechDocs entity documentation page.
- `createFromTemplate` binds to the Scaffolder selected template page.
29 changes: 29 additions & 0 deletions .changeset/serious-yaks-sit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
'@backstage/core-app-api': patch
---

Added support for configuration of route bindings through static configuration, and default targets for external route refs.

In addition to configuring route bindings through code, it is now also possible to configure route bindings under the `app.routes.bindings` key, for example:

```yaml
app:
routes:
bindings:
catalog.createComponent: catalog-import.importPage
```

Each key in the route binding object is of the form `<plugin-id>.<externalRouteName>`, where the route name is key used in the `externalRoutes` object passed to `createPlugin`. The value is of the same form, but with the name taken from the plugin `routes` option instead.

The equivalent of the above configuration in code is the following:

```ts
const app = createApp({
// ...
bindRoutes({ bind }) {
bind(catalogPlugin.externalRoutes, {
createComponent: catalogImportPlugin.routes.importPage,
});
},
});
```
8 changes: 8 additions & 0 deletions .changeset/warm-mayflies-yell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@backstage/plugin-scaffolder': minor
---

Added the following default targets for external routes:

- `registerComponent` binds to the catalog import page.
- `viewTechDoc` binds to the TechDocs entity documentation page.
15 changes: 15 additions & 0 deletions docs/frontend-system/architecture/07-routes.md
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,21 @@ Note that we are not importing and using the `RouteRef`s directly in the app, an

Another thing to note is that this indirection in the routing is particularly useful for open source plugins that need to provide flexibility in how they are integrated. For plugins that you build internally for your own Backstage application, you can choose to use direct imports or even concrete route path strings directly. Although there can be some benefits to using the full routing system even in internal plugins: it can help you structure your routes, and as you will see further down it also helps you manage route parameters.

### Default Targets for External Route References

It is possible to define a default target for an external route reference, potentially removing the need to bind the route in the app. This reduces the need for configuration when installing new plugins through providing a sensible default. It is of course still possible to override the route binding in the app.

The default target uses the same syntax as the route binding configuration, and will only be used if the target plugin an route exists. For example, this is how the catalog can define a default target for the create component external route in a way that removes the need for the binding in the previous example:

```tsx title="plugins/catalog/src/routes.ts"
import { createExternalRouteRef } from '@backstage/frontend-plugin-api';

export const createComponentExternalRouteRef = createExternalRouteRef({
// highlight-next-line
defaultTarget: 'scaffolder.createComponent',
});
```

### Optional External Route References

It is possible to define an `ExternalRouteRef` as optional, so it is not required to bind it in the app.
Expand Down
27 changes: 27 additions & 0 deletions docs/plugins/composability.md
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,33 @@ concrete routes directly. Although there can be some benefits to using the full
routing system even in internal plugins. It can help you structure your routes,
and as you will see further down it also helps you manage route parameters.

You can also use static configuration to bind routes, removing the need to make
changes to the app code. It does however mean that you won't get type safety
when binding routes and compile-time validation of the bindings. Static
configuration of route bindings is done under the `app.routes.bindings` key in
`app-config.yaml`. It works the same way as [route bindings in the new frontend system](../frontend-system/architecture/07-routes.md#binding-external-route-references),
for example:

```yaml
app:
routes:
bindings:
bar.headerLink: foo.root
```

### Default Targets for External Route References

Following the `1.28` release of Backstage you can now define default targets for
external route references. They work the same way as [default targets in the new frontend system](../frontend-system/architecture/07-routes.md#default-targets-for-external-route-references),
for example:

```ts
export const createComponentExternalRouteRef = createExternalRouteRef({
// highlight-next-line
defaultTarget: 'scaffolder.createComponent',
});
```

### Optional External Routes

When creating an `ExternalRouteRef` it is possible to mark it as optional:
Expand Down
34 changes: 4 additions & 30 deletions packages/app/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,30 +33,21 @@ import {
OAuthRequestDialog,
SignInPage,
} from '@backstage/core-components';
import { apiDocsPlugin, ApiExplorerPage } from '@backstage/plugin-api-docs';
import {
CatalogEntityPage,
CatalogIndexPage,
catalogPlugin,
} from '@backstage/plugin-catalog';
import { ApiExplorerPage } from '@backstage/plugin-api-docs';
import { CatalogEntityPage, CatalogIndexPage } from '@backstage/plugin-catalog';

import { CatalogGraphPage } from '@backstage/plugin-catalog-graph';
import {
CatalogImportPage,
catalogImportPlugin,
} from '@backstage/plugin-catalog-import';
import { orgPlugin } from '@backstage/plugin-org';
import { CatalogImportPage } from '@backstage/plugin-catalog-import';
import { HomepageCompositionRoot, VisitListener } from '@backstage/plugin-home';

import { ScaffolderPage, scaffolderPlugin } from '@backstage/plugin-scaffolder';
import { ScaffolderPage } from '@backstage/plugin-scaffolder';
import {
ScaffolderFieldExtensions,
ScaffolderLayouts,
} from '@backstage/plugin-scaffolder-react';
import { SearchPage } from '@backstage/plugin-search';
import {
TechDocsIndexPage,
techdocsPlugin,
TechDocsReaderPage,
} from '@backstage/plugin-techdocs';
import { TechDocsAddons } from '@backstage/plugin-techdocs-react';
Expand Down Expand Up @@ -119,23 +110,6 @@ const app = createApp({
);
},
},
bindRoutes({ bind }) {
bind(catalogPlugin.externalRoutes, {
createComponent: scaffolderPlugin.routes.root,
viewTechDoc: techdocsPlugin.routes.docRoot,
createFromTemplate: scaffolderPlugin.routes.selectedTemplate,
});
bind(apiDocsPlugin.externalRoutes, {
registerApi: catalogImportPlugin.routes.importPage,
});
bind(scaffolderPlugin.externalRoutes, {
registerComponent: catalogImportPlugin.routes.importPage,
viewTechDoc: techdocsPlugin.routes.docRoot,
});
bind(orgPlugin.externalRoutes, {
catalogIndex: catalogPlugin.routes.catalogIndex,
});
},
});

const routes = (
Expand Down
85 changes: 54 additions & 31 deletions packages/core-app-api/src/app/AppManager.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
renderWithEffects,
withLogCollector,
} from '@backstage/test-utils';
import { render, screen, waitFor } from '@testing-library/react';
import { screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import React, { PropsWithChildren, ReactNode } from 'react';
import { BrowserRouter, Navigate, Route, Routes } from 'react-router-dom';
Expand Down Expand Up @@ -624,7 +624,7 @@ describe('Integration Test', () => {
expect(capturedEvents).toHaveLength(2);
});

it('should throw some error when the route has duplicate params', () => {
it('should throw some error when the route has duplicate params', async () => {
const app = new AppManager({
apis: [],
defaultApis: [],
Expand All @@ -641,31 +641,43 @@ describe('Integration Test', () => {
},
});

const expectedMessage =
'Parameter :thing is duplicated in path test/:thing/some/:thing';

const Provider = app.getProvider();
const Router = app.getRouter();
const { error: errorLogs } = withLogCollector(() => {
render(
<Provider>
<Router>
<Routes>
<Route path="/test/:thing" element={<ExposedComponent />}>
<Route path="/some/:thing" element={<HiddenComponent />} />
</Route>
</Routes>
</Router>
</Provider>,
);
const { error: errorLogs } = await withLogCollector(async () => {
await expect(() =>
renderWithEffects(
<Provider>
<Router>
<Routes>
<Route path="/test/:thing" element={<ExposedComponent />}>
<Route path="/some/:thing" element={<HiddenComponent />} />
</Route>
</Routes>
</Router>
</Provider>,
),
).rejects.toThrow(expectedMessage);
});

expect(errorLogs).toEqual([
expect.objectContaining({
message: expect.stringContaining(
'Parameter :thing is duplicated in path test/:thing/some/:thing',
),
detail: new Error(expectedMessage),
type: 'unhandled exception',
}),
expect.objectContaining({
detail: new Error(expectedMessage),
type: 'unhandled exception',
}),
expect.stringContaining(
'The above error occurred in the <Provider> component:',
),
]);
});

it('should throw an error when required external plugin routes are not bound', () => {
it('should throw an error when required external plugin routes are not bound', async () => {
const app = new AppManager({
apis: [],
defaultApis: [],
Expand All @@ -676,25 +688,36 @@ describe('Integration Test', () => {
configLoader: async () => [],
});

const expectedMessage =
"External route 'extRouteRef1' of the 'blob' plugin must be bound to a target route. See https://backstage.io/link?bind-routes for details.";

const Provider = app.getProvider();
const Router = app.getRouter();
const { error: errorLogs } = withLogCollector(() => {
render(
<Provider>
<Router>
<Routes>
<Route path="/test/:thing" element={<ExposedComponent />} />
</Routes>
</Router>
</Provider>,
);
const { error: errorLogs } = await withLogCollector(async () => {
await expect(() =>
renderWithEffects(
<Provider>
<Router>
<Routes>
<Route path="/test/:thing" element={<ExposedComponent />} />
</Routes>
</Router>
</Provider>,
),
).rejects.toThrow(expectedMessage);
});
expect(errorLogs).toEqual([
expect.objectContaining({
message: expect.stringMatching(
/^External route 'extRouteRef1' of the 'blob' plugin must be bound to a target route/,
),
detail: new Error(expectedMessage),
type: 'unhandled exception',
}),
expect.objectContaining({
detail: new Error(expectedMessage),
type: 'unhandled exception',
}),
expect.stringContaining(
'The above error occurred in the <Provider> component:',
),
]);
});

Expand Down
40 changes: 24 additions & 16 deletions packages/core-app-api/src/app/AppManager.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,10 @@ export class AppManager implements BackstageApp {

const appContext = new AppContextImpl(this);

// We only validate routes once
let routesHaveBeenValidated = false;
// We only bind and validate routes once
let routeBindings: ReturnType<typeof resolveRouteBindings>;
// Store and keep throwing the same error if we encounter one
let routeValidationError: Error | undefined = undefined;

const Provider = ({ children }: PropsWithChildren<{}>) => {
const needsFeatureFlagRegistrationRef = useRef(true);
Expand All @@ -243,7 +245,7 @@ export class AppManager implements BackstageApp {
[],
);

const { routing, featureFlags, routeBindings } = useMemo(() => {
const { routing, featureFlags } = useMemo(() => {
const usesReactRouterBeta = isReactRouterBeta();
if (usesReactRouterBeta) {
// eslint-disable-next-line no-console
Expand Down Expand Up @@ -275,21 +277,9 @@ DEPRECATION WARNING: React Router Beta is deprecated and support for it will be

// Initialize APIs once all plugins are available
this.getApiHolder();
return {
...result,
routeBindings: resolveRouteBindings(this.bindRoutes),
};
return result;
}, [children]);

if (!routesHaveBeenValidated) {
routesHaveBeenValidated = true;
validateRouteParameters(routing.paths, routing.parents);
validateRouteBindings(
routeBindings,
this.plugins as Iterable<BackstagePlugin>,
);
}

const loadedConfig = useConfigLoader(
this.configLoader,
this.components,
Expand All @@ -307,6 +297,24 @@ DEPRECATION WARNING: React Router Beta is deprecated and support for it will be
return loadedConfig.node;
}

if (!routeBindings) {
routeBindings = resolveRouteBindings(
this.bindRoutes,
loadedConfig.api,
this.plugins,
);

try {
validateRouteParameters(routing.paths, routing.parents);
validateRouteBindings(routeBindings, this.plugins);
} catch (error) {
routeValidationError = error;
throw error;
}
} else if (routeValidationError) {
throw routeValidationError;
}

// We can't register feature flags just after the element traversal, because the
// config API isn't available yet and implementations frequently depend on it.
// Instead we make it happen immediately, to make sure all flags are available
Expand Down
Loading
Loading