From 6abf24efd7d93537ace817fcf101fe3d18bb2e85 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 29 Nov 2022 12:49:51 -0500 Subject: [PATCH 01/42] Update config to handle relative frontend/backend during webpack build. Signed-off-by: Aramis Sennyey --- packages/app-defaults/src/defaults/apis.ts | 17 +++++-- .../cli/src/commands/build/buildFrontend.ts | 5 +- packages/cli/src/commands/index.ts | 8 +++ packages/cli/src/commands/repo/build.ts | 4 ++ packages/cli/src/lib/bundler/config.ts | 6 ++- packages/cli/src/lib/bundler/server.ts | 3 ++ packages/cli/src/lib/config.ts | 5 ++ packages/config-loader/src/lib/cli.ts | 49 +++++++++++++++++++ packages/config-loader/src/loader.ts | 12 ++++- packages/core-app-api/src/app/AppManager.tsx | 5 +- packages/core-app-api/src/app/types.ts | 4 ++ .../core-app-api/src/routing/RouteResolver.ts | 16 +++--- 12 files changed, 113 insertions(+), 21 deletions(-) create mode 100644 packages/config-loader/src/lib/cli.ts diff --git a/packages/app-defaults/src/defaults/apis.ts b/packages/app-defaults/src/defaults/apis.ts index 3f5cfc1c58..c53ce63ada 100644 --- a/packages/app-defaults/src/defaults/apis.ts +++ b/packages/app-defaults/src/defaults/apis.ts @@ -64,10 +64,19 @@ export const apis = [ createApiFactory({ api: discoveryApiRef, deps: { configApi: configApiRef }, - factory: ({ configApi }) => - UrlPatternDiscovery.compile( - `${configApi.getString('backend.baseUrl')}/api/{{ pluginId }}`, - ), + factory: ({ configApi }) => { + let baseUrl; + try { + // Try parsing the url relative to the current document origin, if it fails the URL is misformed. + baseUrl = new URL( + configApi.getString('backend.baseUrl'), + document.location.origin, + ).href.replace(/\/$/, ''); + } catch (err) { + baseUrl = configApi.getString('backend.baseUrl'); + } + return UrlPatternDiscovery.compile(`${baseUrl}/api/{{ pluginId }}`); + }, }), createApiFactory({ api: alertApiRef, diff --git a/packages/cli/src/commands/build/buildFrontend.ts b/packages/cli/src/commands/build/buildFrontend.ts index 6d7ca2e6a9..7f1687ca03 100644 --- a/packages/cli/src/commands/build/buildFrontend.ts +++ b/packages/cli/src/commands/build/buildFrontend.ts @@ -19,15 +19,17 @@ import { resolve as resolvePath } from 'path'; import { buildBundle } from '../../lib/bundler'; import { getEnvironmentParallelism } from '../../lib/parallel'; import { loadCliConfig } from '../../lib/config'; +import { CliConfigOptions } from '@backstage/config-loader/src/lib/cli'; interface BuildAppOptions { targetDir: string; writeStats: boolean; + cliOptions?: CliConfigOptions; configPaths: string[]; } export async function buildFrontend(options: BuildAppOptions) { - const { targetDir, writeStats, configPaths } = options; + const { targetDir, writeStats, configPaths, cliOptions } = options; const { name } = await fs.readJson(resolvePath(targetDir, 'package.json')); await buildBundle({ targetDir, @@ -37,6 +39,7 @@ export async function buildFrontend(options: BuildAppOptions) { ...(await loadCliConfig({ args: configPaths, fromPackage: name, + cliOptions, })), }); } diff --git a/packages/cli/src/commands/index.ts b/packages/cli/src/commands/index.ts index 82cb60e62f..33291cb70c 100644 --- a/packages/cli/src/commands/index.ts +++ b/packages/cli/src/commands/index.ts @@ -39,6 +39,14 @@ export function registerRepoCommand(program: Command) { '--all', 'Build all packages, including bundled app and backend packages.', ) + .option( + '--public-path ', + 'Public path for hosting the website on, can be relative.', + ) + .option( + '--backend-url ', + 'Backend url, expects just the origin or sub-route. Do not include /api. Can be relative.', + ) .option( '--since ', 'Only build packages and their dev dependents that changed since the specified ref', diff --git a/packages/cli/src/commands/repo/build.ts b/packages/cli/src/commands/repo/build.ts index 21b1580cb9..86d2e781a6 100644 --- a/packages/cli/src/commands/repo/build.ts +++ b/packages/cli/src/commands/repo/build.ts @@ -153,6 +153,10 @@ export async function command(opts: OptionValues, cmd: Command): Promise { return; } await buildFrontend({ + cliOptions: { + publicPath: opts.publicPath, + backendUrl: opts.backendUrl, + }, targetDir: pkg.dir, configPaths: (buildOptions.config as string[]) ?? [], writeStats: Boolean(buildOptions.stats), diff --git a/packages/cli/src/lib/bundler/config.ts b/packages/cli/src/lib/bundler/config.ts index 1974153f02..bc0ff2db3b 100644 --- a/packages/cli/src/lib/bundler/config.ts +++ b/packages/cli/src/lib/bundler/config.ts @@ -36,10 +36,12 @@ import { runPlain } from '../run'; import ESLintPlugin from 'eslint-webpack-plugin'; import pickBy from 'lodash/pickBy'; +const DUMMY_URL = 'http://dummyurl.org'; + export function resolveBaseUrl(config: Config): URL { const baseUrl = config.getString('app.baseUrl'); try { - return new URL(baseUrl); + return new URL(baseUrl, DUMMY_URL); } catch (error) { throw new Error(`Invalid app.baseUrl, ${error}`); } @@ -88,7 +90,7 @@ export async function createConfig( const externalPkgs = packages.filter(p => !isChildPath(paths.root, p.dir)); const baseUrl = frontendConfig.getString('app.baseUrl'); - const validBaseUrl = new URL(baseUrl); + const validBaseUrl = new URL(baseUrl, DUMMY_URL); const publicPath = validBaseUrl.pathname.replace(/\/$/, ''); if (checksEnabled) { plugins.push( diff --git a/packages/cli/src/lib/bundler/server.ts b/packages/cli/src/lib/bundler/server.ts index fb11316fa6..1e76fbc662 100644 --- a/packages/cli/src/lib/bundler/server.ts +++ b/packages/cli/src/lib/bundler/server.ts @@ -60,6 +60,9 @@ export async function serveBundle(options: ServeOptions) { // Paths with dots should still use the history fallback. // See https://github.com/facebookincubator/create-react-app/issues/387. disableDotRule: true, + + // The index needs to be rewritten relative to the new public path, including subroutes. + index: config.output?.publicPath ?? '/index.html', }, https: url.protocol === 'https:' diff --git a/packages/cli/src/lib/config.ts b/packages/cli/src/lib/config.ts index a76571c3f6..e28f2d7f6f 100644 --- a/packages/cli/src/lib/config.ts +++ b/packages/cli/src/lib/config.ts @@ -32,6 +32,10 @@ type Options = { withFilteredKeys?: boolean; withDeprecatedKeys?: boolean; fullVisibility?: boolean; + cliOptions?: { + publicPath?: string; + backendUrl?: string; + }; }; export async function loadCliConfig(options: Options) { @@ -76,6 +80,7 @@ export async function loadCliConfig(options: Options) { experimentalEnvFunc: options.mockEnv ? async name => process.env[name] || 'x' : undefined, + cliOptions: options.cliOptions, configRoot: paths.targetRoot, configTargets: configTargets, }); diff --git a/packages/config-loader/src/lib/cli.ts b/packages/config-loader/src/lib/cli.ts new file mode 100644 index 0000000000..9d935e2826 --- /dev/null +++ b/packages/config-loader/src/lib/cli.ts @@ -0,0 +1,49 @@ +/* + * 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 { AppConfig } from '@backstage/config'; +import { JsonObject } from '@backstage/types'; + +export type CliConfigOptions = { + publicPath?: string; + backendUrl?: string; +}; + +/** + * Read specific parameters from the CLI and add them to the build config. + * @param opts CLI passed parameters. + * @returns Array of config, empty if there is no relevant passed in cli options. + * + * @public + */ +export function readCliConfig(opts?: CliConfigOptions): AppConfig[] { + if (!opts || Object.keys(opts).length === 0) return []; + const data: JsonObject = {}; + + if (opts.publicPath) { + data.app = { + baseUrl: opts.publicPath, + }; + } + + if (opts.backendUrl) { + data.backend = { + baseUrl: opts.backendUrl, + }; + } + + return [{ data, context: 'cli' }]; +} diff --git a/packages/config-loader/src/loader.ts b/packages/config-loader/src/loader.ts index f5ce0eb80c..44bd4f94e8 100644 --- a/packages/config-loader/src/loader.ts +++ b/packages/config-loader/src/loader.ts @@ -28,6 +28,7 @@ import { readEnvConfig, } from './lib'; import fetch from 'node-fetch'; +import { CliConfigOptions, readCliConfig } from './lib/cli'; /** @public */ export type ConfigTarget = { path: string } | { url: string }; @@ -81,6 +82,11 @@ export type LoadConfigOptions = { * An optional configuration that enables watching of config files. */ watch?: LoadConfigOptionsWatch; + + /** + * New options from the CLI that affect the build config. + */ + cliOptions?: CliConfigOptions; }; /** @@ -230,6 +236,8 @@ export async function loadConfig( } } + const cliConfigs = readCliConfig(options.cliOptions); + const envConfigs = readEnvConfig(process.env); const watchConfigFile = (watchProp: LoadConfigOptionsWatch) => { @@ -318,7 +326,7 @@ export async function loadConfig( return { appConfigs: remote - ? [...remoteConfigs, ...fileConfigs, ...envConfigs] - : [...fileConfigs, ...envConfigs], + ? [...remoteConfigs, ...fileConfigs, ...envConfigs, ...cliConfigs] + : [...fileConfigs, ...envConfigs, ...cliConfigs], }; } diff --git a/packages/core-app-api/src/app/AppManager.tsx b/packages/core-app-api/src/app/AppManager.tsx index 0ca09c0a0c..9b3dc841d0 100644 --- a/packages/core-app-api/src/app/AppManager.tsx +++ b/packages/core-app-api/src/app/AppManager.tsx @@ -116,7 +116,7 @@ function getBasePath(configApi: Config) { function readBasePath(configApi: ConfigApi) { let { pathname } = new URL( configApi.getOptionalString('app.baseUrl') ?? '/', - 'http://dummy.dev', // baseUrl can be specified as just a path + document.location.origin, // baseUrl can be specified as just a path ); pathname = pathname.replace(/\/*$/, ''); return pathname; @@ -332,7 +332,6 @@ export class AppManager implements BackstageApp { routeParents={routing.parents} routeObjects={routing.objects} routeBindings={routeBindings} - basePath={getBasePath(loadedConfig.api)} > + diff --git a/packages/core-app-api/src/app/types.ts b/packages/core-app-api/src/app/types.ts index 70022534a1..b548533df3 100644 --- a/packages/core-app-api/src/app/types.ts +++ b/packages/core-app-api/src/app/types.ts @@ -60,6 +60,10 @@ export type ErrorBoundaryFallbackProps = { resetError: () => void; }; +export type RouterProps = { + basename: string; +}; + /** * A set of replaceable core components that are part of every Backstage app. * diff --git a/packages/core-app-api/src/routing/RouteResolver.ts b/packages/core-app-api/src/routing/RouteResolver.ts index f17c549bc6..1b23aa30f1 100644 --- a/packages/core-app-api/src/routing/RouteResolver.ts +++ b/packages/core-app-api/src/routing/RouteResolver.ts @@ -214,15 +214,13 @@ export class RouteResolver { // Next we figure out the base path, which is the combination of the common parent path // between our current location and our target location, as well as the additional path // that is the difference between the parent path and the base of our target location. - const basePath = - this.appBasePath + - resolveBasePath( - targetRef, - relativeSourceLocation, - this.routePaths, - this.routeParents, - this.routeObjects, - ); + const basePath = resolveBasePath( + targetRef, + relativeSourceLocation, + this.routePaths, + this.routeParents, + this.routeObjects, + ); const routeFunc: RouteFunc = (...[params]) => { return joinPaths(basePath, generatePath(targetPath, params)); From 62ad114c9ee4bc5a18dc893b8d2017f9bf2c6bb0 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 29 Nov 2022 12:52:10 -0500 Subject: [PATCH 02/42] Update vocabulary so that lint:docs works. Signed-off-by: Aramis Sennyey --- .github/vale/Vocab/Backstage/accept.txt | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/.github/vale/Vocab/Backstage/accept.txt b/.github/vale/Vocab/Backstage/accept.txt index bba59094e3..90af39794d 100644 --- a/.github/vale/Vocab/Backstage/accept.txt +++ b/.github/vale/Vocab/Backstage/accept.txt @@ -19,12 +19,14 @@ automations autoscaling Autoscaling autoselect +authProvider Avro backend's backported backporting Bigtable Billett +birth_date bitbucket Bitrise Blackbox @@ -46,6 +48,7 @@ cli cloudbuild Cloudflare Cloudformation +clusterName cncf Cobertura codeblocks @@ -66,6 +69,9 @@ configs const cookiecutter Corti +createApp +createPlugin +createTheme cron cronjobs crontab @@ -109,6 +115,7 @@ etag Expedia facto failover +family_name Fargate Figma firehydrant @@ -135,6 +142,7 @@ graphviz Hackathons haproxy hardcoded +headerLinks Helidon Heroku hoc @@ -255,12 +263,15 @@ Platformize Podman postgres postpack +postTransfomers pre prebaked preconfigured prepack Preprarer productional +projectId +projectKey Protobuf proxying Proxying @@ -334,6 +345,7 @@ subheader subheaders subkey subroutes +subscription_id subtree superfences Superfences @@ -382,7 +394,10 @@ unregistration untracked upsert upvote +URI +URI's URIs +url URLs utils Valentina From c7ae6565a863e3731b8f4122ca0f1c75da69535c Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 29 Nov 2022 12:53:18 -0500 Subject: [PATCH 03/42] Fix test cases. Signed-off-by: Aramis Sennyey --- .../src/components/Link/Link.test.tsx | 47 +------------------ .../src/components/Link/Link.tsx | 2 +- .../src/components/Table/Table.tsx | 2 +- 3 files changed, 4 insertions(+), 47 deletions(-) diff --git a/packages/core-components/src/components/Link/Link.test.tsx b/packages/core-components/src/components/Link/Link.test.tsx index d65351d840..7be8753b34 100644 --- a/packages/core-components/src/components/Link/Link.test.tsx +++ b/packages/core-components/src/components/Link/Link.test.tsx @@ -21,11 +21,9 @@ import { TestApiProvider, wrapInTestApp, } from '@backstage/test-utils'; -import { analyticsApiRef, configApiRef } from '@backstage/core-plugin-api'; -import { isExternalUri, Link, useResolvedPath } from './Link'; +import { analyticsApiRef } from '@backstage/core-plugin-api'; +import { isExternalUri, Link } from './Link'; import { Route, Routes } from 'react-router'; -import { renderHook, WrapperComponent } from '@testing-library/react-hooks'; -import { ConfigReader } from '@backstage/config'; describe('', () => { it('navigates using react-router', async () => { @@ -132,45 +130,4 @@ describe('', () => { expect(isExternalUri(uri)).toBe(expected); }); }); - - describe('useResolvedPath', () => { - const wrapper: WrapperComponent<{}> = ({ children }) => { - const configApi = new ConfigReader({ - app: { baseUrl: 'http://localhost:3000/example' }, - }); - return ( - - {children} - - ); - }; - - 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); - }); - }); - }); }); diff --git a/packages/core-components/src/components/Link/Link.tsx b/packages/core-components/src/components/Link/Link.tsx index 66b7453abd..239f6382c4 100644 --- a/packages/core-components/src/components/Link/Link.tsx +++ b/packages/core-components/src/components/Link/Link.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import { configApiRef, useAnalytics, useApi } from '@backstage/core-plugin-api'; +import { useAnalytics } from '@backstage/core-plugin-api'; import classnames from 'classnames'; // eslint-disable-next-line no-restricted-imports import MaterialLink, { diff --git a/packages/core-components/src/components/Table/Table.tsx b/packages/core-components/src/components/Table/Table.tsx index 81471324a0..1d16dc9b83 100644 --- a/packages/core-components/src/components/Table/Table.tsx +++ b/packages/core-components/src/components/Table/Table.tsx @@ -373,7 +373,7 @@ export function Table(props: TableProps) { const newData = (data as any[]).filter( el => !!Object.entries(selectedFilters) - .filter(([, value]) => !!value.length) + .filter(([, value]: [any, any]) => !!value.length) .every(([key, filterValue]) => { const fieldValue = extractValueByField( el, From 55fedf7af397277c286e8d8bd3d29fc69b257dc9 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Wed, 5 Oct 2022 14:49:43 -0400 Subject: [PATCH 04/42] Add changeset. Signed-off-by: Aramis Sennyey --- .changeset/quick-islands-tease.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .changeset/quick-islands-tease.md diff --git a/.changeset/quick-islands-tease.md b/.changeset/quick-islands-tease.md new file mode 100644 index 0000000000..9498e67539 --- /dev/null +++ b/.changeset/quick-islands-tease.md @@ -0,0 +1,9 @@ +--- +'@backstage/app-defaults': minor +'@backstage/cli': minor +'@backstage/config-loader': minor +'@backstage/core-app-api': minor +'@backstage/core-components': minor +--- + +Added support for adding CLI options for setting frontend public path (for example, /backstage) as well as the backend url that the front end uses to communicate with the backend. Both of these can now be specified as relative. From f2641eabb57f23013e84b7c52dd2439a8b61ed20 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 29 Nov 2022 12:54:18 -0500 Subject: [PATCH 05/42] reset basePath Signed-off-by: Aramis Sennyey --- packages/core-app-api/src/app/AppManager.tsx | 5 +- packages/core-app-api/src/app/types.ts | 4 - .../core-app-api/src/routing/RouteResolver.ts | 16 +-- .../src/components/Link/Link.test.tsx | 99 ++++++++++++++++++- .../src/components/Link/Link.tsx | 2 +- 5 files changed, 110 insertions(+), 16 deletions(-) diff --git a/packages/core-app-api/src/app/AppManager.tsx b/packages/core-app-api/src/app/AppManager.tsx index 9b3dc841d0..571eaf72e0 100644 --- a/packages/core-app-api/src/app/AppManager.tsx +++ b/packages/core-app-api/src/app/AppManager.tsx @@ -332,6 +332,7 @@ export class AppManager implements BackstageApp { routeParents={routing.parents} routeObjects={routing.objects} routeBindings={routeBindings} + basePath={getBasePath(loadedConfig.api)} > + {children} @@ -437,7 +438,7 @@ export class AppManager implements BackstageApp { } return ( - + <>{children} diff --git a/packages/core-app-api/src/app/types.ts b/packages/core-app-api/src/app/types.ts index b548533df3..70022534a1 100644 --- a/packages/core-app-api/src/app/types.ts +++ b/packages/core-app-api/src/app/types.ts @@ -60,10 +60,6 @@ export type ErrorBoundaryFallbackProps = { resetError: () => void; }; -export type RouterProps = { - basename: string; -}; - /** * A set of replaceable core components that are part of every Backstage app. * diff --git a/packages/core-app-api/src/routing/RouteResolver.ts b/packages/core-app-api/src/routing/RouteResolver.ts index 1b23aa30f1..f17c549bc6 100644 --- a/packages/core-app-api/src/routing/RouteResolver.ts +++ b/packages/core-app-api/src/routing/RouteResolver.ts @@ -214,13 +214,15 @@ export class RouteResolver { // Next we figure out the base path, which is the combination of the common parent path // between our current location and our target location, as well as the additional path // that is the difference between the parent path and the base of our target location. - const basePath = resolveBasePath( - targetRef, - relativeSourceLocation, - this.routePaths, - this.routeParents, - this.routeObjects, - ); + const basePath = + this.appBasePath + + resolveBasePath( + targetRef, + relativeSourceLocation, + this.routePaths, + this.routeParents, + this.routeObjects, + ); const routeFunc: RouteFunc = (...[params]) => { return joinPaths(basePath, generatePath(targetPath, params)); diff --git a/packages/core-components/src/components/Link/Link.test.tsx b/packages/core-components/src/components/Link/Link.test.tsx index 7be8753b34..c18812d897 100644 --- a/packages/core-components/src/components/Link/Link.test.tsx +++ b/packages/core-components/src/components/Link/Link.test.tsx @@ -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 { ConfigReader } from '@backstage/config'; +import { renderHook, WrapperComponent } from '@testing-library/react-hooks'; describe('', () => { it('navigates using react-router', async () => { @@ -105,6 +107,58 @@ describe('', () => { }); }); + describe('resolves a sub-path correctly', () => { + it('when it starts with base path', async () => { + const testString = 'This is test string'; + const linkText = 'Navigate!'; + const configApi = new ConfigReader({ + app: { baseUrl: 'http://localhost:3000/example' }, + }); + + const { getByText } = render( + wrapInTestApp( + + {linkText} + + {testString}

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

} /> + + , + ), + ); + + expect(() => getByText(testString)).toThrow(); + fireEvent.click(getByText(linkText)); + await waitFor(() => { + expect(getByText(testString)).toBeInTheDocument(); + }); + }); + }); + describe('isExternalUri', () => { it.each([ [true, 'http://'], @@ -130,4 +184,45 @@ describe('', () => { expect(isExternalUri(uri)).toBe(expected); }); }); + + describe('useResolvedPath', () => { + const wrapper: WrapperComponent<{}> = ({ children }) => { + const configApi = new ConfigReader({ + app: { baseUrl: 'http://localhost:3000/example' }, + }); + return ( + + {children} + + ); + }; + + 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); + }); + }); + }); }); diff --git a/packages/core-components/src/components/Link/Link.tsx b/packages/core-components/src/components/Link/Link.tsx index 239f6382c4..66b7453abd 100644 --- a/packages/core-components/src/components/Link/Link.tsx +++ b/packages/core-components/src/components/Link/Link.tsx @@ -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'; // eslint-disable-next-line no-restricted-imports import MaterialLink, { From 310c6a5827d36d25da9a8af08e18be2e54a3fadd Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 29 Nov 2022 12:54:58 -0500 Subject: [PATCH 06/42] Remove Vale fixes Signed-off-by: Aramis Sennyey --- .github/vale/Vocab/Backstage/accept.txt | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/.github/vale/Vocab/Backstage/accept.txt b/.github/vale/Vocab/Backstage/accept.txt index 90af39794d..3278176bee 100644 --- a/.github/vale/Vocab/Backstage/accept.txt +++ b/.github/vale/Vocab/Backstage/accept.txt @@ -1,6 +1,5 @@ abc accessors -ACLs addon addons ADRs @@ -19,14 +18,12 @@ automations autoscaling Autoscaling autoselect -authProvider Avro backend's backported backporting Bigtable Billett -birth_date bitbucket Bitrise Blackbox @@ -48,7 +45,6 @@ cli cloudbuild Cloudflare Cloudformation -clusterName cncf Cobertura codeblocks @@ -69,9 +65,6 @@ configs const cookiecutter Corti -createApp -createPlugin -createTheme cron cronjobs crontab @@ -115,7 +108,6 @@ etag Expedia facto failover -family_name Fargate Figma firehydrant @@ -142,7 +134,6 @@ graphviz Hackathons haproxy hardcoded -headerLinks Helidon Heroku hoc @@ -166,6 +157,7 @@ JaCoCo JavaScript jenkins Jira +JWTs jq js json @@ -263,15 +255,12 @@ Platformize Podman postgres postpack -postTransfomers pre prebaked preconfigured prepack Preprarer productional -projectId -projectKey Protobuf proxying Proxying @@ -345,7 +334,6 @@ subheader subheaders subkey subroutes -subscription_id subtree superfences Superfences @@ -364,7 +352,6 @@ templater Templater templaters Templaters -TFRecord theia thumbsup todo @@ -382,7 +369,6 @@ transpiled transpiler transpilers truthy -TSDoc typeahead ui unbreak @@ -394,10 +380,7 @@ unregistration untracked upsert upvote -URI -URI's URIs -url URLs utils Valentina From fd7c409fe2d61688e7bfb83a6690de01f69ced63 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 11 Oct 2022 17:07:12 -0400 Subject: [PATCH 07/42] Revert tsx fix. Signed-off-by: Aramis Sennyey --- packages/core-components/src/components/Table/Table.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core-components/src/components/Table/Table.tsx b/packages/core-components/src/components/Table/Table.tsx index 1d16dc9b83..81471324a0 100644 --- a/packages/core-components/src/components/Table/Table.tsx +++ b/packages/core-components/src/components/Table/Table.tsx @@ -373,7 +373,7 @@ export function Table(props: TableProps) { const newData = (data as any[]).filter( el => !!Object.entries(selectedFilters) - .filter(([, value]: [any, any]) => !!value.length) + .filter(([, value]) => !!value.length) .every(([key, filterValue]) => { const fieldValue = extractValueByField( el, From a271aa4c6cffde05387f9378e67024651ad58970 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 11 Oct 2022 17:48:14 -0400 Subject: [PATCH 08/42] Revert ordering Signed-off-by: Aramis Sennyey --- packages/core-components/src/components/Link/Link.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core-components/src/components/Link/Link.test.tsx b/packages/core-components/src/components/Link/Link.test.tsx index c18812d897..52e687d2e4 100644 --- a/packages/core-components/src/components/Link/Link.test.tsx +++ b/packages/core-components/src/components/Link/Link.test.tsx @@ -24,8 +24,8 @@ import { import { analyticsApiRef, configApiRef } from '@backstage/core-plugin-api'; import { isExternalUri, Link, useResolvedPath } from './Link'; import { Route, Routes } from 'react-router'; -import { ConfigReader } from '@backstage/config'; import { renderHook, WrapperComponent } from '@testing-library/react-hooks'; +import { ConfigReader } from '@backstage/config'; describe('', () => { it('navigates using react-router', async () => { From cca591ee96cfc910843e1857f490f98f514517e2 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 29 Nov 2022 12:55:29 -0500 Subject: [PATCH 09/42] Add e2e test case to test cli build options and webpack subroute handling. Signed-off-by: Aramis Sennyey --- packages/cli/e2e-test.config.js | 22 +++ packages/cli/e2e-tests/serve.test.ts | 160 ++++++++++++++++++ .../e2e-tests/test-project/app-config.yaml | 6 + .../cli/e2e-tests/test-project/backstage.json | 3 + .../cli/e2e-tests/test-project/package.json | 54 ++++++ .../test-project/packages/app/package.json | 82 +++++++++ .../packages/app/public/index.html | 79 +++++++++ .../test-project/packages/app/src/index.tsx | 22 +++ .../packages/backend/package.json | 51 ++++++ .../packages/backend/src/index.ts | 15 ++ packages/cli/e2e-tests/test-project/yarn.lock | 0 packages/cli/package.json | 5 +- packages/cli/src/commands/build/command.ts | 1 + packages/cli/src/commands/index.ts | 8 + packages/cli/src/commands/repo/build.ts | 5 +- packages/cli/src/lib/bundler/server.ts | 2 +- packages/config-loader/src/lib/cli.test.ts | 86 ++++++++++ packages/config-loader/src/lib/cli.ts | 2 + yarn.lock | 1 + 19 files changed, 598 insertions(+), 6 deletions(-) create mode 100644 packages/cli/e2e-test.config.js create mode 100644 packages/cli/e2e-tests/serve.test.ts create mode 100644 packages/cli/e2e-tests/test-project/app-config.yaml create mode 100644 packages/cli/e2e-tests/test-project/backstage.json create mode 100644 packages/cli/e2e-tests/test-project/package.json create mode 100644 packages/cli/e2e-tests/test-project/packages/app/package.json create mode 100644 packages/cli/e2e-tests/test-project/packages/app/public/index.html create mode 100644 packages/cli/e2e-tests/test-project/packages/app/src/index.tsx create mode 100644 packages/cli/e2e-tests/test-project/packages/backend/package.json create mode 100644 packages/cli/e2e-tests/test-project/packages/backend/src/index.ts create mode 100644 packages/cli/e2e-tests/test-project/yarn.lock create mode 100644 packages/config-loader/src/lib/cli.test.ts diff --git a/packages/cli/e2e-test.config.js b/packages/cli/e2e-test.config.js new file mode 100644 index 0000000000..1c1cf7a75b --- /dev/null +++ b/packages/cli/e2e-test.config.js @@ -0,0 +1,22 @@ +/* + * 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. + */ + +const path = require('path'); + +module.exports = require('./config/jest').then(baseConfig => ({ + ...baseConfig, + rootDir: path.resolve(__dirname, 'e2e-tests'), +})); diff --git a/packages/cli/e2e-tests/serve.test.ts b/packages/cli/e2e-tests/serve.test.ts new file mode 100644 index 0000000000..b18ff64251 --- /dev/null +++ b/packages/cli/e2e-tests/serve.test.ts @@ -0,0 +1,160 @@ +/* + * 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 { execSync, spawn, SpawnOptionsWithoutStdio } from 'child_process'; +import EventEmitter from 'events'; +import mock from 'mock-fs'; +import fetch from 'node-fetch'; +import path from 'path'; + +const executeCommand = ( + command: string, + args: string[], + options?: SpawnOptionsWithoutStdio, + events?: EventEmitter, + eventConfig?: { + killRegex?: RegExp; + signalRegex?: RegExp[]; + }, +): Promise<{ + exit: number; + stdout: string; + stderr: string; +}> => { + return new Promise((resolve, reject) => { + const stdout: Buffer[] = []; + const stderr: Buffer[] = []; + + const shell = process.platform === 'win32'; + const proc = spawn(command, args, { ...options, shell }); + + proc.stdout?.on('data', data => { + stdout.push(Buffer.from(data)); + }); + + proc.stderr?.on('data', data => { + stderr.push(Buffer.from(data)); + }); + let intervalId: NodeJS.Timer | undefined = undefined; + if (eventConfig) { + intervalId = setInterval(() => { + const stdoutStr = Buffer.concat(stdout).toString('utf8'); + if (eventConfig.killRegex?.test(stdoutStr)) { + proc.kill('SIGINT'); + } else if ( + eventConfig.signalRegex?.some(regex => regex.test(stdoutStr)) + ) { + events?.emit('hit'); + } + }, 1000); + } + + events?.on('stop', signal => { + console.log(signal); + if (intervalId) { + try { + clearInterval(intervalId); + } catch (err) { + console.error(err); + } + } + proc.kill(signal); + }); + + proc.on('error', (...errorArgs) => { + if (intervalId) { + try { + clearInterval(intervalId); + } catch (err) { + console.error(err); + } + } + reject(errorArgs); + }); + proc.on('exit', code => { + if (intervalId) { + try { + clearInterval(intervalId); + } catch (err) { + console.error(err); + } + } + resolve({ + exit: code ?? 0, + stdout: Buffer.concat(stdout).toString('utf8'), + stderr: Buffer.concat(stderr).toString('utf8'), + }); + }); + }); +}; + +const timeout = 40000; + +jest.setTimeout(timeout * 2); + +describe('end-to-end', () => { + const entryPoint = path.resolve(__dirname, '../bin/backstage-cli'); + + it.skip('shows help text', async () => { + const proc = await executeCommand(entryPoint, ['--help']); + expect(proc.stdout).toContain('Usage: backstage-cli [options]'); + expect(proc.exit).toEqual(0); + }); + + it('builds frontend with correct url overrides', async () => { + const cwd = path.resolve(__dirname, 'test-project/packages/app'); + const buildProc = await executeCommand( + entryPoint, + ['package', 'build', '--public-path', '/test', '--backend-url', '/api'], + { + cwd, + }, + ); + expect(buildProc.stderr).toContain( + 'Loaded config from app-config.yaml, cli', + ); + + console.log(buildProc.stderr, buildProc.stdout); + + expect(buildProc.exit).toEqual(0); + }); + + it('starts frontend on correct url', async () => { + const cwd = path.resolve(__dirname, 'test-project/packages/app'); + + const startEmitter = new EventEmitter(); + startEmitter.on('hit', async () => { + const response = await fetch('http://localhost:3000/test/catalog'); + const text = await response.text(); + startEmitter.emit('stop', 'SIGINT'); + expect(response.status).toBe(200); + expect(text.length).toBeGreaterThan(0); + }); + const startProc = await executeCommand( + entryPoint, + ['package', 'start'], + { + cwd, + }, + startEmitter, + { + signalRegex: [/webpack compiled/], + }, + ); + + expect(startProc.exit).toEqual(0); + }); +}); diff --git a/packages/cli/e2e-tests/test-project/app-config.yaml b/packages/cli/e2e-tests/test-project/app-config.yaml new file mode 100644 index 0000000000..477580da86 --- /dev/null +++ b/packages/cli/e2e-tests/test-project/app-config.yaml @@ -0,0 +1,6 @@ +app: + baseUrl: http://localhost:3000/test + title: test + +backend: + baseUrl: http://localhost:7007 \ No newline at end of file diff --git a/packages/cli/e2e-tests/test-project/backstage.json b/packages/cli/e2e-tests/test-project/backstage.json new file mode 100644 index 0000000000..3e73a3c875 --- /dev/null +++ b/packages/cli/e2e-tests/test-project/backstage.json @@ -0,0 +1,3 @@ +{ + "version": "1.7.0-next.0" +} diff --git a/packages/cli/e2e-tests/test-project/package.json b/packages/cli/e2e-tests/test-project/package.json new file mode 100644 index 0000000000..7ebf258618 --- /dev/null +++ b/packages/cli/e2e-tests/test-project/package.json @@ -0,0 +1,54 @@ +{ + "name": "root", + "version": "1.0.0", + "private": true, + "engines": { + "node": "14 || 16" + }, + "scripts": { + "dev": "concurrently \"yarn start\" \"yarn start-backend\"", + "start": "yarn workspace app start", + "start-backend": "yarn workspace backend start", + "build": "backstage-cli repo build --all", + "build-image": "yarn workspace backend build-image", + "tsc": "tsc", + "tsc:full": "tsc --skipLibCheck false --incremental false", + "clean": "backstage-cli repo clean", + "test": "backstage-cli repo test", + "test:all": "backstage-cli repo test --coverage", + "lint": "backstage-cli repo lint --since origin/master", + "lint:all": "backstage-cli repo lint", + "prettier:check": "prettier --check .", + "create-plugin": "backstage-cli create-plugin --scope internal", + "new": "backstage-cli new --scope internal" + }, + "workspaces": { + "packages": [ + "packages/*", + "plugins/*" + ] + }, + "devDependencies": { + "@backstage/cli": "^0.20.0-next.0", + "@spotify/prettier-config": "^12.0.0", + "concurrently": "^6.0.0", + "lerna": "^4.0.0", + "node-gyp": "^9.0.0", + "prettier": "^2.3.2", + "typescript": "~4.6.4" + }, + "resolutions": { + "@types/react": "^17", + "@types/react-dom": "^17" + }, + "prettier": "@spotify/prettier-config", + "lint-staged": { + "*.{js,jsx,ts,tsx,mjs,cjs}": [ + "eslint --fix", + "prettier --write" + ], + "*.{json,md}": [ + "prettier --write" + ] + } +} diff --git a/packages/cli/e2e-tests/test-project/packages/app/package.json b/packages/cli/e2e-tests/test-project/packages/app/package.json new file mode 100644 index 0000000000..1157a698f5 --- /dev/null +++ b/packages/cli/e2e-tests/test-project/packages/app/package.json @@ -0,0 +1,82 @@ +{ + "name": "app", + "version": "0.0.0", + "private": true, + "bundled": true, + "backstage": { + "role": "frontend" + }, + "scripts": { + "start": "backstage-cli package start", + "build": "backstage-cli package build", + "clean": "backstage-cli package clean", + "test": "backstage-cli package test", + "lint": "backstage-cli package lint", + "test:e2e": "cross-env PORT=3001 start-server-and-test start http://localhost:3001 cy:dev", + "test:e2e:ci": "cross-env PORT=3001 start-server-and-test start http://localhost:3001 cy:run", + "cy:dev": "cypress open", + "cy:run": "cypress run --browser chrome" + }, + "dependencies": { + "@backstage/app-defaults": "^1.0.7-next.0", + "@backstage/catalog-model": "^1.1.2-next.0", + "@backstage/cli": "^0.20.0-next.0", + "@backstage/core-app-api": "^1.1.1-next.0", + "@backstage/core-components": "^0.11.2-next.0", + "@backstage/core-plugin-api": "^1.0.7-next.0", + "@backstage/integration-react": "^1.1.5-next.0", + "@backstage/plugin-api-docs": "^0.8.10-next.0", + "@backstage/plugin-catalog": "^1.5.2-next.0", + "@backstage/plugin-catalog-common": "^1.0.7-next.0", + "@backstage/plugin-catalog-graph": "^0.2.22-next.0", + "@backstage/plugin-catalog-import": "^0.8.13-next.0", + "@backstage/plugin-catalog-react": "^1.1.5-next.0", + "@backstage/plugin-github-actions": "^0.5.10-next.0", + "@backstage/plugin-org": "^0.5.10-next.0", + "@backstage/plugin-permission-react": "^0.4.6-next.0", + "@backstage/plugin-scaffolder": "^1.7.0-next.0", + "@backstage/plugin-search": "^1.0.3-next.0", + "@backstage/plugin-search-react": "^1.1.1-next.0", + "@backstage/plugin-tech-radar": "^0.5.17-next.0", + "@backstage/plugin-techdocs": "^1.3.3-next.0", + "@backstage/plugin-techdocs-module-addons-contrib": "^1.0.5-next.0", + "@backstage/plugin-techdocs-react": "^1.0.5-next.0", + "@backstage/plugin-user-settings": "^0.5.0-next.0", + "@backstage/theme": "^0.2.16", + "@material-ui/core": "^4.12.2", + "@material-ui/icons": "^4.9.1", + "history": "^5.0.0", + "react": "^17.0.2", + "react-dom": "^17.0.2", + "react-router": "^6.3.0", + "react-router-dom": "^6.3.0", + "react-use": "^17.2.4" + }, + "devDependencies": { + "@backstage/test-utils": "^1.2.1-next.0", + "@testing-library/jest-dom": "^5.10.1", + "@testing-library/react": "^12.1.3", + "@testing-library/user-event": "^14.0.0", + "@types/node": "^16.11.26", + "@types/react-dom": "*", + "cross-env": "^7.0.0", + "cypress": "^9.7.0", + "eslint-plugin-cypress": "^2.10.3", + "start-server-and-test": "^1.10.11" + }, + "browserslist": { + "production": [ + ">0.2%", + "not dead", + "not op_mini all" + ], + "development": [ + "last 1 chrome version", + "last 1 firefox version", + "last 1 safari version" + ] + }, + "files": [ + "dist" + ] +} diff --git a/packages/cli/e2e-tests/test-project/packages/app/public/index.html b/packages/cli/e2e-tests/test-project/packages/app/public/index.html new file mode 100644 index 0000000000..a936c73602 --- /dev/null +++ b/packages/cli/e2e-tests/test-project/packages/app/public/index.html @@ -0,0 +1,79 @@ + + + + + + + + + + + + + + + + + <%= config.getString('app.title') %> + <% if (config.has('app.googleAnalyticsTrackingId')) { %> + + + <% } %> + + + +
+ + + diff --git a/packages/cli/e2e-tests/test-project/packages/app/src/index.tsx b/packages/cli/e2e-tests/test-project/packages/app/src/index.tsx new file mode 100644 index 0000000000..145adc1f48 --- /dev/null +++ b/packages/cli/e2e-tests/test-project/packages/app/src/index.tsx @@ -0,0 +1,22 @@ +/* + * 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 '@backstage/cli/asset-types'; +import React from 'react'; +import ReactDOM from 'react-dom'; +import App from './App'; + +ReactDOM.render(
, document.getElementById('root')); diff --git a/packages/cli/e2e-tests/test-project/packages/backend/package.json b/packages/cli/e2e-tests/test-project/packages/backend/package.json new file mode 100644 index 0000000000..5c649f08b6 --- /dev/null +++ b/packages/cli/e2e-tests/test-project/packages/backend/package.json @@ -0,0 +1,51 @@ +{ + "name": "backend", + "version": "0.0.0", + "main": "dist/index.cjs.js", + "types": "src/index.ts", + "private": true, + "backstage": { + "role": "backend" + }, + "scripts": { + "start": "backstage-cli package start", + "build": "backstage-cli package build", + "lint": "backstage-cli package lint", + "test": "backstage-cli package test", + "clean": "backstage-cli package clean" + }, + "dependencies": { + "@backstage/backend-common": "^0.15.0", + "@backstage/backend-tasks": "^0.3.4", + "@backstage/catalog-client": "^1.0.4", + "@backstage/catalog-model": "^1.1.0", + "@backstage/config": "^1.0.1", + "@backstage/plugin-app-backend": "^0.3.35", + "@backstage/plugin-auth-backend": "^0.15.1", + "@backstage/plugin-catalog-backend": "^1.3.1", + "@backstage/plugin-permission-common": "^0.6.3", + "@backstage/plugin-permission-node": "^0.6.4", + "@backstage/plugin-proxy-backend": "^0.2.29", + "@backstage/plugin-scaffolder-backend": "^1.5.0", + "@backstage/plugin-search-backend": "^1.0.1", + "@backstage/plugin-search-backend-module-pg": "^0.3.6", + "@backstage/plugin-search-backend-node": "^1.0.1", + "@backstage/plugin-techdocs-backend": "^1.2.1", + "dockerode": "^3.3.1", + "express": "^4.17.1", + "express-promise-router": "^4.1.0", + "pg": "^8.3.0", + "winston": "^3.2.1" + }, + "devDependencies": { + "@backstage/cli": "^0.18.1", + "@types/dockerode": "^3.3.0", + "@types/express": "^4.17.6", + "@types/express-serve-static-core": "^4.17.5", + "@types/luxon": "^2.0.4", + "better-sqlite3": "^7.5.0" + }, + "files": [ + "dist" + ] +} diff --git a/packages/cli/e2e-tests/test-project/packages/backend/src/index.ts b/packages/cli/e2e-tests/test-project/packages/backend/src/index.ts new file mode 100644 index 0000000000..b61d59e88d --- /dev/null +++ b/packages/cli/e2e-tests/test-project/packages/backend/src/index.ts @@ -0,0 +1,15 @@ +/* + * 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. + */ diff --git a/packages/cli/e2e-tests/test-project/yarn.lock b/packages/cli/e2e-tests/test-project/yarn.lock new file mode 100644 index 0000000000..e69de29bb2 diff --git a/packages/cli/package.json b/packages/cli/package.json index 5a65998301..d605c6f31f 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -24,7 +24,9 @@ "lint": "backstage-cli package lint", "test": "backstage-cli package test", "clean": "backstage-cli package clean", - "start": "nodemon --" + "start": "nodemon --", + "test:e2e": "backstage-cli test --config e2e-test.config.js", + "test:e2e:ci": "backstage-cli test --config e2e-test.config.js --watchAll=false --ci" }, "bin": { "backstage-cli": "bin/backstage-cli" @@ -153,6 +155,7 @@ "@types/tar": "^6.1.1", "@types/terser-webpack-plugin": "^5.0.4", "@types/yarnpkg__lockfile": "^1.1.4", + "cross-fetch": "^3.1.5", "del": "^6.0.0", "mock-fs": "^5.1.0", "msw": "^0.49.0", diff --git a/packages/cli/src/commands/build/command.ts b/packages/cli/src/commands/build/command.ts index fcaf58e1ee..b5120f7ae0 100644 --- a/packages/cli/src/commands/build/command.ts +++ b/packages/cli/src/commands/build/command.ts @@ -29,6 +29,7 @@ export async function command(opts: OptionValues): Promise { targetDir: paths.targetDir, configPaths: opts.config as string[], writeStats: Boolean(opts.stats), + cliOptions: opts, }); } if (role === 'backend') { diff --git a/packages/cli/src/commands/index.ts b/packages/cli/src/commands/index.ts index 33291cb70c..90d3afd4d5 100644 --- a/packages/cli/src/commands/index.ts +++ b/packages/cli/src/commands/index.ts @@ -134,6 +134,14 @@ export function registerScriptCommand(program: Command) { '--stats', 'If bundle stats are available, write them to the output directory. Applies to app packages only.', ) + .option( + '--public-path ', + 'Public path for hosting the website on, can be relative.', + ) + .option( + '--backend-url ', + 'Backend url, expects just the origin or sub-route. Do not include /api. Can be relative.', + ) .option( '--config ', 'Config files to load instead of app-config.yaml. Applies to app packages only.', diff --git a/packages/cli/src/commands/repo/build.ts b/packages/cli/src/commands/repo/build.ts index 86d2e781a6..55574a53b0 100644 --- a/packages/cli/src/commands/repo/build.ts +++ b/packages/cli/src/commands/repo/build.ts @@ -153,10 +153,7 @@ export async function command(opts: OptionValues, cmd: Command): Promise { return; } await buildFrontend({ - cliOptions: { - publicPath: opts.publicPath, - backendUrl: opts.backendUrl, - }, + cliOptions: opts, targetDir: pkg.dir, configPaths: (buildOptions.config as string[]) ?? [], writeStats: Boolean(buildOptions.stats), diff --git a/packages/cli/src/lib/bundler/server.ts b/packages/cli/src/lib/bundler/server.ts index 1e76fbc662..cef55d3265 100644 --- a/packages/cli/src/lib/bundler/server.ts +++ b/packages/cli/src/lib/bundler/server.ts @@ -62,7 +62,7 @@ export async function serveBundle(options: ServeOptions) { disableDotRule: true, // The index needs to be rewritten relative to the new public path, including subroutes. - index: config.output?.publicPath ?? '/index.html', + index: `${config.output?.publicPath}index.html`, }, https: url.protocol === 'https:' diff --git a/packages/config-loader/src/lib/cli.test.ts b/packages/config-loader/src/lib/cli.test.ts new file mode 100644 index 0000000000..4f4e76142a --- /dev/null +++ b/packages/config-loader/src/lib/cli.test.ts @@ -0,0 +1,86 @@ +/* + * 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 { readCliConfig } from './cli'; + +describe('readCliConfig', () => { + it('should return empty config for empty cli', () => { + expect(readCliConfig({})).toEqual([]); + }); + + it('should return empty config for no matching keys', () => { + expect( + readCliConfig({ + test: '123', + } as any), + ).toEqual([]); + }); + + it('should return backend.baseUrl when backendUrl present in cli options', () => { + expect( + readCliConfig({ + backendUrl: 'http://localhost:3000', + }), + ).toEqual([ + { + data: { + backend: { + baseUrl: 'http://localhost:3000', + }, + }, + context: 'cli', + }, + ]); + }); + + it('should return app.baseUrl when publicPath present in cli options', () => { + expect( + readCliConfig({ + publicPath: 'http://localhost:3000', + }), + ).toEqual([ + { + data: { + app: { + baseUrl: 'http://localhost:3000', + }, + }, + context: 'cli', + }, + ]); + }); + + it('should return app.baseUrl and backend.baseUrl when publicPath and backendUrl present in cli options', () => { + expect( + readCliConfig({ + publicPath: 'http://localhost:3000', + backendUrl: 'http://localhost:3000/api', + }), + ).toEqual([ + { + data: { + app: { + baseUrl: 'http://localhost:3000', + }, + backend: { + baseUrl: 'http://localhost:3000/api', + }, + }, + context: 'cli', + }, + ]); + }); +}); diff --git a/packages/config-loader/src/lib/cli.ts b/packages/config-loader/src/lib/cli.ts index 9d935e2826..5e6eacf293 100644 --- a/packages/config-loader/src/lib/cli.ts +++ b/packages/config-loader/src/lib/cli.ts @@ -45,5 +45,7 @@ export function readCliConfig(opts?: CliConfigOptions): AppConfig[] { }; } + if (Object.keys(data).length === 0) return []; + return [{ data, context: 'cli' }]; } diff --git a/yarn.lock b/yarn.lock index 621ed3dd96..7fa022e4c6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3653,6 +3653,7 @@ __metadata: chalk: ^4.0.0 chokidar: ^3.3.1 commander: ^9.1.0 + cross-fetch: ^3.1.5 css-loader: ^6.5.1 del: ^6.0.0 diff: ^5.0.0 From 09d930c2eba3d9ac8313b5ee3d665b6401ca3973 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 29 Nov 2022 12:56:20 -0500 Subject: [PATCH 10/42] Add changesets and remove routing changes. Signed-off-by: Aramis Sennyey --- .changeset/quick-islands-tease.md | 9 --------- .changeset/short-games-march.md | 5 +++++ .changeset/spotty-numbers-occur.md | 11 +++++++++++ .changeset/young-apples-hide.md | 5 +++++ .github/vale/Vocab/Backstage/accept.txt | 4 ---- packages/app/src/App.tsx | 2 +- packages/core-app-api/src/app/AppManager.tsx | 2 +- 7 files changed, 23 insertions(+), 15 deletions(-) delete mode 100644 .changeset/quick-islands-tease.md create mode 100644 .changeset/short-games-march.md create mode 100644 .changeset/spotty-numbers-occur.md create mode 100644 .changeset/young-apples-hide.md diff --git a/.changeset/quick-islands-tease.md b/.changeset/quick-islands-tease.md deleted file mode 100644 index 9498e67539..0000000000 --- a/.changeset/quick-islands-tease.md +++ /dev/null @@ -1,9 +0,0 @@ ---- -'@backstage/app-defaults': minor -'@backstage/cli': minor -'@backstage/config-loader': minor -'@backstage/core-app-api': minor -'@backstage/core-components': minor ---- - -Added support for adding CLI options for setting frontend public path (for example, /backstage) as well as the backend url that the front end uses to communicate with the backend. Both of these can now be specified as relative. diff --git a/.changeset/short-games-march.md b/.changeset/short-games-march.md new file mode 100644 index 0000000000..54c40b943e --- /dev/null +++ b/.changeset/short-games-march.md @@ -0,0 +1,5 @@ +--- +'@backstage/app-defaults': minor +--- + +Added support for parsing relative backend urls to the api factory. Allows relative backend urls to be specified in config options. diff --git a/.changeset/spotty-numbers-occur.md b/.changeset/spotty-numbers-occur.md new file mode 100644 index 0000000000..1b69516617 --- /dev/null +++ b/.changeset/spotty-numbers-occur.md @@ -0,0 +1,11 @@ +--- +'@backstage/cli': minor +--- + +Adds support for two new CLI options on the `repo build` and `package build` commands. Both options are only available to frontend components. Backend config continues to be supplied at runtime. + +CLI options take precedent over any provided config. + +The new options are meant to add greater support for running the same bundle in multiple environments, ex: test, stage and prod. +`--publicPath`: Frontend subroute to host the app on, can be relative or absolute. +`--backendUrl`: Backend url that the {backend.basePath}, e.g. /api, endpoint can be reached on. Can be relative or absolute. diff --git a/.changeset/young-apples-hide.md b/.changeset/young-apples-hide.md new file mode 100644 index 0000000000..c445c45ead --- /dev/null +++ b/.changeset/young-apples-hide.md @@ -0,0 +1,5 @@ +--- +'@backstage/config-loader': minor +--- + +Added new CLI parsing that overrides all other config options, like env and yaml. Currently supports --publicPath and --backendUrl. diff --git a/.github/vale/Vocab/Backstage/accept.txt b/.github/vale/Vocab/Backstage/accept.txt index 3278176bee..ca031c24ca 100644 --- a/.github/vale/Vocab/Backstage/accept.txt +++ b/.github/vale/Vocab/Backstage/accept.txt @@ -32,7 +32,6 @@ boolean Brex builtins callout -CDNs Chai changeset changesets @@ -157,7 +156,6 @@ JaCoCo JavaScript jenkins Jira -JWTs jq js json @@ -167,7 +165,6 @@ JWTs Kaewkasi Keyv Knex -KPIs kubectl kubernetes kubernetes @@ -397,7 +394,6 @@ widget's winston www WWW -XCMetrics XML xyz yaml diff --git a/packages/app/src/App.tsx b/packages/app/src/App.tsx index fd73e99bcb..e766df868d 100644 --- a/packages/app/src/App.tsx +++ b/packages/app/src/App.tsx @@ -150,7 +150,7 @@ const AppRouter = app.getRouter(); const routes = ( - } /> + } /> {/* TODO(rubenl): Move this to / once its more mature and components exist */} }> {homePage} diff --git a/packages/core-app-api/src/app/AppManager.tsx b/packages/core-app-api/src/app/AppManager.tsx index 571eaf72e0..bbd27f84ce 100644 --- a/packages/core-app-api/src/app/AppManager.tsx +++ b/packages/core-app-api/src/app/AppManager.tsx @@ -116,7 +116,7 @@ function getBasePath(configApi: Config) { function readBasePath(configApi: ConfigApi) { let { pathname } = new URL( configApi.getOptionalString('app.baseUrl') ?? '/', - document.location.origin, // baseUrl can be specified as just a path + 'http://dummy.dev', // baseUrl can be specified as just a path ); pathname = pathname.replace(/\/*$/, ''); return pathname; From 1a845e86a95d62edc5a8be68a902bcc66e8fb82a Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 29 Nov 2022 12:56:54 -0500 Subject: [PATCH 11/42] Remove cli options parsing from @backstage/config-loader. Update fixtures. Signed-off-by: Aramis Sennyey --- packages/cli/e2e-test.config.js | 22 ------ .../e2e-tests/test-project/app-config.yaml | 6 -- packages/cli/package.json | 2 +- .../__fixtures__/test-project/app-config.yaml | 6 ++ .../__fixtures__}/test-project/backstage.json | 0 .../__fixtures__}/test-project/package.json | 0 .../test-project/packages/app/package.json | 0 .../packages/app/public/index.html | 0 .../test-project/packages/app/src/index.tsx | 1 - .../packages/backend/package.json | 0 .../packages/backend/src/index.ts | 1 + .../__fixtures__}/test-project/yarn.lock | 0 .../cli/src/commands/build/buildFrontend.ts | 3 +- .../src/lib/config.test.ts} | 2 +- packages/cli/src/lib/config.ts | 47 +++++++++++-- packages/cli/{e2e-tests => src}/serve.test.ts | 68 ++++++++++--------- packages/config-loader/src/lib/cli.ts | 51 -------------- packages/config-loader/src/loader.ts | 12 +--- yarn.lock | 2 +- 19 files changed, 89 insertions(+), 134 deletions(-) delete mode 100644 packages/cli/e2e-test.config.js delete mode 100644 packages/cli/e2e-tests/test-project/app-config.yaml create mode 100644 packages/cli/src/__fixtures__/test-project/app-config.yaml rename packages/cli/{e2e-tests => src/__fixtures__}/test-project/backstage.json (100%) rename packages/cli/{e2e-tests => src/__fixtures__}/test-project/package.json (100%) rename packages/cli/{e2e-tests => src/__fixtures__}/test-project/packages/app/package.json (100%) rename packages/cli/{e2e-tests => src/__fixtures__}/test-project/packages/app/public/index.html (100%) rename packages/cli/{e2e-tests => src/__fixtures__}/test-project/packages/app/src/index.tsx (96%) rename packages/cli/{e2e-tests => src/__fixtures__}/test-project/packages/backend/package.json (100%) rename packages/cli/{e2e-tests => src/__fixtures__}/test-project/packages/backend/src/index.ts (98%) rename packages/cli/{e2e-tests => src/__fixtures__}/test-project/yarn.lock (100%) rename packages/{config-loader/src/lib/cli.test.ts => cli/src/lib/config.test.ts} (98%) rename packages/cli/{e2e-tests => src}/serve.test.ts (77%) delete mode 100644 packages/config-loader/src/lib/cli.ts diff --git a/packages/cli/e2e-test.config.js b/packages/cli/e2e-test.config.js deleted file mode 100644 index 1c1cf7a75b..0000000000 --- a/packages/cli/e2e-test.config.js +++ /dev/null @@ -1,22 +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. - */ - -const path = require('path'); - -module.exports = require('./config/jest').then(baseConfig => ({ - ...baseConfig, - rootDir: path.resolve(__dirname, 'e2e-tests'), -})); diff --git a/packages/cli/e2e-tests/test-project/app-config.yaml b/packages/cli/e2e-tests/test-project/app-config.yaml deleted file mode 100644 index 477580da86..0000000000 --- a/packages/cli/e2e-tests/test-project/app-config.yaml +++ /dev/null @@ -1,6 +0,0 @@ -app: - baseUrl: http://localhost:3000/test - title: test - -backend: - baseUrl: http://localhost:7007 \ No newline at end of file diff --git a/packages/cli/package.json b/packages/cli/package.json index d605c6f31f..0fcd8cd89f 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -86,6 +86,7 @@ "express": "^4.17.1", "fork-ts-checker-webpack-plugin": "^7.0.0-alpha.8", "fs-extra": "10.1.0", + "get-port": "^6.1.2", "glob": "^7.1.7", "global-agent": "^3.0.0", "handlebars": "^4.7.3", @@ -155,7 +156,6 @@ "@types/tar": "^6.1.1", "@types/terser-webpack-plugin": "^5.0.4", "@types/yarnpkg__lockfile": "^1.1.4", - "cross-fetch": "^3.1.5", "del": "^6.0.0", "mock-fs": "^5.1.0", "msw": "^0.49.0", diff --git a/packages/cli/src/__fixtures__/test-project/app-config.yaml b/packages/cli/src/__fixtures__/test-project/app-config.yaml new file mode 100644 index 0000000000..c4d4705aa5 --- /dev/null +++ b/packages/cli/src/__fixtures__/test-project/app-config.yaml @@ -0,0 +1,6 @@ +app: + baseUrl: http://localhost:${PORT}/test + title: test + +backend: + baseUrl: http://localhost:${BACKEND_PORT} \ No newline at end of file diff --git a/packages/cli/e2e-tests/test-project/backstage.json b/packages/cli/src/__fixtures__/test-project/backstage.json similarity index 100% rename from packages/cli/e2e-tests/test-project/backstage.json rename to packages/cli/src/__fixtures__/test-project/backstage.json diff --git a/packages/cli/e2e-tests/test-project/package.json b/packages/cli/src/__fixtures__/test-project/package.json similarity index 100% rename from packages/cli/e2e-tests/test-project/package.json rename to packages/cli/src/__fixtures__/test-project/package.json diff --git a/packages/cli/e2e-tests/test-project/packages/app/package.json b/packages/cli/src/__fixtures__/test-project/packages/app/package.json similarity index 100% rename from packages/cli/e2e-tests/test-project/packages/app/package.json rename to packages/cli/src/__fixtures__/test-project/packages/app/package.json diff --git a/packages/cli/e2e-tests/test-project/packages/app/public/index.html b/packages/cli/src/__fixtures__/test-project/packages/app/public/index.html similarity index 100% rename from packages/cli/e2e-tests/test-project/packages/app/public/index.html rename to packages/cli/src/__fixtures__/test-project/packages/app/public/index.html diff --git a/packages/cli/e2e-tests/test-project/packages/app/src/index.tsx b/packages/cli/src/__fixtures__/test-project/packages/app/src/index.tsx similarity index 96% rename from packages/cli/e2e-tests/test-project/packages/app/src/index.tsx rename to packages/cli/src/__fixtures__/test-project/packages/app/src/index.tsx index 145adc1f48..3c96b4476d 100644 --- a/packages/cli/e2e-tests/test-project/packages/app/src/index.tsx +++ b/packages/cli/src/__fixtures__/test-project/packages/app/src/index.tsx @@ -17,6 +17,5 @@ import '@backstage/cli/asset-types'; import React from 'react'; import ReactDOM from 'react-dom'; -import App from './App'; ReactDOM.render(
, document.getElementById('root')); diff --git a/packages/cli/e2e-tests/test-project/packages/backend/package.json b/packages/cli/src/__fixtures__/test-project/packages/backend/package.json similarity index 100% rename from packages/cli/e2e-tests/test-project/packages/backend/package.json rename to packages/cli/src/__fixtures__/test-project/packages/backend/package.json diff --git a/packages/cli/e2e-tests/test-project/packages/backend/src/index.ts b/packages/cli/src/__fixtures__/test-project/packages/backend/src/index.ts similarity index 98% rename from packages/cli/e2e-tests/test-project/packages/backend/src/index.ts rename to packages/cli/src/__fixtures__/test-project/packages/backend/src/index.ts index b61d59e88d..8b9b6bd586 100644 --- a/packages/cli/e2e-tests/test-project/packages/backend/src/index.ts +++ b/packages/cli/src/__fixtures__/test-project/packages/backend/src/index.ts @@ -13,3 +13,4 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +export {}; diff --git a/packages/cli/e2e-tests/test-project/yarn.lock b/packages/cli/src/__fixtures__/test-project/yarn.lock similarity index 100% rename from packages/cli/e2e-tests/test-project/yarn.lock rename to packages/cli/src/__fixtures__/test-project/yarn.lock diff --git a/packages/cli/src/commands/build/buildFrontend.ts b/packages/cli/src/commands/build/buildFrontend.ts index 7f1687ca03..92e42a8d64 100644 --- a/packages/cli/src/commands/build/buildFrontend.ts +++ b/packages/cli/src/commands/build/buildFrontend.ts @@ -18,8 +18,7 @@ import fs from 'fs-extra'; import { resolve as resolvePath } from 'path'; import { buildBundle } from '../../lib/bundler'; import { getEnvironmentParallelism } from '../../lib/parallel'; -import { loadCliConfig } from '../../lib/config'; -import { CliConfigOptions } from '@backstage/config-loader/src/lib/cli'; +import { loadCliConfig, CliConfigOptions } from '../../lib/config'; interface BuildAppOptions { targetDir: string; diff --git a/packages/config-loader/src/lib/cli.test.ts b/packages/cli/src/lib/config.test.ts similarity index 98% rename from packages/config-loader/src/lib/cli.test.ts rename to packages/cli/src/lib/config.test.ts index 4f4e76142a..bc6d8b6931 100644 --- a/packages/config-loader/src/lib/cli.test.ts +++ b/packages/cli/src/lib/config.test.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { readCliConfig } from './cli'; +import { readCliConfig } from './config'; describe('readCliConfig', () => { it('should return empty config for empty cli', () => { diff --git a/packages/cli/src/lib/config.ts b/packages/cli/src/lib/config.ts index e28f2d7f6f..1220586ce0 100644 --- a/packages/cli/src/lib/config.ts +++ b/packages/cli/src/lib/config.ts @@ -19,11 +19,17 @@ import { loadConfig, loadConfigSchema, } from '@backstage/config-loader'; -import { ConfigReader } from '@backstage/config'; +import { AppConfig, ConfigReader } from '@backstage/config'; import { paths } from './paths'; import { isValidUrl } from './urls'; import { getPackages } from '@manypkg/get-packages'; import { PackageGraph } from './monorepo'; +import { JsonObject } from '@backstage/types'; + +export type CliConfigOptions = { + publicPath?: string; + backendUrl?: string; +}; type Options = { args: string[]; @@ -32,12 +38,37 @@ type Options = { withFilteredKeys?: boolean; withDeprecatedKeys?: boolean; fullVisibility?: boolean; - cliOptions?: { - publicPath?: string; - backendUrl?: string; - }; + cliOptions?: CliConfigOptions; }; +/** + * Read specific parameters from the CLI and add them to the build config. + * @param opts CLI passed parameters. + * @returns Array of config, empty if there is no relevant passed in cli options. + * + * @public + */ +export function readCliConfig(opts?: CliConfigOptions): AppConfig[] { + if (!opts || Object.keys(opts).length === 0) return []; + const data: JsonObject = {}; + + if (opts.publicPath) { + data.app = { + baseUrl: opts.publicPath, + }; + } + + if (opts.backendUrl) { + data.backend = { + baseUrl: opts.backendUrl, + }; + } + + if (Object.keys(data).length === 0) return []; + + return [{ data, context: 'cli' }]; +} + export async function loadCliConfig(options: Options) { const configTargets: ConfigTarget[] = []; options.args.forEach(arg => { @@ -76,15 +107,19 @@ export async function loadCliConfig(options: Options) { packagePaths: [paths.resolveTargetRoot('package.json')], }); + const cliConfigs = readCliConfig(options.cliOptions); + const { appConfigs } = await loadConfig({ experimentalEnvFunc: options.mockEnv ? async name => process.env[name] || 'x' : undefined, - cliOptions: options.cliOptions, configRoot: paths.targetRoot, configTargets: configTargets, }); + // Add the cliConfigs to the end of the appConfigs array for final overriding. + appConfigs.push(...cliConfigs); + // printing to stderr to not clobber stdout in case the cli command // outputs structured data (e.g. as config:schema does) process.stderr.write( diff --git a/packages/cli/e2e-tests/serve.test.ts b/packages/cli/src/serve.test.ts similarity index 77% rename from packages/cli/e2e-tests/serve.test.ts rename to packages/cli/src/serve.test.ts index b18ff64251..f43ea8053d 100644 --- a/packages/cli/e2e-tests/serve.test.ts +++ b/packages/cli/src/serve.test.ts @@ -14,11 +14,11 @@ * limitations under the License. */ -import { execSync, spawn, SpawnOptionsWithoutStdio } from 'child_process'; +import { spawn, SpawnOptionsWithoutStdio } from 'child_process'; import EventEmitter from 'events'; -import mock from 'mock-fs'; import fetch from 'node-fetch'; import path from 'path'; +import getPort from 'get-port'; const executeCommand = ( command: string, @@ -48,6 +48,11 @@ const executeCommand = ( proc.stderr?.on('data', data => { stderr.push(Buffer.from(data)); }); + + /** + * Set an interval to check if we should kill the process. + * This was the easiest way I could think of of testing across two processes. + */ let intervalId: NodeJS.Timer | undefined = undefined; if (eventConfig) { intervalId = setInterval(() => { @@ -62,8 +67,7 @@ const executeCommand = ( }, 1000); } - events?.on('stop', signal => { - console.log(signal); + const clearEventInterval = () => { if (intervalId) { try { clearInterval(intervalId); @@ -71,27 +75,22 @@ const executeCommand = ( console.error(err); } } + }; + + /** + * Need a way to kill the process from another process. + */ + events?.on('stop', signal => { + clearEventInterval(); proc.kill(signal); }); proc.on('error', (...errorArgs) => { - if (intervalId) { - try { - clearInterval(intervalId); - } catch (err) { - console.error(err); - } - } + clearEventInterval(); reject(errorArgs); }); proc.on('exit', code => { - if (intervalId) { - try { - clearInterval(intervalId); - } catch (err) { - console.error(err); - } - } + clearEventInterval(); resolve({ exit: code ?? 0, stdout: Buffer.concat(stdout).toString('utf8'), @@ -101,43 +100,41 @@ const executeCommand = ( }); }; -const timeout = 40000; +const timeout = 100000; +// Builds initially (with no cache) take a loooong time. jest.setTimeout(timeout * 2); +const testProjectDir = path.resolve( + __dirname, + '__fixtures__/test-project/packages/app', +); + describe('end-to-end', () => { const entryPoint = path.resolve(__dirname, '../bin/backstage-cli'); - it.skip('shows help text', async () => { - const proc = await executeCommand(entryPoint, ['--help']); - expect(proc.stdout).toContain('Usage: backstage-cli [options]'); - expect(proc.exit).toEqual(0); - }); - it('builds frontend with correct url overrides', async () => { - const cwd = path.resolve(__dirname, 'test-project/packages/app'); const buildProc = await executeCommand( entryPoint, ['package', 'build', '--public-path', '/test', '--backend-url', '/api'], { - cwd, + cwd: testProjectDir, }, ); expect(buildProc.stderr).toContain( 'Loaded config from app-config.yaml, cli', ); - console.log(buildProc.stderr, buildProc.stdout); - expect(buildProc.exit).toEqual(0); }); it('starts frontend on correct url', async () => { - const cwd = path.resolve(__dirname, 'test-project/packages/app'); - const startEmitter = new EventEmitter(); + const frontendPort = await getPort(); startEmitter.on('hit', async () => { - const response = await fetch('http://localhost:3000/test/catalog'); + const response = await fetch( + `http://localhost:3000/${frontendPort}/catalog`, + ); const text = await response.text(); startEmitter.emit('stop', 'SIGINT'); expect(response.status).toBe(200); @@ -147,7 +144,12 @@ describe('end-to-end', () => { entryPoint, ['package', 'start'], { - cwd, + cwd: testProjectDir, + env: { + ...process.env, + PORT: `${frontendPort}`, + BACKEND_PORT: `${await getPort()}`, + }, }, startEmitter, { diff --git a/packages/config-loader/src/lib/cli.ts b/packages/config-loader/src/lib/cli.ts deleted file mode 100644 index 5e6eacf293..0000000000 --- a/packages/config-loader/src/lib/cli.ts +++ /dev/null @@ -1,51 +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 { AppConfig } from '@backstage/config'; -import { JsonObject } from '@backstage/types'; - -export type CliConfigOptions = { - publicPath?: string; - backendUrl?: string; -}; - -/** - * Read specific parameters from the CLI and add them to the build config. - * @param opts CLI passed parameters. - * @returns Array of config, empty if there is no relevant passed in cli options. - * - * @public - */ -export function readCliConfig(opts?: CliConfigOptions): AppConfig[] { - if (!opts || Object.keys(opts).length === 0) return []; - const data: JsonObject = {}; - - if (opts.publicPath) { - data.app = { - baseUrl: opts.publicPath, - }; - } - - if (opts.backendUrl) { - data.backend = { - baseUrl: opts.backendUrl, - }; - } - - if (Object.keys(data).length === 0) return []; - - return [{ data, context: 'cli' }]; -} diff --git a/packages/config-loader/src/loader.ts b/packages/config-loader/src/loader.ts index 44bd4f94e8..f5ce0eb80c 100644 --- a/packages/config-loader/src/loader.ts +++ b/packages/config-loader/src/loader.ts @@ -28,7 +28,6 @@ import { readEnvConfig, } from './lib'; import fetch from 'node-fetch'; -import { CliConfigOptions, readCliConfig } from './lib/cli'; /** @public */ export type ConfigTarget = { path: string } | { url: string }; @@ -82,11 +81,6 @@ export type LoadConfigOptions = { * An optional configuration that enables watching of config files. */ watch?: LoadConfigOptionsWatch; - - /** - * New options from the CLI that affect the build config. - */ - cliOptions?: CliConfigOptions; }; /** @@ -236,8 +230,6 @@ export async function loadConfig( } } - const cliConfigs = readCliConfig(options.cliOptions); - const envConfigs = readEnvConfig(process.env); const watchConfigFile = (watchProp: LoadConfigOptionsWatch) => { @@ -326,7 +318,7 @@ export async function loadConfig( return { appConfigs: remote - ? [...remoteConfigs, ...fileConfigs, ...envConfigs, ...cliConfigs] - : [...fileConfigs, ...envConfigs, ...cliConfigs], + ? [...remoteConfigs, ...fileConfigs, ...envConfigs] + : [...fileConfigs, ...envConfigs], }; } diff --git a/yarn.lock b/yarn.lock index 7fa022e4c6..634623d6b4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3653,7 +3653,6 @@ __metadata: chalk: ^4.0.0 chokidar: ^3.3.1 commander: ^9.1.0 - cross-fetch: ^3.1.5 css-loader: ^6.5.1 del: ^6.0.0 diff: ^5.0.0 @@ -3673,6 +3672,7 @@ __metadata: express: ^4.17.1 fork-ts-checker-webpack-plugin: ^7.0.0-alpha.8 fs-extra: 10.1.0 + get-port: ^6.1.2 glob: ^7.1.7 global-agent: ^3.0.0 handlebars: ^4.7.3 From 4fc7bff6a7255eeaeec3c28573d515353812389d Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Wed, 2 Nov 2022 17:22:22 -0400 Subject: [PATCH 12/42] fix prettier Signed-off-by: Aramis Sennyey --- packages/cli/src/__fixtures__/test-project/app-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/__fixtures__/test-project/app-config.yaml b/packages/cli/src/__fixtures__/test-project/app-config.yaml index c4d4705aa5..69bb98f0c7 100644 --- a/packages/cli/src/__fixtures__/test-project/app-config.yaml +++ b/packages/cli/src/__fixtures__/test-project/app-config.yaml @@ -3,4 +3,4 @@ app: title: test backend: - baseUrl: http://localhost:${BACKEND_PORT} \ No newline at end of file + baseUrl: http://localhost:${BACKEND_PORT} From 8e88108346970547b3a6d65c8728a057edf2a3e2 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Wed, 2 Nov 2022 17:53:59 -0400 Subject: [PATCH 13/42] update test case. Signed-off-by: Aramis Sennyey --- packages/cli/src/serve.test.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/serve.test.ts b/packages/cli/src/serve.test.ts index f43ea8053d..32a94370bd 100644 --- a/packages/cli/src/serve.test.ts +++ b/packages/cli/src/serve.test.ts @@ -20,6 +20,8 @@ import fetch from 'node-fetch'; import path from 'path'; import getPort from 'get-port'; +jest.setTimeout(150000); + const executeCommand = ( command: string, args: string[], @@ -100,11 +102,6 @@ const executeCommand = ( }); }; -const timeout = 100000; - -// Builds initially (with no cache) take a loooong time. -jest.setTimeout(timeout * 2); - const testProjectDir = path.resolve( __dirname, '__fixtures__/test-project/packages/app', @@ -133,7 +130,7 @@ describe('end-to-end', () => { const frontendPort = await getPort(); startEmitter.on('hit', async () => { const response = await fetch( - `http://localhost:3000/${frontendPort}/catalog`, + `http://localhost:${frontendPort}/test/catalog`, ); const text = await response.text(); startEmitter.emit('stop', 'SIGINT'); @@ -153,7 +150,8 @@ describe('end-to-end', () => { }, startEmitter, { - signalRegex: [/webpack compiled/], + // Need to match console colors as well, so use .* instead of just ' '. + signalRegex: [/compiled.*successfully/], }, ); From 39a9b3afb00c8a43c499ca767713282cb624cc5b Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Wed, 2 Nov 2022 18:00:26 -0400 Subject: [PATCH 14/42] Update changesets. Signed-off-by: Aramis Sennyey --- .changeset/spotty-numbers-occur.md | 6 +++--- .changeset/young-apples-hide.md | 5 ----- 2 files changed, 3 insertions(+), 8 deletions(-) delete mode 100644 .changeset/young-apples-hide.md diff --git a/.changeset/spotty-numbers-occur.md b/.changeset/spotty-numbers-occur.md index 1b69516617..dee57f8850 100644 --- a/.changeset/spotty-numbers-occur.md +++ b/.changeset/spotty-numbers-occur.md @@ -4,8 +4,8 @@ Adds support for two new CLI options on the `repo build` and `package build` commands. Both options are only available to frontend components. Backend config continues to be supplied at runtime. -CLI options take precedent over any provided config. +CLI options take precedent over provided config. The new options are meant to add greater support for running the same bundle in multiple environments, ex: test, stage and prod. -`--publicPath`: Frontend subroute to host the app on, can be relative or absolute. -`--backendUrl`: Backend url that the {backend.basePath}, e.g. /api, endpoint can be reached on. Can be relative or absolute. +`--publicPath`: Frontend route to host the app on, can be relative or absolute. +`--backendUrl`: Backend URL that the {backend.basePath}, e.g. /api, endpoint can be reached on. Can be relative or absolute. diff --git a/.changeset/young-apples-hide.md b/.changeset/young-apples-hide.md deleted file mode 100644 index c445c45ead..0000000000 --- a/.changeset/young-apples-hide.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@backstage/config-loader': minor ---- - -Added new CLI parsing that overrides all other config options, like env and yaml. Currently supports --publicPath and --backendUrl. From 2157686421048be683fca8f5296487183a9495ed Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Wed, 2 Nov 2022 18:52:40 -0400 Subject: [PATCH 15/42] Update API report. Signed-off-by: Aramis Sennyey --- packages/cli/cli-report.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/cli/cli-report.md b/packages/cli/cli-report.md index d82f8da60e..9d8756058a 100644 --- a/packages/cli/cli-report.md +++ b/packages/cli/cli-report.md @@ -213,6 +213,8 @@ Options: --experimental-type-build --skip-build-dependencies --stats + --public-path + --backend-url --config -h, --help ``` @@ -404,6 +406,8 @@ Usage: backstage-cli repo build [options] Options: --all + --public-path + --backend-url --since -h, --help ``` From d9e692410a848ea18688f0f7c923f80cb3369381 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Fri, 4 Nov 2022 11:21:13 -0400 Subject: [PATCH 16/42] First round of PR updates, remove extra fixtures fluff and update test case. Signed-off-by: Aramis Sennyey --- .changeset/spotty-numbers-occur.md | 2 +- packages/cli/package.json | 8 +-- .../__fixtures__/test-project/backstage.json | 4 +- .../__fixtures__/test-project/package.json | 39 +--------- .../test-project/packages/app/package.json | 72 +------------------ .../packages/app/public/index.html | 68 ------------------ .../test-project/packages/app/src/index.tsx | 8 +-- .../packages/backend/package.json | 51 ------------- .../packages/backend/src/index.ts | 16 ----- packages/cli/src/serve.test.ts | 3 + 10 files changed, 14 insertions(+), 257 deletions(-) delete mode 100644 packages/cli/src/__fixtures__/test-project/packages/backend/package.json delete mode 100644 packages/cli/src/__fixtures__/test-project/packages/backend/src/index.ts diff --git a/.changeset/spotty-numbers-occur.md b/.changeset/spotty-numbers-occur.md index dee57f8850..fff27a18d3 100644 --- a/.changeset/spotty-numbers-occur.md +++ b/.changeset/spotty-numbers-occur.md @@ -1,5 +1,5 @@ --- -'@backstage/cli': minor +'@backstage/cli': patch --- Adds support for two new CLI options on the `repo build` and `package build` commands. Both options are only available to frontend components. Backend config continues to be supplied at runtime. diff --git a/packages/cli/package.json b/packages/cli/package.json index 0fcd8cd89f..fd8b852558 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -24,9 +24,7 @@ "lint": "backstage-cli package lint", "test": "backstage-cli package test", "clean": "backstage-cli package clean", - "start": "nodemon --", - "test:e2e": "backstage-cli test --config e2e-test.config.js", - "test:e2e:ci": "backstage-cli test --config e2e-test.config.js --watchAll=false --ci" + "start": "nodemon --" }, "bin": { "backstage-cli": "bin/backstage-cli" @@ -86,7 +84,6 @@ "express": "^4.17.1", "fork-ts-checker-webpack-plugin": "^7.0.0-alpha.8", "fs-extra": "10.1.0", - "get-port": "^6.1.2", "glob": "^7.1.7", "global-agent": "^3.0.0", "handlebars": "^4.7.3", @@ -157,6 +154,7 @@ "@types/terser-webpack-plugin": "^5.0.4", "@types/yarnpkg__lockfile": "^1.1.4", "del": "^6.0.0", + "get-port": "^6.1.2", "mock-fs": "^5.1.0", "msw": "^0.49.0", "nodemon": "^2.0.2", @@ -275,4 +273,4 @@ } } } -} +} \ No newline at end of file diff --git a/packages/cli/src/__fixtures__/test-project/backstage.json b/packages/cli/src/__fixtures__/test-project/backstage.json index 3e73a3c875..688e939808 100644 --- a/packages/cli/src/__fixtures__/test-project/backstage.json +++ b/packages/cli/src/__fixtures__/test-project/backstage.json @@ -1,3 +1,3 @@ { - "version": "1.7.0-next.0" -} + "version": "1.0.0" +} \ No newline at end of file diff --git a/packages/cli/src/__fixtures__/test-project/package.json b/packages/cli/src/__fixtures__/test-project/package.json index 7ebf258618..07f754b9c9 100644 --- a/packages/cli/src/__fixtures__/test-project/package.json +++ b/packages/cli/src/__fixtures__/test-project/package.json @@ -5,50 +5,15 @@ "engines": { "node": "14 || 16" }, - "scripts": { - "dev": "concurrently \"yarn start\" \"yarn start-backend\"", - "start": "yarn workspace app start", - "start-backend": "yarn workspace backend start", - "build": "backstage-cli repo build --all", - "build-image": "yarn workspace backend build-image", - "tsc": "tsc", - "tsc:full": "tsc --skipLibCheck false --incremental false", - "clean": "backstage-cli repo clean", - "test": "backstage-cli repo test", - "test:all": "backstage-cli repo test --coverage", - "lint": "backstage-cli repo lint --since origin/master", - "lint:all": "backstage-cli repo lint", - "prettier:check": "prettier --check .", - "create-plugin": "backstage-cli create-plugin --scope internal", - "new": "backstage-cli new --scope internal" - }, "workspaces": { "packages": [ - "packages/*", - "plugins/*" + "packages/*" ] }, "devDependencies": { "@backstage/cli": "^0.20.0-next.0", - "@spotify/prettier-config": "^12.0.0", - "concurrently": "^6.0.0", "lerna": "^4.0.0", "node-gyp": "^9.0.0", - "prettier": "^2.3.2", "typescript": "~4.6.4" - }, - "resolutions": { - "@types/react": "^17", - "@types/react-dom": "^17" - }, - "prettier": "@spotify/prettier-config", - "lint-staged": { - "*.{js,jsx,ts,tsx,mjs,cjs}": [ - "eslint --fix", - "prettier --write" - ], - "*.{json,md}": [ - "prettier --write" - ] } -} +} \ No newline at end of file diff --git a/packages/cli/src/__fixtures__/test-project/packages/app/package.json b/packages/cli/src/__fixtures__/test-project/packages/app/package.json index 1157a698f5..91acce03ef 100644 --- a/packages/cli/src/__fixtures__/test-project/packages/app/package.json +++ b/packages/cli/src/__fixtures__/test-project/packages/app/package.json @@ -6,77 +6,7 @@ "backstage": { "role": "frontend" }, - "scripts": { - "start": "backstage-cli package start", - "build": "backstage-cli package build", - "clean": "backstage-cli package clean", - "test": "backstage-cli package test", - "lint": "backstage-cli package lint", - "test:e2e": "cross-env PORT=3001 start-server-and-test start http://localhost:3001 cy:dev", - "test:e2e:ci": "cross-env PORT=3001 start-server-and-test start http://localhost:3001 cy:run", - "cy:dev": "cypress open", - "cy:run": "cypress run --browser chrome" - }, - "dependencies": { - "@backstage/app-defaults": "^1.0.7-next.0", - "@backstage/catalog-model": "^1.1.2-next.0", - "@backstage/cli": "^0.20.0-next.0", - "@backstage/core-app-api": "^1.1.1-next.0", - "@backstage/core-components": "^0.11.2-next.0", - "@backstage/core-plugin-api": "^1.0.7-next.0", - "@backstage/integration-react": "^1.1.5-next.0", - "@backstage/plugin-api-docs": "^0.8.10-next.0", - "@backstage/plugin-catalog": "^1.5.2-next.0", - "@backstage/plugin-catalog-common": "^1.0.7-next.0", - "@backstage/plugin-catalog-graph": "^0.2.22-next.0", - "@backstage/plugin-catalog-import": "^0.8.13-next.0", - "@backstage/plugin-catalog-react": "^1.1.5-next.0", - "@backstage/plugin-github-actions": "^0.5.10-next.0", - "@backstage/plugin-org": "^0.5.10-next.0", - "@backstage/plugin-permission-react": "^0.4.6-next.0", - "@backstage/plugin-scaffolder": "^1.7.0-next.0", - "@backstage/plugin-search": "^1.0.3-next.0", - "@backstage/plugin-search-react": "^1.1.1-next.0", - "@backstage/plugin-tech-radar": "^0.5.17-next.0", - "@backstage/plugin-techdocs": "^1.3.3-next.0", - "@backstage/plugin-techdocs-module-addons-contrib": "^1.0.5-next.0", - "@backstage/plugin-techdocs-react": "^1.0.5-next.0", - "@backstage/plugin-user-settings": "^0.5.0-next.0", - "@backstage/theme": "^0.2.16", - "@material-ui/core": "^4.12.2", - "@material-ui/icons": "^4.9.1", - "history": "^5.0.0", - "react": "^17.0.2", - "react-dom": "^17.0.2", - "react-router": "^6.3.0", - "react-router-dom": "^6.3.0", - "react-use": "^17.2.4" - }, - "devDependencies": { - "@backstage/test-utils": "^1.2.1-next.0", - "@testing-library/jest-dom": "^5.10.1", - "@testing-library/react": "^12.1.3", - "@testing-library/user-event": "^14.0.0", - "@types/node": "^16.11.26", - "@types/react-dom": "*", - "cross-env": "^7.0.0", - "cypress": "^9.7.0", - "eslint-plugin-cypress": "^2.10.3", - "start-server-and-test": "^1.10.11" - }, - "browserslist": { - "production": [ - ">0.2%", - "not dead", - "not op_mini all" - ], - "development": [ - "last 1 chrome version", - "last 1 firefox version", - "last 1 safari version" - ] - }, "files": [ "dist" ] -} +} \ No newline at end of file diff --git a/packages/cli/src/__fixtures__/test-project/packages/app/public/index.html b/packages/cli/src/__fixtures__/test-project/packages/app/public/index.html index a936c73602..93da08ba81 100644 --- a/packages/cli/src/__fixtures__/test-project/packages/app/public/index.html +++ b/packages/cli/src/__fixtures__/test-project/packages/app/public/index.html @@ -4,76 +4,8 @@ - - - - - - - - - - - <%= config.getString('app.title') %> - <% if (config.has('app.googleAnalyticsTrackingId')) { %> - - - <% } %> -
- diff --git a/packages/cli/src/__fixtures__/test-project/packages/app/src/index.tsx b/packages/cli/src/__fixtures__/test-project/packages/app/src/index.tsx index 3c96b4476d..93500d0086 100644 --- a/packages/cli/src/__fixtures__/test-project/packages/app/src/index.tsx +++ b/packages/cli/src/__fixtures__/test-project/packages/app/src/index.tsx @@ -13,9 +13,5 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -import '@backstage/cli/asset-types'; -import React from 'react'; -import ReactDOM from 'react-dom'; - -ReactDOM.render(
, document.getElementById('root')); +export {} +console.log('Hello World.') \ No newline at end of file diff --git a/packages/cli/src/__fixtures__/test-project/packages/backend/package.json b/packages/cli/src/__fixtures__/test-project/packages/backend/package.json deleted file mode 100644 index 5c649f08b6..0000000000 --- a/packages/cli/src/__fixtures__/test-project/packages/backend/package.json +++ /dev/null @@ -1,51 +0,0 @@ -{ - "name": "backend", - "version": "0.0.0", - "main": "dist/index.cjs.js", - "types": "src/index.ts", - "private": true, - "backstage": { - "role": "backend" - }, - "scripts": { - "start": "backstage-cli package start", - "build": "backstage-cli package build", - "lint": "backstage-cli package lint", - "test": "backstage-cli package test", - "clean": "backstage-cli package clean" - }, - "dependencies": { - "@backstage/backend-common": "^0.15.0", - "@backstage/backend-tasks": "^0.3.4", - "@backstage/catalog-client": "^1.0.4", - "@backstage/catalog-model": "^1.1.0", - "@backstage/config": "^1.0.1", - "@backstage/plugin-app-backend": "^0.3.35", - "@backstage/plugin-auth-backend": "^0.15.1", - "@backstage/plugin-catalog-backend": "^1.3.1", - "@backstage/plugin-permission-common": "^0.6.3", - "@backstage/plugin-permission-node": "^0.6.4", - "@backstage/plugin-proxy-backend": "^0.2.29", - "@backstage/plugin-scaffolder-backend": "^1.5.0", - "@backstage/plugin-search-backend": "^1.0.1", - "@backstage/plugin-search-backend-module-pg": "^0.3.6", - "@backstage/plugin-search-backend-node": "^1.0.1", - "@backstage/plugin-techdocs-backend": "^1.2.1", - "dockerode": "^3.3.1", - "express": "^4.17.1", - "express-promise-router": "^4.1.0", - "pg": "^8.3.0", - "winston": "^3.2.1" - }, - "devDependencies": { - "@backstage/cli": "^0.18.1", - "@types/dockerode": "^3.3.0", - "@types/express": "^4.17.6", - "@types/express-serve-static-core": "^4.17.5", - "@types/luxon": "^2.0.4", - "better-sqlite3": "^7.5.0" - }, - "files": [ - "dist" - ] -} diff --git a/packages/cli/src/__fixtures__/test-project/packages/backend/src/index.ts b/packages/cli/src/__fixtures__/test-project/packages/backend/src/index.ts deleted file mode 100644 index 8b9b6bd586..0000000000 --- a/packages/cli/src/__fixtures__/test-project/packages/backend/src/index.ts +++ /dev/null @@ -1,16 +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. - */ -export {}; diff --git a/packages/cli/src/serve.test.ts b/packages/cli/src/serve.test.ts index 32a94370bd..7256e7c4ad 100644 --- a/packages/cli/src/serve.test.ts +++ b/packages/cli/src/serve.test.ts @@ -126,6 +126,8 @@ describe('end-to-end', () => { }); it('starts frontend on correct url', async () => { + expect.assertions(4); + const startEmitter = new EventEmitter(); const frontendPort = await getPort(); startEmitter.on('hit', async () => { @@ -136,6 +138,7 @@ describe('end-to-end', () => { startEmitter.emit('stop', 'SIGINT'); expect(response.status).toBe(200); expect(text.length).toBeGreaterThan(0); + expect(text).toContain('id="root"'); }); const startProc = await executeCommand( entryPoint, From fce073f658a1d122e8f2f661ee999319a7b7ba66 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Fri, 4 Nov 2022 15:06:43 -0400 Subject: [PATCH 17/42] Update AppManager and test case to handle parsing backend.baseUrl as relative. Signed-off-by: Aramis Sennyey --- packages/app-defaults/src/defaults/apis.ts | 14 +-- .../core-app-api/src/app/AppManager.test.tsx | 90 +++++++++++++++++++ packages/core-app-api/src/app/AppManager.tsx | 29 +++++- 3 files changed, 121 insertions(+), 12 deletions(-) diff --git a/packages/app-defaults/src/defaults/apis.ts b/packages/app-defaults/src/defaults/apis.ts index c53ce63ada..7ff8ce16bc 100644 --- a/packages/app-defaults/src/defaults/apis.ts +++ b/packages/app-defaults/src/defaults/apis.ts @@ -65,17 +65,9 @@ export const apis = [ api: discoveryApiRef, deps: { configApi: configApiRef }, factory: ({ configApi }) => { - let baseUrl; - try { - // Try parsing the url relative to the current document origin, if it fails the URL is misformed. - baseUrl = new URL( - configApi.getString('backend.baseUrl'), - document.location.origin, - ).href.replace(/\/$/, ''); - } catch (err) { - baseUrl = configApi.getString('backend.baseUrl'); - } - return UrlPatternDiscovery.compile(`${baseUrl}/api/{{ pluginId }}`); + return UrlPatternDiscovery.compile( + `${configApi.getString('backend.baseUrl')}/api/{{ pluginId }}`, + ); }, }), createApiFactory({ diff --git a/packages/core-app-api/src/app/AppManager.test.tsx b/packages/core-app-api/src/app/AppManager.test.tsx index c70b43581a..33b7ad5b9a 100644 --- a/packages/core-app-api/src/app/AppManager.test.tsx +++ b/packages/core-app-api/src/app/AppManager.test.tsx @@ -34,6 +34,7 @@ import { createSubRouteRef, createRoutableExtension, analyticsApiRef, + useApiHolder, } from '@backstage/core-plugin-api'; import { AppManager } from './AppManager'; import { AppComponents, AppIcons } from './types'; @@ -569,4 +570,93 @@ describe('Integration Test', () => { ), ]); }); + + it('should add the fully qualified origin when the backend.baseUrl is relative', async () => { + const app = new AppManager({ + apis: [], + defaultApis: [], + themes: [ + { + id: 'light', + title: 'Light Theme', + variant: 'light', + Provider: ({ children }) => <>{children}, + }, + ], + icons, + plugins: [], + components, + configLoader: async () => [ + { + context: 'test', + data: { + backend: { + baseUrl: '/', + }, + }, + }, + ], + }); + + const Provider = app.getProvider(); + const ConfigDisplay = () => { + const apiHolder = useApiHolder(); + const config = apiHolder.get(configApiRef); + return {config?.getString('backend.baseUrl')}; + }; + + const dom = await renderWithEffects( + + + , + ); + + // Verify that the backend.baseUrl is set correctly by the AppManager. + expect(dom.getByText(document.location.origin)).toBeTruthy(); + }); + + it('should NOT change the origin when the backend.baseUrl is absolute', async () => { + const backendUrl = 'http://localhost:7007'; + const app = new AppManager({ + apis: [], + defaultApis: [], + themes: [ + { + id: 'light', + title: 'Light Theme', + variant: 'light', + Provider: ({ children }) => <>{children}, + }, + ], + icons, + plugins: [], + components, + configLoader: async () => [ + { + context: 'test', + data: { + backend: { + baseUrl: backendUrl, + }, + }, + }, + ], + }); + + const Provider = app.getProvider(); + const ConfigDisplay = () => { + const apiHolder = useApiHolder(); + const config = apiHolder.get(configApiRef); + return {config?.getString('backend.baseUrl')}; + }; + + const dom = await renderWithEffects( + + + , + ); + + // Verify that the backend.baseUrl is set correctly by the AppManager. + expect(dom.getByText(backendUrl)).toBeTruthy(); + }); }); diff --git a/packages/core-app-api/src/app/AppManager.tsx b/packages/core-app-api/src/app/AppManager.tsx index bbd27f84ce..cd571c7799 100644 --- a/packages/core-app-api/src/app/AppManager.tsx +++ b/packages/core-app-api/src/app/AppManager.tsx @@ -154,7 +154,34 @@ function useConfigLoader( }; } - const configReader = ConfigReader.fromConfigs(config.value ?? []); + let configReader = ConfigReader.fromConfigs(config.value ?? []); + + const getFullBackendUrl = () => { + // Backend.baseUrl should always be defined. + let url; + try { + url = new URL( + configReader.getString('backend.baseUrl'), + document.location.origin, + ).href.replace(/\/$/, ''); + } catch (err) { + // Backend.baseUrl was not a valid URL. This should be caught during the build process. + } + return url; + }; + + const relativeBackendConfig = { + data: { + backend: { + baseUrl: getFullBackendUrl(), + }, + }, + context: 'relative-override', + }; + + configReader = ConfigReader.fromConfigs( + config.value ? [...config.value, relativeBackendConfig] : [], + ); return { api: configReader }; } From db52cb945ec1446ef03e5adf50784b84dc2fabcb Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Fri, 4 Nov 2022 15:51:54 -0400 Subject: [PATCH 18/42] Prettier fixes. Signed-off-by: Aramis Sennyey --- packages/cli/package.json | 2 +- packages/cli/src/__fixtures__/test-project/backstage.json | 2 +- packages/cli/src/__fixtures__/test-project/package.json | 2 +- .../src/__fixtures__/test-project/packages/app/package.json | 2 +- .../src/__fixtures__/test-project/packages/app/src/index.tsx | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/cli/package.json b/packages/cli/package.json index fd8b852558..41b37fc7b0 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -273,4 +273,4 @@ } } } -} \ No newline at end of file +} diff --git a/packages/cli/src/__fixtures__/test-project/backstage.json b/packages/cli/src/__fixtures__/test-project/backstage.json index 688e939808..1587a66968 100644 --- a/packages/cli/src/__fixtures__/test-project/backstage.json +++ b/packages/cli/src/__fixtures__/test-project/backstage.json @@ -1,3 +1,3 @@ { "version": "1.0.0" -} \ No newline at end of file +} diff --git a/packages/cli/src/__fixtures__/test-project/package.json b/packages/cli/src/__fixtures__/test-project/package.json index 07f754b9c9..e1f6b310f3 100644 --- a/packages/cli/src/__fixtures__/test-project/package.json +++ b/packages/cli/src/__fixtures__/test-project/package.json @@ -16,4 +16,4 @@ "node-gyp": "^9.0.0", "typescript": "~4.6.4" } -} \ No newline at end of file +} diff --git a/packages/cli/src/__fixtures__/test-project/packages/app/package.json b/packages/cli/src/__fixtures__/test-project/packages/app/package.json index 91acce03ef..95e50a6774 100644 --- a/packages/cli/src/__fixtures__/test-project/packages/app/package.json +++ b/packages/cli/src/__fixtures__/test-project/packages/app/package.json @@ -9,4 +9,4 @@ "files": [ "dist" ] -} \ No newline at end of file +} diff --git a/packages/cli/src/__fixtures__/test-project/packages/app/src/index.tsx b/packages/cli/src/__fixtures__/test-project/packages/app/src/index.tsx index 93500d0086..03a9b39841 100644 --- a/packages/cli/src/__fixtures__/test-project/packages/app/src/index.tsx +++ b/packages/cli/src/__fixtures__/test-project/packages/app/src/index.tsx @@ -13,5 +13,5 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -export {} -console.log('Hello World.') \ No newline at end of file +export {}; +console.log('Hello World.'); From 477a3bf8ef5e0fe23d3b5b0d57351c466e8f41c6 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Fri, 4 Nov 2022 16:49:09 -0400 Subject: [PATCH 19/42] Update changeset Signed-off-by: Aramis Sennyey --- .changeset/metal-fishes-grow.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/metal-fishes-grow.md diff --git a/.changeset/metal-fishes-grow.md b/.changeset/metal-fishes-grow.md new file mode 100644 index 0000000000..80b97c73a1 --- /dev/null +++ b/.changeset/metal-fishes-grow.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-app-api': patch +--- + +Updated AppManager to return an absolute url for backend.baseUrl when a relative url is provided. From 833091d0133edf75a175e9af0dbcaf5b8992d41e Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Fri, 4 Nov 2022 16:51:55 -0400 Subject: [PATCH 20/42] Vale fix. Signed-off-by: Aramis Sennyey --- .changeset/metal-fishes-grow.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/metal-fishes-grow.md b/.changeset/metal-fishes-grow.md index 80b97c73a1..01ac5119d4 100644 --- a/.changeset/metal-fishes-grow.md +++ b/.changeset/metal-fishes-grow.md @@ -2,4 +2,4 @@ '@backstage/core-app-api': patch --- -Updated AppManager to return an absolute url for backend.baseUrl when a relative url is provided. +Updated AppManager to return an absolute URL for `backend.baseUrl` when a relative URL is provided. From 2682ebe7d21a1750e8ccb3b51524a876673d58a6 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Mon, 14 Nov 2022 12:12:04 -0500 Subject: [PATCH 21/42] CR updates, update CLI to be more forceful in rejecting weird relative urls Signed-off-by: Aramis Sennyey --- .changeset/metal-fishes-grow.md | 2 +- packages/app-defaults/src/defaults/apis.ts | 7 ++-- packages/cli/src/commands/index.ts | 8 ----- packages/cli/src/lib/config.test.ts | 36 +++++++++++++++----- packages/cli/src/lib/config.ts | 13 +++++-- packages/core-app-api/src/app/AppManager.tsx | 9 +++-- 6 files changed, 48 insertions(+), 27 deletions(-) diff --git a/.changeset/metal-fishes-grow.md b/.changeset/metal-fishes-grow.md index 01ac5119d4..7067a881bb 100644 --- a/.changeset/metal-fishes-grow.md +++ b/.changeset/metal-fishes-grow.md @@ -2,4 +2,4 @@ '@backstage/core-app-api': patch --- -Updated AppManager to return an absolute URL for `backend.baseUrl` when a relative URL is provided. +Apps will now detect when a relative `backend.baseUrl` is provided and update the config accordingly. diff --git a/packages/app-defaults/src/defaults/apis.ts b/packages/app-defaults/src/defaults/apis.ts index 7ff8ce16bc..3f5cfc1c58 100644 --- a/packages/app-defaults/src/defaults/apis.ts +++ b/packages/app-defaults/src/defaults/apis.ts @@ -64,11 +64,10 @@ export const apis = [ createApiFactory({ api: discoveryApiRef, deps: { configApi: configApiRef }, - factory: ({ configApi }) => { - return UrlPatternDiscovery.compile( + factory: ({ configApi }) => + UrlPatternDiscovery.compile( `${configApi.getString('backend.baseUrl')}/api/{{ pluginId }}`, - ); - }, + ), }), createApiFactory({ api: alertApiRef, diff --git a/packages/cli/src/commands/index.ts b/packages/cli/src/commands/index.ts index 90d3afd4d5..2dd2973de1 100644 --- a/packages/cli/src/commands/index.ts +++ b/packages/cli/src/commands/index.ts @@ -39,14 +39,6 @@ export function registerRepoCommand(program: Command) { '--all', 'Build all packages, including bundled app and backend packages.', ) - .option( - '--public-path ', - 'Public path for hosting the website on, can be relative.', - ) - .option( - '--backend-url ', - 'Backend url, expects just the origin or sub-route. Do not include /api. Can be relative.', - ) .option( '--since ', 'Only build packages and their dev dependents that changed since the specified ref', diff --git a/packages/cli/src/lib/config.test.ts b/packages/cli/src/lib/config.test.ts index bc6d8b6931..9db076e8e4 100644 --- a/packages/cli/src/lib/config.test.ts +++ b/packages/cli/src/lib/config.test.ts @@ -32,13 +32,13 @@ describe('readCliConfig', () => { it('should return backend.baseUrl when backendUrl present in cli options', () => { expect( readCliConfig({ - backendUrl: 'http://localhost:3000', + backendUrl: '/', }), ).toEqual([ { data: { backend: { - baseUrl: 'http://localhost:3000', + baseUrl: '/', }, }, context: 'cli', @@ -49,13 +49,13 @@ describe('readCliConfig', () => { it('should return app.baseUrl when publicPath present in cli options', () => { expect( readCliConfig({ - publicPath: 'http://localhost:3000', + publicPath: '/', }), ).toEqual([ { data: { app: { - baseUrl: 'http://localhost:3000', + baseUrl: '/', }, }, context: 'cli', @@ -66,21 +66,41 @@ describe('readCliConfig', () => { it('should return app.baseUrl and backend.baseUrl when publicPath and backendUrl present in cli options', () => { expect( readCliConfig({ - publicPath: 'http://localhost:3000', - backendUrl: 'http://localhost:3000/api', + publicPath: '/', + backendUrl: '/api', }), ).toEqual([ { data: { app: { - baseUrl: 'http://localhost:3000', + baseUrl: '/', }, backend: { - baseUrl: 'http://localhost:3000/api', + baseUrl: '/api', }, }, context: 'cli', }, ]); }); + + it('should throw for public paths that do NOT start with /', () => { + ['http://localhost:3000', './', '../..'].forEach(publicPath => + expect(() => { + readCliConfig({ + publicPath, + }); + }).toThrow('Public path must be relative'), + ); + }); + + it('should throw for backend urls that do NOT start with /', () => { + ['http://localhost:3000', './', '../..'].forEach(backendUrl => + expect(() => { + readCliConfig({ + backendUrl, + }); + }).toThrow('Backend URL must be relative'), + ); + }); }); diff --git a/packages/cli/src/lib/config.ts b/packages/cli/src/lib/config.ts index 1220586ce0..9084f72057 100644 --- a/packages/cli/src/lib/config.ts +++ b/packages/cli/src/lib/config.ts @@ -52,16 +52,23 @@ export function readCliConfig(opts?: CliConfigOptions): AppConfig[] { if (!opts || Object.keys(opts).length === 0) return []; const data: JsonObject = {}; - if (opts.publicPath) { + if (opts.publicPath?.startsWith('/')) { data.app = { baseUrl: opts.publicPath, }; + } else if (opts.publicPath) { + throw new Error( + 'Public path must be relative and start with "/" when specified through CLI options. Path traversals like "./" and assumed relative endpoints like "backstage" are not supported.', + ); } - - if (opts.backendUrl) { + if (opts.backendUrl?.startsWith('/')) { data.backend = { baseUrl: opts.backendUrl, }; + } else if (opts.backendUrl) { + throw new Error( + 'Backend URL must be relative and start with "/" when specified through CLI options. Path traversals like "./" and assumed relative endpoint like "api" are not supported.', + ); } if (Object.keys(data).length === 0) return []; diff --git a/packages/core-app-api/src/app/AppManager.tsx b/packages/core-app-api/src/app/AppManager.tsx index cd571c7799..176171e975 100644 --- a/packages/core-app-api/src/app/AppManager.tsx +++ b/packages/core-app-api/src/app/AppManager.tsx @@ -179,9 +179,12 @@ function useConfigLoader( context: 'relative-override', }; - configReader = ConfigReader.fromConfigs( - config.value ? [...config.value, relativeBackendConfig] : [], - ); + // Config reader may not have backend.baseUrl set. config.value may be undefined. + if (configReader.getOptionalString('backend.baseUrl')?.startsWith('/')) { + config.value?.push(relativeBackendConfig); + } + + configReader = ConfigReader.fromConfigs(config.value ?? []); return { api: configReader }; } From b03b9c04479cac7e0a394c79ae91af9e73bec7da Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Mon, 14 Nov 2022 18:20:50 -0500 Subject: [PATCH 22/42] update api report Signed-off-by: Aramis Sennyey --- packages/cli/cli-report.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/cli/cli-report.md b/packages/cli/cli-report.md index 9d8756058a..dc0232e987 100644 --- a/packages/cli/cli-report.md +++ b/packages/cli/cli-report.md @@ -406,8 +406,6 @@ Usage: backstage-cli repo build [options] Options: --all - --public-path - --backend-url --since -h, --help ``` From 83696693d397caf4fb9c1558a2a17da2d3c6b237 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 22 Nov 2022 14:31:11 -0500 Subject: [PATCH 23/42] Add backend.baseUrl and app.baseUrl to overrides Signed-off-by: Aramis Sennyey --- .../core-app-api/src/app/AppManager.test.tsx | 200 +++++++++++------- packages/core-app-api/src/app/AppManager.tsx | 30 +-- 2 files changed, 134 insertions(+), 96 deletions(-) diff --git a/packages/core-app-api/src/app/AppManager.test.tsx b/packages/core-app-api/src/app/AppManager.test.tsx index 33b7ad5b9a..b8aeb21818 100644 --- a/packages/core-app-api/src/app/AppManager.test.tsx +++ b/packages/core-app-api/src/app/AppManager.test.tsx @@ -571,92 +571,128 @@ describe('Integration Test', () => { ]); }); - it('should add the fully qualified origin when the backend.baseUrl is relative', async () => { - const app = new AppManager({ - apis: [], - defaultApis: [], - themes: [ - { - id: 'light', - title: 'Light Theme', - variant: 'light', - Provider: ({ children }) => <>{children}, - }, - ], - icons, - plugins: [], - components, - configLoader: async () => [ - { - context: 'test', - data: { - backend: { - baseUrl: '/', - }, + describe('relative url resolvers', () => { + const checkConfigValue = async ( + data: object, + configString: string, + expectedValue: string, + ) => { + const app = new AppManager({ + apis: [], + defaultApis: [], + themes: [ + { + id: 'light', + title: 'Light Theme', + variant: 'light', + Provider: ({ children }) => <>{children}, + }, + ], + icons, + plugins: [], + components, + configLoader: async () => [ + { + context: 'test', + data, + }, + ], + }); + + const Provider = app.getProvider(); + const ConfigDisplay = () => { + const apiHolder = useApiHolder(); + const config = apiHolder.get(configApiRef); + return {config?.getString(configString)}; + }; + + const dom = await renderWithEffects( + + + , + ); + + // Verify that the backend.baseUrl is set correctly by the AppManager. + expect(dom.getByText(expectedValue)).toBeTruthy(); + }; + [ + { + data: { + backend: { + baseUrl: '/', }, }, - ], - }); - - const Provider = app.getProvider(); - const ConfigDisplay = () => { - const apiHolder = useApiHolder(); - const config = apiHolder.get(configApiRef); - return {config?.getString('backend.baseUrl')}; - }; - - const dom = await renderWithEffects( - - - , - ); - - // Verify that the backend.baseUrl is set correctly by the AppManager. - expect(dom.getByText(document.location.origin)).toBeTruthy(); - }); - - it('should NOT change the origin when the backend.baseUrl is absolute', async () => { - const backendUrl = 'http://localhost:7007'; - const app = new AppManager({ - apis: [], - defaultApis: [], - themes: [ - { - id: 'light', - title: 'Light Theme', - variant: 'light', - Provider: ({ children }) => <>{children}, - }, - ], - icons, - plugins: [], - components, - configLoader: async () => [ - { - context: 'test', - data: { - backend: { - baseUrl: backendUrl, - }, + paths: ['backend.baseUrl'], + }, + { + data: { + app: { + baseUrl: '/', }, }, - ], + paths: ['app.baseUrl'], + }, + { + data: { + backend: { + baseUrl: '/', + }, + app: { + baseUrl: '/', + }, + }, + paths: ['app.baseUrl', 'backend.baseUrl'], + }, + ].forEach(item => { + item.paths.forEach(path => { + it('should add the fully qualified origin when the relevant urls are relative', async () => { + await checkConfigValue(item.data, path, 'http://localhost'); + }); + }); + }); + [ + { + data: { + backend: { + baseUrl: 'https://google.com', + }, + }, + paths: ['backend.baseUrl'], + }, + { + data: { + app: { + baseUrl: 'https://bing.com', + }, + }, + paths: ['app.baseUrl'], + }, + { + data: { + backend: { + baseUrl: 'https://google.com', + }, + app: { + baseUrl: 'https://bing.com', + }, + }, + paths: ['app.baseUrl', 'backend.baseUrl'], + }, + ].forEach(item => { + item.paths.forEach(path => { + it('should NOT change the origin when the relevant urls are absolute', async () => { + await checkConfigValue( + item.data, + path, + path + .split('.') + .reduce( + (o, i) => o[i] as { [key: string]: object }, + item.data as { [key: string]: object }, + ) as unknown as string, + ); + }); + }); }); - - const Provider = app.getProvider(); - const ConfigDisplay = () => { - const apiHolder = useApiHolder(); - const config = apiHolder.get(configApiRef); - return {config?.getString('backend.baseUrl')}; - }; - - const dom = await renderWithEffects( - - - , - ); - - // Verify that the backend.baseUrl is set correctly by the AppManager. - expect(dom.getByText(backendUrl)).toBeTruthy(); }); }); diff --git a/packages/core-app-api/src/app/AppManager.tsx b/packages/core-app-api/src/app/AppManager.tsx index 176171e975..abf4d44318 100644 --- a/packages/core-app-api/src/app/AppManager.tsx +++ b/packages/core-app-api/src/app/AppManager.tsx @@ -156,33 +156,35 @@ function useConfigLoader( let configReader = ConfigReader.fromConfigs(config.value ?? []); - const getFullBackendUrl = () => { + const resolveRelativeUrl = (relativeUrl: string) => { // Backend.baseUrl should always be defined. let url; try { - url = new URL( - configReader.getString('backend.baseUrl'), - document.location.origin, - ).href.replace(/\/$/, ''); + url = new URL(relativeUrl, document.location.origin).href.replace( + /\/$/, + '', + ); } catch (err) { // Backend.baseUrl was not a valid URL. This should be caught during the build process. } return url; }; - const relativeBackendConfig = { + config.value?.push({ data: { + app: { + baseUrl: resolveRelativeUrl( + configReader.getOptionalString('app.baseUrl') || '/', + ), + }, backend: { - baseUrl: getFullBackendUrl(), + baseUrl: resolveRelativeUrl( + configReader.getOptionalString('backend.baseUrl') || '/', + ), }, }, - context: 'relative-override', - }; - - // Config reader may not have backend.baseUrl set. config.value may be undefined. - if (configReader.getOptionalString('backend.baseUrl')?.startsWith('/')) { - config.value?.push(relativeBackendConfig); - } + context: 'relative-resolver', + }); configReader = ConfigReader.fromConfigs(config.value ?? []); From 63310e3987af133a8d4c248f5ea46adf03dbfcb7 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 22 Nov 2022 14:42:57 -0500 Subject: [PATCH 24/42] Remove CLI-related changes. Signed-off-by: Aramis Sennyey --- .changeset/metal-fishes-grow.md | 2 +- .changeset/short-games-march.md | 5 - .changeset/young-chairs-check.md | 5 + packages/cli/package.json | 3 +- .../__fixtures__/test-project/app-config.yaml | 6 - .../__fixtures__/test-project/backstage.json | 3 - .../__fixtures__/test-project/package.json | 19 -- .../test-project/packages/app/package.json | 12 -- .../packages/app/public/index.html | 11 -- .../test-project/packages/app/src/index.tsx | 17 -- .../src/__fixtures__/test-project/yarn.lock | 0 packages/cli/src/commands/build/command.ts | 1 - packages/cli/src/commands/index.ts | 8 - packages/cli/src/commands/repo/build.ts | 1 - packages/cli/src/lib/bundler/config.ts | 6 +- packages/cli/src/lib/config.test.ts | 106 ------------ packages/cli/src/lib/config.ts | 49 +----- packages/cli/src/serve.test.ts | 163 ------------------ 18 files changed, 11 insertions(+), 406 deletions(-) delete mode 100644 .changeset/short-games-march.md create mode 100644 .changeset/young-chairs-check.md delete mode 100644 packages/cli/src/__fixtures__/test-project/app-config.yaml delete mode 100644 packages/cli/src/__fixtures__/test-project/backstage.json delete mode 100644 packages/cli/src/__fixtures__/test-project/package.json delete mode 100644 packages/cli/src/__fixtures__/test-project/packages/app/package.json delete mode 100644 packages/cli/src/__fixtures__/test-project/packages/app/public/index.html delete mode 100644 packages/cli/src/__fixtures__/test-project/packages/app/src/index.tsx delete mode 100644 packages/cli/src/__fixtures__/test-project/yarn.lock delete mode 100644 packages/cli/src/lib/config.test.ts delete mode 100644 packages/cli/src/serve.test.ts diff --git a/.changeset/metal-fishes-grow.md b/.changeset/metal-fishes-grow.md index 7067a881bb..885bcecd1b 100644 --- a/.changeset/metal-fishes-grow.md +++ b/.changeset/metal-fishes-grow.md @@ -2,4 +2,4 @@ '@backstage/core-app-api': patch --- -Apps will now detect when a relative `backend.baseUrl` is provided and update the config accordingly. +Apps will now detect when a relative `backend.baseUrl` or `app.baseUrl` is provided and update the config to produce a full URL. diff --git a/.changeset/short-games-march.md b/.changeset/short-games-march.md deleted file mode 100644 index 54c40b943e..0000000000 --- a/.changeset/short-games-march.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@backstage/app-defaults': minor ---- - -Added support for parsing relative backend urls to the api factory. Allows relative backend urls to be specified in config options. diff --git a/.changeset/young-chairs-check.md b/.changeset/young-chairs-check.md new file mode 100644 index 0000000000..b0611af148 --- /dev/null +++ b/.changeset/young-chairs-check.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-app-api': patch +--- + +Apps will now detect when a relative `backend.baseUrl` or `app.baseUrl` is provided and update it to be a full URL. This means that you can provide relative URLs and they will be resolved as expected across the application. diff --git a/packages/cli/package.json b/packages/cli/package.json index 41b37fc7b0..661d53d897 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -154,7 +154,6 @@ "@types/terser-webpack-plugin": "^5.0.4", "@types/yarnpkg__lockfile": "^1.1.4", "del": "^6.0.0", - "get-port": "^6.1.2", "mock-fs": "^5.1.0", "msw": "^0.49.0", "nodemon": "^2.0.2", @@ -273,4 +272,4 @@ } } } -} +} \ No newline at end of file diff --git a/packages/cli/src/__fixtures__/test-project/app-config.yaml b/packages/cli/src/__fixtures__/test-project/app-config.yaml deleted file mode 100644 index 69bb98f0c7..0000000000 --- a/packages/cli/src/__fixtures__/test-project/app-config.yaml +++ /dev/null @@ -1,6 +0,0 @@ -app: - baseUrl: http://localhost:${PORT}/test - title: test - -backend: - baseUrl: http://localhost:${BACKEND_PORT} diff --git a/packages/cli/src/__fixtures__/test-project/backstage.json b/packages/cli/src/__fixtures__/test-project/backstage.json deleted file mode 100644 index 1587a66968..0000000000 --- a/packages/cli/src/__fixtures__/test-project/backstage.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "version": "1.0.0" -} diff --git a/packages/cli/src/__fixtures__/test-project/package.json b/packages/cli/src/__fixtures__/test-project/package.json deleted file mode 100644 index e1f6b310f3..0000000000 --- a/packages/cli/src/__fixtures__/test-project/package.json +++ /dev/null @@ -1,19 +0,0 @@ -{ - "name": "root", - "version": "1.0.0", - "private": true, - "engines": { - "node": "14 || 16" - }, - "workspaces": { - "packages": [ - "packages/*" - ] - }, - "devDependencies": { - "@backstage/cli": "^0.20.0-next.0", - "lerna": "^4.0.0", - "node-gyp": "^9.0.0", - "typescript": "~4.6.4" - } -} diff --git a/packages/cli/src/__fixtures__/test-project/packages/app/package.json b/packages/cli/src/__fixtures__/test-project/packages/app/package.json deleted file mode 100644 index 95e50a6774..0000000000 --- a/packages/cli/src/__fixtures__/test-project/packages/app/package.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "name": "app", - "version": "0.0.0", - "private": true, - "bundled": true, - "backstage": { - "role": "frontend" - }, - "files": [ - "dist" - ] -} diff --git a/packages/cli/src/__fixtures__/test-project/packages/app/public/index.html b/packages/cli/src/__fixtures__/test-project/packages/app/public/index.html deleted file mode 100644 index 93da08ba81..0000000000 --- a/packages/cli/src/__fixtures__/test-project/packages/app/public/index.html +++ /dev/null @@ -1,11 +0,0 @@ - - - - - - - - -
- - diff --git a/packages/cli/src/__fixtures__/test-project/packages/app/src/index.tsx b/packages/cli/src/__fixtures__/test-project/packages/app/src/index.tsx deleted file mode 100644 index 03a9b39841..0000000000 --- a/packages/cli/src/__fixtures__/test-project/packages/app/src/index.tsx +++ /dev/null @@ -1,17 +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. - */ -export {}; -console.log('Hello World.'); diff --git a/packages/cli/src/__fixtures__/test-project/yarn.lock b/packages/cli/src/__fixtures__/test-project/yarn.lock deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/packages/cli/src/commands/build/command.ts b/packages/cli/src/commands/build/command.ts index b5120f7ae0..fcaf58e1ee 100644 --- a/packages/cli/src/commands/build/command.ts +++ b/packages/cli/src/commands/build/command.ts @@ -29,7 +29,6 @@ export async function command(opts: OptionValues): Promise { targetDir: paths.targetDir, configPaths: opts.config as string[], writeStats: Boolean(opts.stats), - cliOptions: opts, }); } if (role === 'backend') { diff --git a/packages/cli/src/commands/index.ts b/packages/cli/src/commands/index.ts index 2dd2973de1..82cb60e62f 100644 --- a/packages/cli/src/commands/index.ts +++ b/packages/cli/src/commands/index.ts @@ -126,14 +126,6 @@ export function registerScriptCommand(program: Command) { '--stats', 'If bundle stats are available, write them to the output directory. Applies to app packages only.', ) - .option( - '--public-path ', - 'Public path for hosting the website on, can be relative.', - ) - .option( - '--backend-url ', - 'Backend url, expects just the origin or sub-route. Do not include /api. Can be relative.', - ) .option( '--config ', 'Config files to load instead of app-config.yaml. Applies to app packages only.', diff --git a/packages/cli/src/commands/repo/build.ts b/packages/cli/src/commands/repo/build.ts index 55574a53b0..21b1580cb9 100644 --- a/packages/cli/src/commands/repo/build.ts +++ b/packages/cli/src/commands/repo/build.ts @@ -153,7 +153,6 @@ export async function command(opts: OptionValues, cmd: Command): Promise { return; } await buildFrontend({ - cliOptions: opts, targetDir: pkg.dir, configPaths: (buildOptions.config as string[]) ?? [], writeStats: Boolean(buildOptions.stats), diff --git a/packages/cli/src/lib/bundler/config.ts b/packages/cli/src/lib/bundler/config.ts index bc0ff2db3b..ace60883a4 100644 --- a/packages/cli/src/lib/bundler/config.ts +++ b/packages/cli/src/lib/bundler/config.ts @@ -36,12 +36,12 @@ import { runPlain } from '../run'; import ESLintPlugin from 'eslint-webpack-plugin'; import pickBy from 'lodash/pickBy'; -const DUMMY_URL = 'http://dummyurl.org'; +const NO_OP_URL = 'http://test-site-asdf.org'; export function resolveBaseUrl(config: Config): URL { const baseUrl = config.getString('app.baseUrl'); try { - return new URL(baseUrl, DUMMY_URL); + return new URL(baseUrl, NO_OP_URL); } catch (error) { throw new Error(`Invalid app.baseUrl, ${error}`); } @@ -90,7 +90,7 @@ export async function createConfig( const externalPkgs = packages.filter(p => !isChildPath(paths.root, p.dir)); const baseUrl = frontendConfig.getString('app.baseUrl'); - const validBaseUrl = new URL(baseUrl, DUMMY_URL); + const validBaseUrl = new URL(baseUrl, NO_OP_URL); const publicPath = validBaseUrl.pathname.replace(/\/$/, ''); if (checksEnabled) { plugins.push( diff --git a/packages/cli/src/lib/config.test.ts b/packages/cli/src/lib/config.test.ts deleted file mode 100644 index 9db076e8e4..0000000000 --- a/packages/cli/src/lib/config.test.ts +++ /dev/null @@ -1,106 +0,0 @@ -/* - * 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 { readCliConfig } from './config'; - -describe('readCliConfig', () => { - it('should return empty config for empty cli', () => { - expect(readCliConfig({})).toEqual([]); - }); - - it('should return empty config for no matching keys', () => { - expect( - readCliConfig({ - test: '123', - } as any), - ).toEqual([]); - }); - - it('should return backend.baseUrl when backendUrl present in cli options', () => { - expect( - readCliConfig({ - backendUrl: '/', - }), - ).toEqual([ - { - data: { - backend: { - baseUrl: '/', - }, - }, - context: 'cli', - }, - ]); - }); - - it('should return app.baseUrl when publicPath present in cli options', () => { - expect( - readCliConfig({ - publicPath: '/', - }), - ).toEqual([ - { - data: { - app: { - baseUrl: '/', - }, - }, - context: 'cli', - }, - ]); - }); - - it('should return app.baseUrl and backend.baseUrl when publicPath and backendUrl present in cli options', () => { - expect( - readCliConfig({ - publicPath: '/', - backendUrl: '/api', - }), - ).toEqual([ - { - data: { - app: { - baseUrl: '/', - }, - backend: { - baseUrl: '/api', - }, - }, - context: 'cli', - }, - ]); - }); - - it('should throw for public paths that do NOT start with /', () => { - ['http://localhost:3000', './', '../..'].forEach(publicPath => - expect(() => { - readCliConfig({ - publicPath, - }); - }).toThrow('Public path must be relative'), - ); - }); - - it('should throw for backend urls that do NOT start with /', () => { - ['http://localhost:3000', './', '../..'].forEach(backendUrl => - expect(() => { - readCliConfig({ - backendUrl, - }); - }).toThrow('Backend URL must be relative'), - ); - }); -}); diff --git a/packages/cli/src/lib/config.ts b/packages/cli/src/lib/config.ts index 9084f72057..a76571c3f6 100644 --- a/packages/cli/src/lib/config.ts +++ b/packages/cli/src/lib/config.ts @@ -19,17 +19,11 @@ import { loadConfig, loadConfigSchema, } from '@backstage/config-loader'; -import { AppConfig, ConfigReader } from '@backstage/config'; +import { ConfigReader } from '@backstage/config'; import { paths } from './paths'; import { isValidUrl } from './urls'; import { getPackages } from '@manypkg/get-packages'; import { PackageGraph } from './monorepo'; -import { JsonObject } from '@backstage/types'; - -export type CliConfigOptions = { - publicPath?: string; - backendUrl?: string; -}; type Options = { args: string[]; @@ -38,44 +32,8 @@ type Options = { withFilteredKeys?: boolean; withDeprecatedKeys?: boolean; fullVisibility?: boolean; - cliOptions?: CliConfigOptions; }; -/** - * Read specific parameters from the CLI and add them to the build config. - * @param opts CLI passed parameters. - * @returns Array of config, empty if there is no relevant passed in cli options. - * - * @public - */ -export function readCliConfig(opts?: CliConfigOptions): AppConfig[] { - if (!opts || Object.keys(opts).length === 0) return []; - const data: JsonObject = {}; - - if (opts.publicPath?.startsWith('/')) { - data.app = { - baseUrl: opts.publicPath, - }; - } else if (opts.publicPath) { - throw new Error( - 'Public path must be relative and start with "/" when specified through CLI options. Path traversals like "./" and assumed relative endpoints like "backstage" are not supported.', - ); - } - if (opts.backendUrl?.startsWith('/')) { - data.backend = { - baseUrl: opts.backendUrl, - }; - } else if (opts.backendUrl) { - throw new Error( - 'Backend URL must be relative and start with "/" when specified through CLI options. Path traversals like "./" and assumed relative endpoint like "api" are not supported.', - ); - } - - if (Object.keys(data).length === 0) return []; - - return [{ data, context: 'cli' }]; -} - export async function loadCliConfig(options: Options) { const configTargets: ConfigTarget[] = []; options.args.forEach(arg => { @@ -114,8 +72,6 @@ export async function loadCliConfig(options: Options) { packagePaths: [paths.resolveTargetRoot('package.json')], }); - const cliConfigs = readCliConfig(options.cliOptions); - const { appConfigs } = await loadConfig({ experimentalEnvFunc: options.mockEnv ? async name => process.env[name] || 'x' @@ -124,9 +80,6 @@ export async function loadCliConfig(options: Options) { configTargets: configTargets, }); - // Add the cliConfigs to the end of the appConfigs array for final overriding. - appConfigs.push(...cliConfigs); - // printing to stderr to not clobber stdout in case the cli command // outputs structured data (e.g. as config:schema does) process.stderr.write( diff --git a/packages/cli/src/serve.test.ts b/packages/cli/src/serve.test.ts deleted file mode 100644 index 7256e7c4ad..0000000000 --- a/packages/cli/src/serve.test.ts +++ /dev/null @@ -1,163 +0,0 @@ -/* - * 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 { spawn, SpawnOptionsWithoutStdio } from 'child_process'; -import EventEmitter from 'events'; -import fetch from 'node-fetch'; -import path from 'path'; -import getPort from 'get-port'; - -jest.setTimeout(150000); - -const executeCommand = ( - command: string, - args: string[], - options?: SpawnOptionsWithoutStdio, - events?: EventEmitter, - eventConfig?: { - killRegex?: RegExp; - signalRegex?: RegExp[]; - }, -): Promise<{ - exit: number; - stdout: string; - stderr: string; -}> => { - return new Promise((resolve, reject) => { - const stdout: Buffer[] = []; - const stderr: Buffer[] = []; - - const shell = process.platform === 'win32'; - const proc = spawn(command, args, { ...options, shell }); - - proc.stdout?.on('data', data => { - stdout.push(Buffer.from(data)); - }); - - proc.stderr?.on('data', data => { - stderr.push(Buffer.from(data)); - }); - - /** - * Set an interval to check if we should kill the process. - * This was the easiest way I could think of of testing across two processes. - */ - let intervalId: NodeJS.Timer | undefined = undefined; - if (eventConfig) { - intervalId = setInterval(() => { - const stdoutStr = Buffer.concat(stdout).toString('utf8'); - if (eventConfig.killRegex?.test(stdoutStr)) { - proc.kill('SIGINT'); - } else if ( - eventConfig.signalRegex?.some(regex => regex.test(stdoutStr)) - ) { - events?.emit('hit'); - } - }, 1000); - } - - const clearEventInterval = () => { - if (intervalId) { - try { - clearInterval(intervalId); - } catch (err) { - console.error(err); - } - } - }; - - /** - * Need a way to kill the process from another process. - */ - events?.on('stop', signal => { - clearEventInterval(); - proc.kill(signal); - }); - - proc.on('error', (...errorArgs) => { - clearEventInterval(); - reject(errorArgs); - }); - proc.on('exit', code => { - clearEventInterval(); - resolve({ - exit: code ?? 0, - stdout: Buffer.concat(stdout).toString('utf8'), - stderr: Buffer.concat(stderr).toString('utf8'), - }); - }); - }); -}; - -const testProjectDir = path.resolve( - __dirname, - '__fixtures__/test-project/packages/app', -); - -describe('end-to-end', () => { - const entryPoint = path.resolve(__dirname, '../bin/backstage-cli'); - - it('builds frontend with correct url overrides', async () => { - const buildProc = await executeCommand( - entryPoint, - ['package', 'build', '--public-path', '/test', '--backend-url', '/api'], - { - cwd: testProjectDir, - }, - ); - expect(buildProc.stderr).toContain( - 'Loaded config from app-config.yaml, cli', - ); - - expect(buildProc.exit).toEqual(0); - }); - - it('starts frontend on correct url', async () => { - expect.assertions(4); - - const startEmitter = new EventEmitter(); - const frontendPort = await getPort(); - startEmitter.on('hit', async () => { - const response = await fetch( - `http://localhost:${frontendPort}/test/catalog`, - ); - const text = await response.text(); - startEmitter.emit('stop', 'SIGINT'); - expect(response.status).toBe(200); - expect(text.length).toBeGreaterThan(0); - expect(text).toContain('id="root"'); - }); - const startProc = await executeCommand( - entryPoint, - ['package', 'start'], - { - cwd: testProjectDir, - env: { - ...process.env, - PORT: `${frontendPort}`, - BACKEND_PORT: `${await getPort()}`, - }, - }, - startEmitter, - { - // Need to match console colors as well, so use .* instead of just ' '. - signalRegex: [/compiled.*successfully/], - }, - ); - - expect(startProc.exit).toEqual(0); - }); -}); From aa14a5453d75e543fc721f0e4bd4248efe971629 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 22 Nov 2022 15:26:19 -0500 Subject: [PATCH 25/42] cleaning up. Signed-off-by: Aramis Sennyey --- packages/cli/cli-report.md | 2 -- packages/cli/src/commands/build/buildFrontend.ts | 6 ++---- packages/core-app-api/src/app/AppManager.tsx | 4 ++-- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/cli/cli-report.md b/packages/cli/cli-report.md index dc0232e987..d82f8da60e 100644 --- a/packages/cli/cli-report.md +++ b/packages/cli/cli-report.md @@ -213,8 +213,6 @@ Options: --experimental-type-build --skip-build-dependencies --stats - --public-path - --backend-url --config -h, --help ``` diff --git a/packages/cli/src/commands/build/buildFrontend.ts b/packages/cli/src/commands/build/buildFrontend.ts index 92e42a8d64..6d7ca2e6a9 100644 --- a/packages/cli/src/commands/build/buildFrontend.ts +++ b/packages/cli/src/commands/build/buildFrontend.ts @@ -18,17 +18,16 @@ import fs from 'fs-extra'; import { resolve as resolvePath } from 'path'; import { buildBundle } from '../../lib/bundler'; import { getEnvironmentParallelism } from '../../lib/parallel'; -import { loadCliConfig, CliConfigOptions } from '../../lib/config'; +import { loadCliConfig } from '../../lib/config'; interface BuildAppOptions { targetDir: string; writeStats: boolean; - cliOptions?: CliConfigOptions; configPaths: string[]; } export async function buildFrontend(options: BuildAppOptions) { - const { targetDir, writeStats, configPaths, cliOptions } = options; + const { targetDir, writeStats, configPaths } = options; const { name } = await fs.readJson(resolvePath(targetDir, 'package.json')); await buildBundle({ targetDir, @@ -38,7 +37,6 @@ export async function buildFrontend(options: BuildAppOptions) { ...(await loadCliConfig({ args: configPaths, fromPackage: name, - cliOptions, })), }); } diff --git a/packages/core-app-api/src/app/AppManager.tsx b/packages/core-app-api/src/app/AppManager.tsx index abf4d44318..936378122d 100644 --- a/packages/core-app-api/src/app/AppManager.tsx +++ b/packages/core-app-api/src/app/AppManager.tsx @@ -157,7 +157,7 @@ function useConfigLoader( let configReader = ConfigReader.fromConfigs(config.value ?? []); const resolveRelativeUrl = (relativeUrl: string) => { - // Backend.baseUrl should always be defined. + // relativeUrl should always be defined. let url; try { url = new URL(relativeUrl, document.location.origin).href.replace( @@ -165,7 +165,7 @@ function useConfigLoader( '', ); } catch (err) { - // Backend.baseUrl was not a valid URL. This should be caught during the build process. + // relativeUrl was not a valid URL. This should be caught during the build process. } return url; }; From 5f400e81d46cadc1f0880e4ce8b47df053c468ae Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 22 Nov 2022 15:27:24 -0500 Subject: [PATCH 26/42] remove duplicate changeset file. Signed-off-by: Aramis Sennyey --- .changeset/metal-fishes-grow.md | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 .changeset/metal-fishes-grow.md diff --git a/.changeset/metal-fishes-grow.md b/.changeset/metal-fishes-grow.md deleted file mode 100644 index 885bcecd1b..0000000000 --- a/.changeset/metal-fishes-grow.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@backstage/core-app-api': patch ---- - -Apps will now detect when a relative `backend.baseUrl` or `app.baseUrl` is provided and update the config to produce a full URL. From 44200e6be7fde86a6fe1491ad457919edbb13fa4 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 22 Nov 2022 15:27:52 -0500 Subject: [PATCH 27/42] yarn.lock file updates Signed-off-by: Aramis Sennyey --- packages/cli/package.json | 2 +- yarn.lock | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/cli/package.json b/packages/cli/package.json index 661d53d897..5a65998301 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -272,4 +272,4 @@ } } } -} \ No newline at end of file +} diff --git a/yarn.lock b/yarn.lock index 634623d6b4..621ed3dd96 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3672,7 +3672,6 @@ __metadata: express: ^4.17.1 fork-ts-checker-webpack-plugin: ^7.0.0-alpha.8 fs-extra: 10.1.0 - get-port: ^6.1.2 glob: ^7.1.7 global-agent: ^3.0.0 handlebars: ^4.7.3 From 3426aec20164c64801314c362f9b66b22cdb4f68 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 22 Nov 2022 15:31:18 -0500 Subject: [PATCH 28/42] Update changeset message for the cli package. Signed-off-by: Aramis Sennyey --- .changeset/spotty-numbers-occur.md | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/.changeset/spotty-numbers-occur.md b/.changeset/spotty-numbers-occur.md index fff27a18d3..4e098f981e 100644 --- a/.changeset/spotty-numbers-occur.md +++ b/.changeset/spotty-numbers-occur.md @@ -2,10 +2,4 @@ '@backstage/cli': patch --- -Adds support for two new CLI options on the `repo build` and `package build` commands. Both options are only available to frontend components. Backend config continues to be supplied at runtime. - -CLI options take precedent over provided config. - -The new options are meant to add greater support for running the same bundle in multiple environments, ex: test, stage and prod. -`--publicPath`: Frontend route to host the app on, can be relative or absolute. -`--backendUrl`: Backend URL that the {backend.basePath}, e.g. /api, endpoint can be reached on. Can be relative or absolute. +Allow relative URLs to be passed as config values for `app.baseUrl` and `backend.baseUrl`, ie `app.baseUrl` can be `/`. Relative URLs are only supported for frontend builds, the backend still needs the full URL defined before run time. From b05b34ee8ccd030cae0a577bbb1c0ad9612649c2 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 22 Nov 2022 15:54:18 -0500 Subject: [PATCH 29/42] Vale fix. Signed-off-by: Aramis Sennyey --- .changeset/spotty-numbers-occur.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.changeset/spotty-numbers-occur.md b/.changeset/spotty-numbers-occur.md index 4e098f981e..89c0814fd0 100644 --- a/.changeset/spotty-numbers-occur.md +++ b/.changeset/spotty-numbers-occur.md @@ -2,4 +2,6 @@ '@backstage/cli': patch --- -Allow relative URLs to be passed as config values for `app.baseUrl` and `backend.baseUrl`, ie `app.baseUrl` can be `/`. Relative URLs are only supported for frontend builds, the backend still needs the full URL defined before run time. +Allow relative URLs to be passed as config values for `app.baseUrl` and `backend.baseUrl`. For example, `app.baseUrl` can now be `/`. + +Relative URLs are only supported for frontend builds, the backend still needs the full URL defined before run time. From f2ea849af1999815c4e87f84b5bfee917a2e5af9 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Mon, 28 Nov 2022 13:39:48 -0500 Subject: [PATCH 30/42] Only relative-ize the URLs when they match. The backend config should be able to dictate this, but the front end will adjust it. Signed-off-by: Aramis Sennyey --- .../core-app-api/src/app/AppManager.test.tsx | 53 ++++++++++++----- packages/core-app-api/src/app/AppManager.tsx | 59 +++++++++++-------- 2 files changed, 70 insertions(+), 42 deletions(-) diff --git a/packages/core-app-api/src/app/AppManager.test.tsx b/packages/core-app-api/src/app/AppManager.test.tsx index b8aeb21818..89cd108dea 100644 --- a/packages/core-app-api/src/app/AppManager.test.tsx +++ b/packages/core-app-api/src/app/AppManager.test.tsx @@ -619,34 +619,29 @@ describe('Integration Test', () => { { data: { backend: { - baseUrl: '/', + baseUrl: 'http://localhost:8008/', }, - }, - paths: ['backend.baseUrl'], - }, - { - data: { app: { - baseUrl: '/', + baseUrl: 'http://localhost:8008/', }, }, - paths: ['app.baseUrl'], + paths: ['app.baseUrl', 'backend.baseUrl'], }, { data: { backend: { - baseUrl: '/', + baseUrl: 'http://test.com/', }, app: { - baseUrl: '/', + baseUrl: 'http://test.com/', }, }, paths: ['app.baseUrl', 'backend.baseUrl'], }, ].forEach(item => { item.paths.forEach(path => { - it('should add the fully qualified origin when the relevant urls are relative', async () => { - await checkConfigValue(item.data, path, 'http://localhost'); + it('should force the urls to be relative when they are the same', async () => { + await checkConfigValue(item.data, path, 'http://localhost/'); }); }); }); @@ -654,19 +649,45 @@ describe('Integration Test', () => { { data: { backend: { - baseUrl: 'https://google.com', + baseUrl: 'http://test.com/backstage', + }, + app: { + baseUrl: 'http://test.com/backstage', }, }, - paths: ['backend.baseUrl'], + paths: ['app.baseUrl', 'backend.baseUrl'], }, + ].forEach(item => { + item.paths.forEach(path => { + it('should force the urls to be relative when they are the same AND keep path values', async () => { + await checkConfigValue(item.data, path, 'http://localhost/backstage'); + }); + }); + }); + [ { data: { + backend: { + baseUrl: 'http://test.com/backstage/my-instance', + }, app: { - baseUrl: 'https://bing.com', + baseUrl: 'http://test.com/backstage/my-instance', }, }, - paths: ['app.baseUrl'], + paths: ['app.baseUrl', 'backend.baseUrl'], }, + ].forEach(item => { + item.paths.forEach(path => { + it('should force the urls to be relative when they are the same AND keep nested path values', async () => { + await checkConfigValue( + item.data, + path, + 'http://localhost/backstage/my-instance', + ); + }); + }); + }); + [ { data: { backend: { diff --git a/packages/core-app-api/src/app/AppManager.tsx b/packages/core-app-api/src/app/AppManager.tsx index 936378122d..5b43a43966 100644 --- a/packages/core-app-api/src/app/AppManager.tsx +++ b/packages/core-app-api/src/app/AppManager.tsx @@ -156,35 +156,42 @@ function useConfigLoader( let configReader = ConfigReader.fromConfigs(config.value ?? []); - const resolveRelativeUrl = (relativeUrl: string) => { - // relativeUrl should always be defined. - let url; - try { - url = new URL(relativeUrl, document.location.origin).href.replace( - /\/$/, - '', - ); - } catch (err) { - // relativeUrl was not a valid URL. This should be caught during the build process. - } - return url; + const resolveRelativeUrl = (relativeUrl: string) => + new URL(relativeUrl, document.location.origin).href; + + const getRelativeUrl = (fullUrl: string) => { + const url = new URL(fullUrl); + return fullUrl.replace(url.origin, ''); }; - config.value?.push({ - data: { - app: { - baseUrl: resolveRelativeUrl( - configReader.getOptionalString('app.baseUrl') || '/', - ), + /** + * We only want to override the URLs with the document origin when the URLs match + * and are defined. We use getOptionalString here to not throw when the app.baseUrl + * and backend.baseUrl are not defined. If they are defined but not well formatted URLs + * the above resolveRelativeUrl() method will throw. + */ + if ( + configReader.getOptionalString('app.baseUrl') && + configReader.getOptionalString('backend.baseUrl') && + configReader.getOptionalString('app.baseUrl') === + configReader.getOptionalString('backend.baseUrl') + ) { + config.value?.push({ + data: { + app: { + baseUrl: resolveRelativeUrl( + getRelativeUrl(configReader.getString('app.baseUrl')), + ), + }, + backend: { + baseUrl: resolveRelativeUrl( + getRelativeUrl(configReader.getString('backend.baseUrl')), + ), + }, }, - backend: { - baseUrl: resolveRelativeUrl( - configReader.getOptionalString('backend.baseUrl') || '/', - ), - }, - }, - context: 'relative-resolver', - }); + context: 'relative-resolver', + }); + } configReader = ConfigReader.fromConfigs(config.value ?? []); From b045e9983473416fb2b60d261020bc6dc21a7c45 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Mon, 28 Nov 2022 13:44:51 -0500 Subject: [PATCH 31/42] update doc. Signed-off-by: Aramis Sennyey --- .changeset/young-chairs-check.md | 2 +- packages/core-app-api/src/app/AppManager.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/young-chairs-check.md b/.changeset/young-chairs-check.md index b0611af148..85d5bbf664 100644 --- a/.changeset/young-chairs-check.md +++ b/.changeset/young-chairs-check.md @@ -2,4 +2,4 @@ '@backstage/core-app-api': patch --- -Apps will now detect when a relative `backend.baseUrl` or `app.baseUrl` is provided and update it to be a full URL. This means that you can provide relative URLs and they will be resolved as expected across the application. +Apps will now rewrite `app.baseUrl` and `backend.baseUrl` to match `location.origin` when `app.baseUrl` is the same as `backend.baseUrl`. This will help reduce the number of front end builds you have to do with a specific config. diff --git a/packages/core-app-api/src/app/AppManager.tsx b/packages/core-app-api/src/app/AppManager.tsx index 5b43a43966..80d836a286 100644 --- a/packages/core-app-api/src/app/AppManager.tsx +++ b/packages/core-app-api/src/app/AppManager.tsx @@ -168,7 +168,7 @@ function useConfigLoader( * We only want to override the URLs with the document origin when the URLs match * and are defined. We use getOptionalString here to not throw when the app.baseUrl * and backend.baseUrl are not defined. If they are defined but not well formatted URLs - * the above resolveRelativeUrl() method will throw. + * the above getRelativeUrl() method will throw. */ if ( configReader.getOptionalString('app.baseUrl') && From c6fc99450d323eb6aff93badda08e314c0f5c5d6 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 29 Nov 2022 11:05:08 -0500 Subject: [PATCH 32/42] Add shortcut for empty config (mostly in test cases). Signed-off-by: Aramis Sennyey --- packages/core-app-api/src/app/AppManager.tsx | 79 +++++++++++--------- 1 file changed, 43 insertions(+), 36 deletions(-) diff --git a/packages/core-app-api/src/app/AppManager.tsx b/packages/core-app-api/src/app/AppManager.tsx index 80d836a286..9c3f0eab6d 100644 --- a/packages/core-app-api/src/app/AppManager.tsx +++ b/packages/core-app-api/src/app/AppManager.tsx @@ -154,46 +154,53 @@ function useConfigLoader( }; } - let configReader = ConfigReader.fromConfigs(config.value ?? []); - - const resolveRelativeUrl = (relativeUrl: string) => - new URL(relativeUrl, document.location.origin).href; - - const getRelativeUrl = (fullUrl: string) => { - const url = new URL(fullUrl); - return fullUrl.replace(url.origin, ''); - }; + let configReader; /** - * We only want to override the URLs with the document origin when the URLs match - * and are defined. We use getOptionalString here to not throw when the app.baseUrl - * and backend.baseUrl are not defined. If they are defined but not well formatted URLs - * the above getRelativeUrl() method will throw. + * config.value can be undefined or empty. If it's either, don't bother overriding anything. */ - if ( - configReader.getOptionalString('app.baseUrl') && - configReader.getOptionalString('backend.baseUrl') && - configReader.getOptionalString('app.baseUrl') === - configReader.getOptionalString('backend.baseUrl') - ) { - config.value?.push({ - data: { - app: { - baseUrl: resolveRelativeUrl( - getRelativeUrl(configReader.getString('app.baseUrl')), - ), - }, - backend: { - baseUrl: resolveRelativeUrl( - getRelativeUrl(configReader.getString('backend.baseUrl')), - ), - }, - }, - context: 'relative-resolver', - }); - } + if (config.value?.length) { + configReader = ConfigReader.fromConfigs(config.value); - configReader = ConfigReader.fromConfigs(config.value ?? []); + const resolveRelativeUrl = (relativeUrl: string) => + new URL(relativeUrl, document.location.origin).href; + + const getRelativeUrl = (fullUrl: string) => { + const url = new URL(fullUrl); + return fullUrl.replace(url.origin, ''); + }; + + const getOrigin = (url: string) => new URL(url).origin; + + const appBaseUrl = configReader.getString('app.baseUrl'); + const backendBaseUrl = configReader.getString('backend.baseUrl'); + const appOrigin = getOrigin(appBaseUrl); + const backendOrigin = getOrigin(backendBaseUrl); + + /** + * We only want to override the URLs with the document origin when the URLs match + * and are defined. We use getOptionalString here to not throw when the app.baseUrl + * and backend.baseUrl are not defined. If they are defined but not well formatted URLs + * the above getRelativeUrl() method will throw. + */ + if (appOrigin === backendOrigin) { + config.value.push({ + data: { + app: { + baseUrl: resolveRelativeUrl(getRelativeUrl(appBaseUrl)), + }, + backend: { + baseUrl: resolveRelativeUrl(getRelativeUrl(backendBaseUrl)), + }, + }, + context: 'relative-resolver', + }); + } + + configReader = ConfigReader.fromConfigs(config.value); + } else { + configReader = ConfigReader.fromConfigs([]); + } return { api: configReader }; } From 43ad38e75dca2c9c66bc029628734d67a3e5f99b Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 29 Nov 2022 11:41:33 -0500 Subject: [PATCH 33/42] Fix test cases. Signed-off-by: Aramis Sennyey --- packages/app/src/App.test.tsx | 1 + packages/dev-utils/src/devApp/render.test.tsx | 8 +++++++- packages/techdocs-cli-embedded-app/src/App.test.tsx | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/app/src/App.test.tsx b/packages/app/src/App.test.tsx index 8c7cefeb2c..6eaa9513cd 100644 --- a/packages/app/src/App.test.tsx +++ b/packages/app/src/App.test.tsx @@ -28,6 +28,7 @@ describe('App', () => { app: { title: 'Test', support: { url: 'http://localhost:7007/support' }, + baseUrl: 'http://localhost:3000', }, backend: { baseUrl: 'http://localhost:7007' }, lighthouse: { diff --git a/packages/dev-utils/src/devApp/render.test.tsx b/packages/dev-utils/src/devApp/render.test.tsx index 83a24bd9b2..f4e90df33d 100644 --- a/packages/dev-utils/src/devApp/render.test.tsx +++ b/packages/dev-utils/src/devApp/render.test.tsx @@ -24,7 +24,13 @@ const anyEnv = (process.env = { ...process.env }) as any; describe('DevAppBuilder', () => { it('should be able to render a component in a dev app', async () => { anyEnv.APP_CONFIG = [ - { context: 'test', data: { app: { title: 'Test App' } } }, + { + context: 'test', + data: { + app: { title: 'Test App', baseUrl: 'http://localhost:3000' }, + backend: { baseUrl: 'http://localhost:7007' }, + }, + }, ]; const MyComponent = () => { diff --git a/packages/techdocs-cli-embedded-app/src/App.test.tsx b/packages/techdocs-cli-embedded-app/src/App.test.tsx index 75658d271e..59ed11bb93 100644 --- a/packages/techdocs-cli-embedded-app/src/App.test.tsx +++ b/packages/techdocs-cli-embedded-app/src/App.test.tsx @@ -22,7 +22,7 @@ jest.mock('./config', () => ({ configLoader: async () => [ { data: { - app: { title: 'Test' }, + app: { title: 'Test', baseUrl: 'http://localhost:3000' }, backend: { baseUrl: 'http://localhost:7007' }, techdocs: { storageUrl: 'http://localhost:7007/api/techdocs/static/docs', From fc0aa3e00ac063c72b025f6b87fdfc4f549c44ae Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 29 Nov 2022 13:06:08 -0500 Subject: [PATCH 34/42] Rebase fixes Signed-off-by: Aramis Sennyey --- .changeset/spotty-numbers-occur.md | 7 ------- .github/vale/Vocab/Backstage/accept.txt | 6 ++++++ packages/app/src/App.tsx | 2 +- packages/core-app-api/src/app/AppManager.tsx | 6 +++--- 4 files changed, 10 insertions(+), 11 deletions(-) delete mode 100644 .changeset/spotty-numbers-occur.md diff --git a/.changeset/spotty-numbers-occur.md b/.changeset/spotty-numbers-occur.md deleted file mode 100644 index 89c0814fd0..0000000000 --- a/.changeset/spotty-numbers-occur.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -'@backstage/cli': patch ---- - -Allow relative URLs to be passed as config values for `app.baseUrl` and `backend.baseUrl`. For example, `app.baseUrl` can now be `/`. - -Relative URLs are only supported for frontend builds, the backend still needs the full URL defined before run time. diff --git a/.github/vale/Vocab/Backstage/accept.txt b/.github/vale/Vocab/Backstage/accept.txt index ca031c24ca..bba59094e3 100644 --- a/.github/vale/Vocab/Backstage/accept.txt +++ b/.github/vale/Vocab/Backstage/accept.txt @@ -1,5 +1,6 @@ abc accessors +ACLs addon addons ADRs @@ -32,6 +33,7 @@ boolean Brex builtins callout +CDNs Chai changeset changesets @@ -165,6 +167,7 @@ JWTs Kaewkasi Keyv Knex +KPIs kubectl kubernetes kubernetes @@ -349,6 +352,7 @@ templater Templater templaters Templaters +TFRecord theia thumbsup todo @@ -366,6 +370,7 @@ transpiled transpiler transpilers truthy +TSDoc typeahead ui unbreak @@ -394,6 +399,7 @@ widget's winston www WWW +XCMetrics XML xyz yaml diff --git a/packages/app/src/App.tsx b/packages/app/src/App.tsx index e766df868d..fd73e99bcb 100644 --- a/packages/app/src/App.tsx +++ b/packages/app/src/App.tsx @@ -150,7 +150,7 @@ const AppRouter = app.getRouter(); const routes = ( - } /> + } /> {/* TODO(rubenl): Move this to / once its more mature and components exist */} }> {homePage} diff --git a/packages/core-app-api/src/app/AppManager.tsx b/packages/core-app-api/src/app/AppManager.tsx index 9c3f0eab6d..fff061217c 100644 --- a/packages/core-app-api/src/app/AppManager.tsx +++ b/packages/core-app-api/src/app/AppManager.tsx @@ -463,7 +463,7 @@ export class AppManager implements BackstageApp { } return ( - + {children} @@ -472,7 +472,7 @@ export class AppManager implements BackstageApp { if (isReactRouterBeta()) { return ( - + @@ -484,7 +484,7 @@ export class AppManager implements BackstageApp { } return ( - + <>{children} From 6647a686a740c63d61d9c2151231d6d6808da212 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 29 Nov 2022 13:08:41 -0500 Subject: [PATCH 35/42] Don't allow relative urls at CLI time. Signed-off-by: Aramis Sennyey --- packages/cli/src/lib/bundler/config.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/lib/bundler/config.ts b/packages/cli/src/lib/bundler/config.ts index ace60883a4..1974153f02 100644 --- a/packages/cli/src/lib/bundler/config.ts +++ b/packages/cli/src/lib/bundler/config.ts @@ -36,12 +36,10 @@ import { runPlain } from '../run'; import ESLintPlugin from 'eslint-webpack-plugin'; import pickBy from 'lodash/pickBy'; -const NO_OP_URL = 'http://test-site-asdf.org'; - export function resolveBaseUrl(config: Config): URL { const baseUrl = config.getString('app.baseUrl'); try { - return new URL(baseUrl, NO_OP_URL); + return new URL(baseUrl); } catch (error) { throw new Error(`Invalid app.baseUrl, ${error}`); } @@ -90,7 +88,7 @@ export async function createConfig( const externalPkgs = packages.filter(p => !isChildPath(paths.root, p.dir)); const baseUrl = frontendConfig.getString('app.baseUrl'); - const validBaseUrl = new URL(baseUrl, NO_OP_URL); + const validBaseUrl = new URL(baseUrl); const publicPath = validBaseUrl.pathname.replace(/\/$/, ''); if (checksEnabled) { plugins.push( From 5850ef9b84a4c8ab084c882ab65a201793532a10 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 29 Nov 2022 13:09:46 -0500 Subject: [PATCH 36/42] Add changeset for CLI change. Signed-off-by: Aramis Sennyey --- .changeset/five-trainers-admire.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/five-trainers-admire.md diff --git a/.changeset/five-trainers-admire.md b/.changeset/five-trainers-admire.md new file mode 100644 index 0000000000..6d6a023828 --- /dev/null +++ b/.changeset/five-trainers-admire.md @@ -0,0 +1,5 @@ +--- +'@backstage/cli': patch +--- + +Fix webpack dev server issue where it wasn't serving index.html from correct endpoint on subsequent requests. From 5e547434021a334075f454a5f5f44b0a4042cae2 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 29 Nov 2022 17:05:22 -0500 Subject: [PATCH 37/42] Fix test cases. Signed-off-by: Aramis Sennyey --- .../core-app-api/src/app/AppManager.compat.test.tsx | 5 ++++- .../core-app-api/src/app/AppManager.stable.test.tsx | 10 ++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/core-app-api/src/app/AppManager.compat.test.tsx b/packages/core-app-api/src/app/AppManager.compat.test.tsx index bdebd79dc4..a0f27d751b 100644 --- a/packages/core-app-api/src/app/AppManager.compat.test.tsx +++ b/packages/core-app-api/src/app/AppManager.compat.test.tsx @@ -81,7 +81,10 @@ describe.each(['beta', 'stable'])('react-router %s', rrVersion => { configLoader: async () => [ { context: 'test', - data: { app: { baseUrl: 'http://localhost/foo' } }, + data: { + app: { baseUrl: 'http://localhost/foo' }, + backend: { baseUrl: 'http://localhost' }, + }, }, ], bindRoutes: () => {}, diff --git a/packages/core-app-api/src/app/AppManager.stable.test.tsx b/packages/core-app-api/src/app/AppManager.stable.test.tsx index a9948a6070..8c60293391 100644 --- a/packages/core-app-api/src/app/AppManager.stable.test.tsx +++ b/packages/core-app-api/src/app/AppManager.stable.test.tsx @@ -62,7 +62,10 @@ describe('AppManager', () => { configLoader: async () => [ { context: 'test', - data: { app: { baseUrl: 'http://localhost/foo' } }, + data: { + app: { baseUrl: 'http://localhost/foo' }, + backend: { baseUrl: 'http://localhost' }, + }, }, ], }); @@ -94,7 +97,10 @@ describe('AppManager', () => { configLoader: async () => [ { context: 'test', - data: { app: { baseUrl: 'http://localhost/foo' } }, + data: { + app: { baseUrl: 'http://localhost/foo' }, + backend: { baseUrl: 'http://localhost' }, + }, }, ], }); From 334f98910825a61e335ce3dbc2ae585efec77955 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Tue, 29 Nov 2022 18:57:59 -0500 Subject: [PATCH 38/42] Remove merged test case. Signed-off-by: Aramis Sennyey --- .../src/components/Link/Link.test.tsx | 52 ------------------- 1 file changed, 52 deletions(-) diff --git a/packages/core-components/src/components/Link/Link.test.tsx b/packages/core-components/src/components/Link/Link.test.tsx index 52e687d2e4..d65351d840 100644 --- a/packages/core-components/src/components/Link/Link.test.tsx +++ b/packages/core-components/src/components/Link/Link.test.tsx @@ -107,58 +107,6 @@ describe('', () => { }); }); - describe('resolves a sub-path correctly', () => { - it('when it starts with base path', async () => { - const testString = 'This is test string'; - const linkText = 'Navigate!'; - const configApi = new ConfigReader({ - app: { baseUrl: 'http://localhost:3000/example' }, - }); - - const { getByText } = render( - wrapInTestApp( - - {linkText} - - {testString}

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

} /> - - , - ), - ); - - expect(() => getByText(testString)).toThrow(); - fireEvent.click(getByText(linkText)); - await waitFor(() => { - expect(getByText(testString)).toBeInTheDocument(); - }); - }); - }); - describe('isExternalUri', () => { it.each([ [true, 'http://'], From 3ead02bc493b51c22f5a0eb0c623fa6be01189ac Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Wed, 30 Nov 2022 11:56:07 -0500 Subject: [PATCH 39/42] Revert test case changes. Signed-off-by: Aramis Sennyey --- .changeset/young-chairs-check.md | 2 +- packages/app/src/App.test.tsx | 1 - .../src/app/AppManager.compat.test.tsx | 1 - .../src/app/AppManager.stable.test.tsx | 2 - .../core-app-api/src/app/AppManager.test.tsx | 197 +++++++----------- packages/dev-utils/src/devApp/render.test.tsx | 3 +- .../src/App.test.tsx | 1 - 7 files changed, 81 insertions(+), 126 deletions(-) diff --git a/.changeset/young-chairs-check.md b/.changeset/young-chairs-check.md index 85d5bbf664..92426dd5e3 100644 --- a/.changeset/young-chairs-check.md +++ b/.changeset/young-chairs-check.md @@ -2,4 +2,4 @@ '@backstage/core-app-api': patch --- -Apps will now rewrite `app.baseUrl` and `backend.baseUrl` to match `location.origin` when `app.baseUrl` is the same as `backend.baseUrl`. This will help reduce the number of front end builds you have to do with a specific config. +Apps will now rewrite `app.baseUrl` and `backend.baseUrl` to match `location.origin` when `app.baseUrl` is the same as `backend.baseUrl`. This will help reduce the number of frontend builds you have to do with a specific config. diff --git a/packages/app/src/App.test.tsx b/packages/app/src/App.test.tsx index 6eaa9513cd..0c021a8f23 100644 --- a/packages/app/src/App.test.tsx +++ b/packages/app/src/App.test.tsx @@ -30,7 +30,6 @@ describe('App', () => { support: { url: 'http://localhost:7007/support' }, baseUrl: 'http://localhost:3000', }, - backend: { baseUrl: 'http://localhost:7007' }, lighthouse: { baseUrl: 'http://localhost:3003', }, diff --git a/packages/core-app-api/src/app/AppManager.compat.test.tsx b/packages/core-app-api/src/app/AppManager.compat.test.tsx index a0f27d751b..cfbcc87a01 100644 --- a/packages/core-app-api/src/app/AppManager.compat.test.tsx +++ b/packages/core-app-api/src/app/AppManager.compat.test.tsx @@ -83,7 +83,6 @@ describe.each(['beta', 'stable'])('react-router %s', rrVersion => { context: 'test', data: { app: { baseUrl: 'http://localhost/foo' }, - backend: { baseUrl: 'http://localhost' }, }, }, ], diff --git a/packages/core-app-api/src/app/AppManager.stable.test.tsx b/packages/core-app-api/src/app/AppManager.stable.test.tsx index 8c60293391..ba16353d47 100644 --- a/packages/core-app-api/src/app/AppManager.stable.test.tsx +++ b/packages/core-app-api/src/app/AppManager.stable.test.tsx @@ -64,7 +64,6 @@ describe('AppManager', () => { context: 'test', data: { app: { baseUrl: 'http://localhost/foo' }, - backend: { baseUrl: 'http://localhost' }, }, }, ], @@ -99,7 +98,6 @@ describe('AppManager', () => { context: 'test', data: { app: { baseUrl: 'http://localhost/foo' }, - backend: { baseUrl: 'http://localhost' }, }, }, ], diff --git a/packages/core-app-api/src/app/AppManager.test.tsx b/packages/core-app-api/src/app/AppManager.test.tsx index 89cd108dea..dd77c9b4bf 100644 --- a/packages/core-app-api/src/app/AppManager.test.tsx +++ b/packages/core-app-api/src/app/AppManager.test.tsx @@ -572,52 +572,10 @@ describe('Integration Test', () => { }); describe('relative url resolvers', () => { - const checkConfigValue = async ( - data: object, - configString: string, - expectedValue: string, - ) => { - const app = new AppManager({ - apis: [], - defaultApis: [], - themes: [ - { - id: 'light', - title: 'Light Theme', - variant: 'light', - Provider: ({ children }) => <>{children}, - }, - ], - icons, - plugins: [], - components, - configLoader: async () => [ - { - context: 'test', - data, - }, - ], - }); - - const Provider = app.getProvider(); - const ConfigDisplay = () => { - const apiHolder = useApiHolder(); - const config = apiHolder.get(configApiRef); - return {config?.getString(configString)}; - }; - - const dom = await renderWithEffects( - - - , - ); - - // Verify that the backend.baseUrl is set correctly by the AppManager. - expect(dom.getByText(expectedValue)).toBeTruthy(); - }; - [ - { - data: { + it.each([ + [ + [document.location.href, document.location.href], + { backend: { baseUrl: 'http://localhost:8008/', }, @@ -625,29 +583,13 @@ describe('Integration Test', () => { baseUrl: 'http://localhost:8008/', }, }, - paths: ['app.baseUrl', 'backend.baseUrl'], - }, - { - data: { - backend: { - baseUrl: 'http://test.com/', - }, - app: { - baseUrl: 'http://test.com/', - }, - }, - paths: ['app.baseUrl', 'backend.baseUrl'], - }, - ].forEach(item => { - item.paths.forEach(path => { - it('should force the urls to be relative when they are the same', async () => { - await checkConfigValue(item.data, path, 'http://localhost/'); - }); - }); - }); - [ - { - data: { + ], + [ + [ + `${document.location.origin}/backstage`, + `${document.location.origin}/backstage`, + ], + { backend: { baseUrl: 'http://test.com/backstage', }, @@ -655,65 +597,84 @@ describe('Integration Test', () => { baseUrl: 'http://test.com/backstage', }, }, - paths: ['app.baseUrl', 'backend.baseUrl'], - }, - ].forEach(item => { - item.paths.forEach(path => { - it('should force the urls to be relative when they are the same AND keep path values', async () => { - await checkConfigValue(item.data, path, 'http://localhost/backstage'); - }); - }); - }); - [ - { - data: { + ], + [ + [ + `${document.location.origin}/backstage/instance`, + `${document.location.origin}/backstage/instance`, + ], + { backend: { - baseUrl: 'http://test.com/backstage/my-instance', + baseUrl: 'http://test.com/backstage/instance', }, app: { - baseUrl: 'http://test.com/backstage/my-instance', + baseUrl: 'http://test.com/backstage/instance', }, }, - paths: ['app.baseUrl', 'backend.baseUrl'], - }, - ].forEach(item => { - item.paths.forEach(path => { - it('should force the urls to be relative when they are the same AND keep nested path values', async () => { - await checkConfigValue( - item.data, - path, - 'http://localhost/backstage/my-instance', - ); - }); - }); - }); - [ - { - data: { + ], + [ + [ + `http://test-front.com/backstage/instance`, + `http://test.com/backstage/instance`, + ], + { backend: { - baseUrl: 'https://google.com', + baseUrl: 'http://test.com/backstage/instance', }, app: { - baseUrl: 'https://bing.com', + baseUrl: 'http://test-front.com/backstage/instance', }, }, - paths: ['app.baseUrl', 'backend.baseUrl'], - }, - ].forEach(item => { - item.paths.forEach(path => { - it('should NOT change the origin when the relevant urls are absolute', async () => { - await checkConfigValue( - item.data, - path, - path - .split('.') - .reduce( - (o, i) => o[i] as { [key: string]: object }, - item.data as { [key: string]: object }, - ) as unknown as string, - ); + ], + ])( + 'should be %p when %p', + async ([expectedAppUrl, expectedBackendUrl], data) => { + const app = new AppManager({ + apis: [], + defaultApis: [], + themes: [ + { + id: 'light', + title: 'Light Theme', + variant: 'light', + Provider: ({ children }) => <>{children}, + }, + ], + icons, + plugins: [], + components, + configLoader: async () => [ + { + context: 'test', + data, + }, + ], }); - }); - }); + + const Provider = app.getProvider(); + const ConfigDisplay = ({ configString }: { configString: string }) => { + const apiHolder = useApiHolder(); + const config = apiHolder.get(configApiRef); + return ( + + {configString}: {config?.getString(configString)} + + ); + }; + + const dom = await renderWithEffects( + + + + , + ); + + expect(dom.getByText(`app.baseUrl: ${expectedAppUrl}`)).toBeTruthy(); + + expect( + dom.getByText(`backend.baseUrl: ${expectedBackendUrl}`), + ).toBeTruthy(); + }, + ); }); }); diff --git a/packages/dev-utils/src/devApp/render.test.tsx b/packages/dev-utils/src/devApp/render.test.tsx index f4e90df33d..2da6ef370a 100644 --- a/packages/dev-utils/src/devApp/render.test.tsx +++ b/packages/dev-utils/src/devApp/render.test.tsx @@ -27,8 +27,7 @@ describe('DevAppBuilder', () => { { context: 'test', data: { - app: { title: 'Test App', baseUrl: 'http://localhost:3000' }, - backend: { baseUrl: 'http://localhost:7007' }, + app: { title: 'Test App' }, }, }, ]; diff --git a/packages/techdocs-cli-embedded-app/src/App.test.tsx b/packages/techdocs-cli-embedded-app/src/App.test.tsx index 59ed11bb93..8415388ca9 100644 --- a/packages/techdocs-cli-embedded-app/src/App.test.tsx +++ b/packages/techdocs-cli-embedded-app/src/App.test.tsx @@ -23,7 +23,6 @@ jest.mock('./config', () => ({ { data: { app: { title: 'Test', baseUrl: 'http://localhost:3000' }, - backend: { baseUrl: 'http://localhost:7007' }, techdocs: { storageUrl: 'http://localhost:7007/api/techdocs/static/docs', }, From 120d60b39e71f37eff63256d959854c71d96ebc3 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Wed, 30 Nov 2022 12:19:32 -0500 Subject: [PATCH 40/42] Update how we rewrite the relative config. Signed-off-by: Aramis Sennyey --- packages/app/src/App.test.tsx | 2 +- .../core-app-api/src/app/AppManager.test.tsx | 2 +- packages/core-app-api/src/app/AppManager.tsx | 66 +++++++++++-------- .../src/App.test.tsx | 3 +- 4 files changed, 44 insertions(+), 29 deletions(-) diff --git a/packages/app/src/App.test.tsx b/packages/app/src/App.test.tsx index 0c021a8f23..8c7cefeb2c 100644 --- a/packages/app/src/App.test.tsx +++ b/packages/app/src/App.test.tsx @@ -28,8 +28,8 @@ describe('App', () => { app: { title: 'Test', support: { url: 'http://localhost:7007/support' }, - baseUrl: 'http://localhost:3000', }, + backend: { baseUrl: 'http://localhost:7007' }, lighthouse: { baseUrl: 'http://localhost:3003', }, diff --git a/packages/core-app-api/src/app/AppManager.test.tsx b/packages/core-app-api/src/app/AppManager.test.tsx index dd77c9b4bf..02a9a6fbe0 100644 --- a/packages/core-app-api/src/app/AppManager.test.tsx +++ b/packages/core-app-api/src/app/AppManager.test.tsx @@ -614,7 +614,7 @@ describe('Integration Test', () => { ], [ [ - `http://test-front.com/backstage/instance`, + `${document.location.origin}/backstage/instance`, `http://test.com/backstage/instance`, ], { diff --git a/packages/core-app-api/src/app/AppManager.tsx b/packages/core-app-api/src/app/AppManager.tsx index fff061217c..1387130943 100644 --- a/packages/core-app-api/src/app/AppManager.tsx +++ b/packages/core-app-api/src/app/AppManager.tsx @@ -82,6 +82,7 @@ import { ApiRegistry } from '../apis/system/ApiRegistry'; import { resolveRouteBindings } from './resolveRouteBindings'; import { BackstageRouteObject } from '../routing/types'; import { isReactRouterBeta } from './isReactRouterBeta'; +import { JsonObject } from '@backstage/types'; type CompatiblePlugin = | BackstagePlugin @@ -155,12 +156,11 @@ function useConfigLoader( } let configReader; - /** * config.value can be undefined or empty. If it's either, don't bother overriding anything. */ if (config.value?.length) { - configReader = ConfigReader.fromConfigs(config.value); + const urlConfigReader = ConfigReader.fromConfigs(config.value); const resolveRelativeUrl = (relativeUrl: string) => new URL(relativeUrl, document.location.origin).href; @@ -172,32 +172,46 @@ function useConfigLoader( const getOrigin = (url: string) => new URL(url).origin; - const appBaseUrl = configReader.getString('app.baseUrl'); - const backendBaseUrl = configReader.getString('backend.baseUrl'); - const appOrigin = getOrigin(appBaseUrl); - const backendOrigin = getOrigin(backendBaseUrl); + const appBaseUrl = urlConfigReader.getOptionalString('app.baseUrl'); + const backendBaseUrl = urlConfigReader.getOptionalString('backend.baseUrl'); + let configs = config.value; + const relativeResolverConfig: { data: JsonObject; context: string } = { + data: {}, + context: 'relative-resolver', + }; + if (appBaseUrl && backendBaseUrl) { + const appOrigin = getOrigin(appBaseUrl); + const backendOrigin = getOrigin(backendBaseUrl); - /** - * We only want to override the URLs with the document origin when the URLs match - * and are defined. We use getOptionalString here to not throw when the app.baseUrl - * and backend.baseUrl are not defined. If they are defined but not well formatted URLs - * the above getRelativeUrl() method will throw. - */ - if (appOrigin === backendOrigin) { - config.value.push({ - data: { - app: { - baseUrl: resolveRelativeUrl(getRelativeUrl(appBaseUrl)), - }, - backend: { - baseUrl: resolveRelativeUrl(getRelativeUrl(backendBaseUrl)), - }, - }, - context: 'relative-resolver', - }); + /** + * We only want to override the URLs with the document origin when the URLs match + * and are defined. We use getOptionalString here to not throw when the app.baseUrl + * and backend.baseUrl are not defined. If they are defined but not well formatted URLs + * the above getRelativeUrl() method will throw. + */ + if (appOrigin === backendOrigin) { + relativeResolverConfig.data.backend = { + baseUrl: resolveRelativeUrl(getRelativeUrl(backendBaseUrl)), + }; + } } - - configReader = ConfigReader.fromConfigs(config.value); + if (appBaseUrl) { + /** + * Rewriting app.baseUrl to the current document should be a no-op. The + * document hosting the app should always be the same url as the app + * references. + */ + relativeResolverConfig.data.app = { + baseUrl: resolveRelativeUrl(getRelativeUrl(appBaseUrl)), + }; + } + /** + * Only add the relative config if there is actually data to add. + */ + if (Object.keys(relativeResolverConfig.data).length) { + configs = configs.concat([relativeResolverConfig]); + } + configReader = ConfigReader.fromConfigs(configs); } else { configReader = ConfigReader.fromConfigs([]); } diff --git a/packages/techdocs-cli-embedded-app/src/App.test.tsx b/packages/techdocs-cli-embedded-app/src/App.test.tsx index 8415388ca9..75658d271e 100644 --- a/packages/techdocs-cli-embedded-app/src/App.test.tsx +++ b/packages/techdocs-cli-embedded-app/src/App.test.tsx @@ -22,7 +22,8 @@ jest.mock('./config', () => ({ configLoader: async () => [ { data: { - app: { title: 'Test', baseUrl: 'http://localhost:3000' }, + app: { title: 'Test' }, + backend: { baseUrl: 'http://localhost:7007' }, techdocs: { storageUrl: 'http://localhost:7007/api/techdocs/static/docs', }, From 76f1b3fee5a1cb12eca0a67d32bc86d93dea7442 Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Mon, 5 Dec 2022 18:42:19 -0500 Subject: [PATCH 41/42] Review fixes. Signed-off-by: Aramis Sennyey --- .changeset/five-trainers-admire.md | 2 +- .changeset/young-chairs-check.md | 2 +- packages/core-app-api/src/app/AppManager.tsx | 52 +++++++++++--------- 3 files changed, 30 insertions(+), 26 deletions(-) diff --git a/.changeset/five-trainers-admire.md b/.changeset/five-trainers-admire.md index 6d6a023828..e624fc222a 100644 --- a/.changeset/five-trainers-admire.md +++ b/.changeset/five-trainers-admire.md @@ -2,4 +2,4 @@ '@backstage/cli': patch --- -Fix webpack dev server issue where it wasn't serving index.html from correct endpoint on subsequent requests. +Fix webpack dev server issue where it wasn't serving `index.html` from correct endpoint on subsequent requests. diff --git a/.changeset/young-chairs-check.md b/.changeset/young-chairs-check.md index 92426dd5e3..e70073e9df 100644 --- a/.changeset/young-chairs-check.md +++ b/.changeset/young-chairs-check.md @@ -2,4 +2,4 @@ '@backstage/core-app-api': patch --- -Apps will now rewrite `app.baseUrl` and `backend.baseUrl` to match `location.origin` when `app.baseUrl` is the same as `backend.baseUrl`. This will help reduce the number of frontend builds you have to do with a specific config. +Apps will now rewrite the `app.baseUrl` configuration to match the current `location.origin`. The `backend.baseUrl` will also be rewritten in the same way when the `app.baseUrl` and `backend.baseUrl` have matching origins. This will reduce the need for separate frontend builds for different environments. diff --git a/packages/core-app-api/src/app/AppManager.tsx b/packages/core-app-api/src/app/AppManager.tsx index 1387130943..b62b857ddf 100644 --- a/packages/core-app-api/src/app/AppManager.tsx +++ b/packages/core-app-api/src/app/AppManager.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import { Config } from '@backstage/config'; +import { AppConfig, Config } from '@backstage/config'; import React, { ComponentType, createContext, @@ -82,7 +82,6 @@ import { ApiRegistry } from '../apis/system/ApiRegistry'; import { resolveRouteBindings } from './resolveRouteBindings'; import { BackstageRouteObject } from '../routing/types'; import { isReactRouterBeta } from './isReactRouterBeta'; -import { JsonObject } from '@backstage/types'; type CompatiblePlugin = | BackstagePlugin @@ -162,20 +161,36 @@ function useConfigLoader( if (config.value?.length) { const urlConfigReader = ConfigReader.fromConfigs(config.value); - const resolveRelativeUrl = (relativeUrl: string) => - new URL(relativeUrl, document.location.origin).href; - - const getRelativeUrl = (fullUrl: string) => { - const url = new URL(fullUrl); - return fullUrl.replace(url.origin, ''); - }; - + /** + * Return the origin of the given URL. + * @param url An absolute URL. + * @returns The given URL's origin. + * @throws If fullUrl is not a correctly formatted absolute URL. + */ const getOrigin = (url: string) => new URL(url).origin; + /** + * Resolve an absolute URL as relative to the current document. + * @param fullUrl URL to resolve. + * @returns Absolute URL with origin as the current document origin. + * @throws If fullUrl is not a correctly formatted absolute URL. + */ + const overrideOrigin = (fullUrl: string) => { + return new URL( + fullUrl.replace(getOrigin(fullUrl), ''), + document.location.origin, + ).href; + }; + + /** + * Test configs may not define `app.baseUrl` or `backend.baseUrl` and we + * don't want to enforce here. + */ const appBaseUrl = urlConfigReader.getOptionalString('app.baseUrl'); const backendBaseUrl = urlConfigReader.getOptionalString('backend.baseUrl'); + let configs = config.value; - const relativeResolverConfig: { data: JsonObject; context: string } = { + const relativeResolverConfig: AppConfig = { data: {}, context: 'relative-resolver', }; @@ -183,26 +198,15 @@ function useConfigLoader( const appOrigin = getOrigin(appBaseUrl); const backendOrigin = getOrigin(backendBaseUrl); - /** - * We only want to override the URLs with the document origin when the URLs match - * and are defined. We use getOptionalString here to not throw when the app.baseUrl - * and backend.baseUrl are not defined. If they are defined but not well formatted URLs - * the above getRelativeUrl() method will throw. - */ if (appOrigin === backendOrigin) { relativeResolverConfig.data.backend = { - baseUrl: resolveRelativeUrl(getRelativeUrl(backendBaseUrl)), + baseUrl: overrideOrigin(backendBaseUrl), }; } } if (appBaseUrl) { - /** - * Rewriting app.baseUrl to the current document should be a no-op. The - * document hosting the app should always be the same url as the app - * references. - */ relativeResolverConfig.data.app = { - baseUrl: resolveRelativeUrl(getRelativeUrl(appBaseUrl)), + baseUrl: overrideOrigin(appBaseUrl), }; } /** From e8dd46a5b59cc820e209f1dbc7dcac3afe238a3f Mon Sep 17 00:00:00 2001 From: Aramis Sennyey Date: Mon, 5 Dec 2022 18:44:11 -0500 Subject: [PATCH 42/42] Use useApi not useApiHolder Signed-off-by: Aramis Sennyey --- packages/core-app-api/src/app/AppManager.test.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/core-app-api/src/app/AppManager.test.tsx b/packages/core-app-api/src/app/AppManager.test.tsx index 02a9a6fbe0..e8adeb9a57 100644 --- a/packages/core-app-api/src/app/AppManager.test.tsx +++ b/packages/core-app-api/src/app/AppManager.test.tsx @@ -34,7 +34,7 @@ import { createSubRouteRef, createRoutableExtension, analyticsApiRef, - useApiHolder, + useApi, } from '@backstage/core-plugin-api'; import { AppManager } from './AppManager'; import { AppComponents, AppIcons } from './types'; @@ -653,8 +653,7 @@ describe('Integration Test', () => { const Provider = app.getProvider(); const ConfigDisplay = ({ configString }: { configString: string }) => { - const apiHolder = useApiHolder(); - const config = apiHolder.get(configApiRef); + const config = useApi(configApiRef); return ( {configString}: {config?.getString(configString)}