paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor: move fix to the Link component

Browse Source 
Signed-off-by: Camila Belo <camilaibs@gmail.com>
This commit is contained in:
Camila Belo
2022-07-01 16:20:39 +02:00
parent d1b99b5042
commit 7f5e79961d
14 changed files with 138 additions and 136 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/ninety-coats-learn.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/core-components': patch
---
Fix the relative `sub-paths` by concatenating the app's base URL with them.
.changeset/wicked-feet-poke.md
-7
View File
@@ -1,7 +0,0 @@
---
'@backstage/plugin-catalog': patch
'@backstage/plugin-search-react': patch
'@backstage/plugin-techdocs': patch
---
Fix the relative search result locations by concatenating the app's base URL into it.
packages/core-app-api/api-report.md
+3
View File
@@ -372,6 +372,9 @@ export type FlatRoutesProps = {
children: ReactNode;
};
// @public
export function getBasePath(configApi: Config): string;
// @public
export class GithubAuth {
// (undocumented)
packages/core-app-api/src/app/AppManager.tsx
+2 -1
View File
@@ -95,8 +95,9 @@ const InternalAppContext = createContext<{
* Get the app base path from the configured app baseUrl.
*
* The returned path does not have a trailing slash.
* @public
*/
function getBasePath(configApi: Config) {
export function getBasePath(configApi: Config) {
let { pathname } = new URL(
configApi.getOptionalString('app.baseUrl') ?? '/',
'http://dummy.dev', // baseUrl can be specified as just a path
packages/core-app-api/src/app/index.ts
+1
View File
@@ -16,4 +16,5 @@
export { createSpecializedApp } from './createSpecializedApp';
export { defaultConfigLoader } from './defaultConfigLoader';
export { getBasePath } from './AppManager';
export * from './types';
packages/core-components/package.json
+1
View File
@@ -34,6 +34,7 @@
},
"dependencies": {
"@backstage/config": "^1.0.1",
"@backstage/core-app-api": "^1.0.4-next.0",
"@backstage/core-plugin-api": "^1.0.3",
"@backstage/errors": "^1.1.0-next.0",
"@backstage/theme": "^0.2.16-next.0",
packages/core-components/src/components/Link/Link.test.tsx
+97 -2
View File
@@ -21,9 +21,11 @@ import {
TestApiProvider,
wrapInTestApp,
} from '@backstage/test-utils';
import { analyticsApiRef } from '@backstage/core-plugin-api';
import { isExternalUri, Link } from './Link';
import { analyticsApiRef, configApiRef } from '@backstage/core-plugin-api';
import { isExternalUri, Link, useResolvedPath } from './Link';
import { Route, Routes } from 'react-router';
import { renderHook, WrapperComponent } from '@testing-library/react-hooks';
import { ConfigReader } from '@backstage/config';
describe('<Link />', () => {
it('navigates using react-router', async () => {
@@ -103,6 +105,58 @@ describe('<Link />', () => {
});
});
describe('resolves a sub-path correctly', () => {
it('when it starts with base path', async () => {
const testString = 'This is test string';
const linkText = 'Navigate!';
const configApi = new ConfigReader({
app: { baseUrl: 'http://localhost:3000/example' },
});
const { getByText } = render(
wrapInTestApp(
<TestApiProvider apis={[[configApiRef, configApi]]}>
<Routes>
<Link to="/example/test">{linkText}</Link>
<Route path="/example/test" element={<p>{testString}</p>} />
</Routes>
</TestApiProvider>,
),
);
expect(() => getByText(testString)).toThrow();
fireEvent.click(getByText(linkText));
await waitFor(() => {
expect(getByText(testString)).toBeInTheDocument();
});
});
it('when it does not start with base path', async () => {
const testString = 'This is test string';
const linkText = 'Navigate!';
const configApi = new ConfigReader({
app: { baseUrl: 'http://localhost:3000/example' },
});
const { getByText } = render(
wrapInTestApp(
<TestApiProvider apis={[[configApiRef, configApi]]}>
<Routes>
<Link to="/test">{linkText}</Link>
<Route path="/example/test" element={<p>{testString}</p>} />
</Routes>
</TestApiProvider>,
),
);
expect(() => getByText(testString)).toThrow();
fireEvent.click(getByText(linkText));
await waitFor(() => {
expect(getByText(testString)).toBeInTheDocument();
});
});
});
describe('isExternalUri', () => {
it.each([
[true, 'http://'],
@@ -128,4 +182,45 @@ describe('<Link />', () => {
expect(isExternalUri(uri)).toBe(expected);
});
});
describe('useResolvedPath', () => {
const wrapper: WrapperComponent<{}> = ({ children }) => {
const configApi = new ConfigReader({
app: { baseUrl: 'http://localhost:3000/example' },
});
return (
<TestApiProvider apis={[[configApiRef, configApi]]}>
{children}
</TestApiProvider>
);
};
describe('concatenate base path', () => {
it('when uri is internal and does not start with base path', () => {
const path = '/catalog/default/component/artist-lookup';
const { result } = renderHook(() => useResolvedPath(path), {
wrapper,
});
expect(result.current).toBe('/example'.concat(path));
});
});
describe('does not concatenate base path', () => {
it('when uri is external', () => {
const path = 'https://stackoverflow.com/questions/1/example';
const { result } = renderHook(() => useResolvedPath(path), {
wrapper,
});
expect(result.current).toBe(path);
});
it('when uri already starts with base path', () => {
const path = '/example/catalog/default/component/artist-lookup';
const { result } = renderHook(() => useResolvedPath(path), {
wrapper,
});
expect(result.current).toBe(path);
});
});
});
});
packages/core-components/src/components/Link/Link.tsx
+21 -5
View File
@@ -14,7 +14,7 @@
* limitations under the License.
*/
import { useAnalytics } from '@backstage/core-plugin-api';
import { configApiRef, useAnalytics, useApi } from '@backstage/core-plugin-api';
import classnames from 'classnames';
import MaterialLink, {
LinkProps as MaterialLinkProps,
@@ -25,6 +25,7 @@ import {
Link as RouterLink,
LinkProps as RouterLinkProps,
} from 'react-router-dom';
import { getBasePath } from '@backstage/core-app-api';
const useStyles = makeStyles(
{
@@ -52,6 +53,20 @@ export type LinkProps = MaterialLinkProps &
noTrack?: boolean;
};
export const useResolvedPath = (uri: LinkProps['to']) => {
const configApi = useApi(configApiRef);
const basePath = getBasePath(configApi);
let resolvedPath = String(uri);
const external = isExternalUri(resolvedPath);
if (!external && !resolvedPath.startsWith(basePath)) {
resolvedPath = basePath.concat(resolvedPath);
}
return resolvedPath;
};
/**
* Given a react node, try to retrieve its text content.
*/
@@ -84,7 +99,7 @@ export const Link = React.forwardRef<any, LinkProps>(
({ onClick, noTrack, ...props }, ref) => {
const classes = useStyles();
const analytics = useAnalytics();
const to = String(props.to);
const to = useResolvedPath(props.to);
const linkText = getNodeText(props.children) || to;
const external = isExternalUri(to);
const newWindow = external && !!/^https?:/.exec(to);
@@ -99,11 +114,11 @@ export const Link = React.forwardRef<any, LinkProps>(
return external ? (
// External links
<MaterialLink
{...(newWindow ? { target: '_blank', rel: 'noopener' } : {})}
{...props}
ref={ref}
href={to}
onClick={handleClick}
{...(newWindow ? { target: '_blank', rel: 'noopener' } : {})}
{...props}
className={classnames(classes.externalLink, props.className)}
>
{props.children}
@@ -112,10 +127,11 @@ export const Link = React.forwardRef<any, LinkProps>(
) : (
// Interact with React Router for internal links
<MaterialLink
{...props}
ref={ref}
component={RouterLink}
onClick={handleClick}
{...props}
to={to}
/>
);
},
plugins/catalog/src/components/CatalogSearchResultListItem/CatalogSearchResultListItem.tsx
+3 -9
View File
@@ -30,10 +30,7 @@ import {
IndexableDocument,
ResultHighlight,
} from '@backstage/plugin-search-common';
import {
HighlightedSearchResultText,
useSearchResultLocation,
} from '@backstage/plugin-search-react';
import { HighlightedSearchResultText } from '@backstage/plugin-search-react';
const useStyles = makeStyles({
flexContainer: {
@@ -66,18 +63,15 @@ export function CatalogSearchResultListItem(
const classes = useStyles();
const analytics = useAnalytics();
const to = useSearchResultLocation(result);
const handleClick = () => {
analytics.captureEvent('discover', result.title, {
attributes: { to },
attributes: { to: result.location },
value: props.rank,
});
};
return (
<Link noTrack to={to} onClick={handleClick}>
<Link noTrack to={result.location} onClick={handleClick}>
<ListItem alignItems="flex-start">
{props.icon && <ListItemIcon>{props.icon}</ListItemIcon>}
<div className={classes.flexContainer}>
plugins/search-react/src/components/DefaultResultListItem/DefaultResultListItem.tsx
+2 -6
View File
@@ -29,7 +29,6 @@ import {
Divider,
} from '@material-ui/core';
import { Link } from '@backstage/core-components';
import { useSearchResultLocation } from './useSearchResultLocation';
/**
* Props for {@link DefaultResultListItem}
@@ -59,18 +58,15 @@ export const DefaultResultListItemComponent = ({
lineClamp = 5,
}: DefaultResultListItemProps) => {
const analytics = useAnalytics();
const to = useSearchResultLocation(result);
const handleClick = () => {
analytics.captureEvent('discover', result.title, {
attributes: { to },
attributes: { to: result.location },
value: rank,
});
};
return (
<Link noTrack to={to} onClick={handleClick}>
<Link noTrack to={result.location} onClick={handleClick}>
<ListItem alignItems="center">
{icon && <ListItemIcon>{icon}</ListItemIcon>}
<ListItemText
plugins/search-react/src/components/DefaultResultListItem/index.ts
-1
View File
@@ -16,4 +16,3 @@
export { DefaultResultListItem } from './DefaultResultListItem';
export type { DefaultResultListItemProps } from './DefaultResultListItem';
export { useSearchResultLocation } from './useSearchResultLocation';
plugins/search-react/src/components/DefaultResultListItem/useSearchResultLocation.test.tsx
-60
View File
@@ -1,60 +0,0 @@
/*
* 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 { ConfigReader } from '@backstage/core-app-api';
import { configApiRef } from '@backstage/core-plugin-api';
import { TestApiProvider } from '@backstage/test-utils';
import { renderHook, WrapperComponent } from '@testing-library/react-hooks';
import { useSearchResultLocation } from './useSearchResultLocation';
describe('useSearchResultLocation', () => {
const baseUrl = 'http://localhost:3000/example';
const configApiMock = new ConfigReader({ app: { baseUrl } });
const wrapper: WrapperComponent<{}> = ({ children }) => (
<TestApiProvider apis={[[configApiRef, configApiMock]]}>
{children}
</TestApiProvider>
);
it('should concatenate base url into relative urls', () => {
const document = {
title: 'artist-lookup',
text: 'Artist Lookup',
location: '/catalog/default/component/artist-lookup',
};
const { result } = renderHook(() => useSearchResultLocation(document), {
wrapper,
});
expect(result.current).toBe(baseUrl.concat(document.location));
});
it('should not concatenate base url into absolute urls', () => {
const document = {
title: 'What is Backstage?',
text: 'An open platform for building developer portals ',
location: 'https://stackoverflow.com/questions/1/what-is-backstage',
};
const { result } = renderHook(() => useSearchResultLocation(document), {
wrapper,
});
expect(result.current).toBe(document.location);
});
});
plugins/search-react/src/components/DefaultResultListItem/useSearchResultLocation.tsx
-36
View File
@@ -1,36 +0,0 @@
/*
* 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 { configApiRef, useApi } from '@backstage/core-plugin-api';
import { SearchDocument } from '@backstage/plugin-search-common';
/**
* Builds a search result url.
* @param document - A search result item.
* @returns Add app base url as location prefix when location is relative.
*/
export const useSearchResultLocation = (document: SearchDocument) => {
const configApi = useApi(configApiRef);
let location = document.location;
const isRelative = !location.match(/^([a-z]*:)?\/\//i);
if (isRelative) {
location = configApi.getString('app.baseUrl').concat(location);
}
return location;
};
plugins/techdocs/src/search/components/TechDocsSearchResultListItem.tsx
+3 -9
View File
@@ -25,10 +25,7 @@ import {
import { Link } from '@backstage/core-components';
import { useAnalytics } from '@backstage/core-plugin-api';
import { ResultHighlight } from '@backstage/plugin-search-common';
import {
HighlightedSearchResultText,
useSearchResultLocation,
} from '@backstage/plugin-search-react';
import { HighlightedSearchResultText } from '@backstage/plugin-search-react';
const useStyles = makeStyles({
flexContainer: {
@@ -77,12 +74,9 @@ export const TechDocsSearchResultListItem = (
const classes = useStyles();
const analytics = useAnalytics();
const to = useSearchResultLocation(result);
const handleClick = () => {
analytics.captureEvent('discover', result.title, {
attributes: { to },
attributes: { to: result.location },
value: rank,
});
};
@@ -157,7 +151,7 @@ export const TechDocsSearchResultListItem = (
const LinkWrapper = ({ children }: PropsWithChildren<{}>) =>
asLink ? (
<Link noTrack to={to} onClick={handleClick}>
<Link noTrack to={result.location} onClick={handleClick}>
{children}
</Link>
) : (