Merge remote-tracking branch 'origin' into migrate-to-msw

* origin:
  fix: ts
  fix: forwardRef to SidebarItem
  fix(cli): one more await in the test
  backend-common: make loadBackendConfig return config directly and log all resolved app-configs

.changeset/eighty-stingrays-type.md

@@ -0,0 +1,5 @@
+---
+'@backstage/core': patch
+---
+
+Add forwardRef to the SidebarItem

.changeset/weak-ducks-tan.md

@@ -0,0 +1,8 @@
+---
+'@backstage/backend-common': minor
+'example-backend': patch
+'@backstage/cli': patch
+'@backstage/create-app': patch
+---
+
+Change loadBackendConfig to return the config directly

packages/backend-common/src/config.ts

@@ -15,12 +15,18 @@
  */
 
 import { findPaths } from '@backstage/cli-common';
+import { Config, ConfigReader } from '@backstage/config';
 import { loadConfig } from '@backstage/config-loader';
+import { Logger } from 'winston';
+
+type Options = {
+  logger: Logger;
+};
 
 /**
  * Load configuration for a Backend
  */
-export async function loadBackendConfig() {
+export async function loadBackendConfig(options: Options): Promise<Config> {
   /* eslint-disable-next-line no-restricted-syntax */
   const paths = findPaths(__dirname);
   const configs = await loadConfig({
@@ -28,5 +34,10 @@ export async function loadBackendConfig() {
     rootPaths: [paths.targetRoot, paths.targetDir],
     shouldReadSecrets: true,
   });
-  return configs;
+
+  options.logger.info(
+    `Loaded config from ${configs.map(c => c.context).join(', ')}`,
+  );
+
+  return ConfigReader.fromConfigs(configs);
 }

packages/backend-common/src/service/lib/ServiceBuilderImpl.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { ConfigReader } from '@backstage/config';
+import { Config } from '@backstage/config';
 import compression from 'compression';
 import cors from 'cors';
 import express, { Router } from 'express';
@@ -77,7 +77,7 @@ export class ServiceBuilderImpl implements ServiceBuilder {
     this.module = moduleRef;
   }
 
-  loadConfig(config: ConfigReader): ServiceBuilder {
+  loadConfig(config: Config): ServiceBuilder {
     const backendConfig = config.getOptionalConfig('backend');
     if (!backendConfig) {
       return this;

packages/backend/src/index.ts

@@ -33,7 +33,7 @@ import {
   SingleHostDiscovery,
   UrlReaders,
 } from '@backstage/backend-common';
-import { ConfigReader } from '@backstage/config';
+import { Config } from '@backstage/config';
 import healthcheck from './plugins/healthcheck';
 import auth from './plugins/auth';
 import catalog from './plugins/catalog';
@@ -47,7 +47,7 @@ import graphql from './plugins/graphql';
 import app from './plugins/app';
 import { PluginEnvironment } from './types';
 
-function makeCreateEnv(config: ConfigReader) {
+function makeCreateEnv(config: Config) {
   const root = getRootLogger();
   const reader = UrlReaders.default({ logger: root, config });
   const discovery = SingleHostDiscovery.fromConfig(config);
@@ -64,9 +64,8 @@ function makeCreateEnv(config: ConfigReader) {
 }
 
 async function main() {
-  const configs = await loadBackendConfig();
-  const configReader = ConfigReader.fromConfigs(configs);
-  const createEnv = makeCreateEnv(configReader);
+  const config = await loadBackendConfig({ logger: getRootLogger() });
+  const createEnv = makeCreateEnv(config);
 
   const healthcheckEnv = useHotMemoize(module, () => createEnv('healthcheck'));
   const catalogEnv = useHotMemoize(module, () => createEnv('catalog'));
@@ -93,7 +92,7 @@ async function main() {
   apiRouter.use(notFoundHandler());
 
   const service = createServiceBuilder(module)
-    .loadConfig(configReader)
+    .loadConfig(config)
     .addRouter('', await healthcheck(healthcheckEnv))
     .addRouter('/api', apiRouter)
     .addRouter('', await app(appEnv));

packages/cli/src/commands/app/build.ts

@@ -26,6 +26,11 @@ export default async (cmd: Command) => {
     env: process.env.APP_ENV ?? process.env.NODE_ENV ?? 'production',
     rootPaths: [paths.targetRoot, paths.targetDir],
   });
+
+  console.log(
+    `Loaded config from ${appConfigs.map(c => c.context).join(', ')}`,
+  );
+
   await buildBundle({
     entry: 'src/index',
     parallel: parseParallel(process.env[PARALLEL_ENV_VAR]),

packages/cli/src/commands/app/serve.ts

@@ -25,6 +25,11 @@ export default async (cmd: Command) => {
     env: process.env.APP_ENV ?? process.env.NODE_ENV ?? 'development',
     rootPaths: [paths.targetRoot, paths.targetDir],
   });
+
+  console.log(
+    `Loaded config from ${appConfigs.map(c => c.context).join(', ')}`,
+  );
+
   const waitForExit = await serveBundle({
     entry: 'src/index',
     checksEnabled: cmd.check,

packages/cli/src/commands/backend/dev.ts

@@ -26,6 +26,10 @@ export default async (cmd: Command) => {
     rootPaths: [paths.targetRoot, paths.targetDir],
   });
 
+  console.log(
+    `Loaded config from ${appConfigs.map(c => c.context).join(', ')}`,
+  );
+
   const waitForExit = await serveBackend({
     entry: 'src/index',
     checksEnabled: cmd.check,

packages/cli/src/commands/config/print.ts

@@ -28,6 +28,10 @@ export default async (cmd: Command) => {
     rootPaths: [paths.targetRoot, paths.targetDir],
   });
 
+  console.log(
+    `Loaded config from ${appConfigs.map(c => c.context).join(', ')}`,
+  );
+
   const flatConfig = ConfigReader.fromConfigs(appConfigs).get();
 
   if (cmd.format === 'json') {

packages/cli/src/commands/plugin/export.ts

@@ -25,6 +25,11 @@ export default async (cmd: Command) => {
     env: process.env.APP_ENV ?? process.env.NODE_ENV ?? 'production',
     rootPaths: [paths.targetRoot, paths.targetDir],
   });
+
+  console.log(
+    `Loaded config from ${appConfigs.map(c => c.context).join(', ')}`,
+  );
+
   await buildBundle({
     entry: 'dev/index',
     statsJsonEnabled: cmd.stats,

packages/cli/src/commands/plugin/serve.ts

@@ -25,6 +25,11 @@ export default async (cmd: Command) => {
     env: process.env.APP_ENV ?? process.env.NODE_ENV ?? 'development',
     rootPaths: [paths.targetRoot, paths.targetDir],
   });
+
+  console.log(
+    `Loaded config from ${appConfigs.map(c => c.context).join(', ')}`,
+  );
+
   const waitForExit = await serveBundle({
     entry: 'dev/index',
     checksEnabled: cmd.check,

packages/cli/src/commands/remove-plugin/removePlugin.test.ts

@@ -114,7 +114,7 @@ describe('removePlugin', () => {
     it('removes plugin exports from /packages/app/src/package.json', async () => {
       const testFilePath = path.join(tempDir, 'test.ts');
       const pluginsFilePaths = path.join(appPath, 'src', 'plugins.ts');
-      createTestPluginFile(testFilePath, pluginsFilePaths);
+      await createTestPluginFile(testFilePath, pluginsFilePaths);
       try {
         await removeReferencesFromPluginsFile(testFilePath, testPluginName);
         const testFileContent = removeEmptyLines(

packages/core/src/layout/Sidebar/Items.tsx

@@ -25,7 +25,14 @@ import { BackstageTheme } from '@backstage/theme';
 import { IconComponent } from '@backstage/core-api';
 import SearchIcon from '@material-ui/icons/Search';
 import clsx from 'clsx';
-import React, { FC, useContext, useState, KeyboardEventHandler } from 'react';
+import React, {
+  FC,
+  useContext,
+  useState,
+  KeyboardEventHandler,
+  forwardRef,
+  ReactNode,
+} from 'react';
 import { NavLink } from 'react-router-dom';
 import { sidebarConfig, SidebarContext } from './config';
 
@@ -115,74 +122,93 @@ type SidebarItemProps = {
   to?: string;
   hasNotifications?: boolean;
   onClick?: () => void;
+  children?: ReactNode;
 };
 
-export const SidebarItem: FC<SidebarItemProps> = ({
-  icon: Icon,
-  text,
-  to,
-  hasNotifications = false,
-  onClick,
-  children,
-}) => {
-  const classes = useStyles();
-  // XXX (@koroeskohr): unsure this is optimal. But I just really didn't want to have the item component
-  // depend on the current location, and at least have it being optionally forced to selected.
-  // Still waiting on a Q answered to fine tune the implementation
-  const { isOpen } = useContext(SidebarContext);
+export const SidebarItem = forwardRef<any, SidebarItemProps>(
+  (
+    { icon: Icon, text, to, hasNotifications = false, onClick, children },
+    ref,
+  ) => {
+    const classes = useStyles();
+    // XXX (@koroeskohr): unsure this is optimal. But I just really didn't want to have the item component
+    // depend on the current location, and at least have it being optionally forced to selected.
+    // Still waiting on a Q answered to fine tune the implementation
+    const { isOpen } = useContext(SidebarContext);
 
-  const itemIcon = (
-    <Badge
-      color="secondary"
-      variant="dot"
-      overlap="circle"
-      invisible={!hasNotifications}
-    >
-      <Icon fontSize="small" className={classes.icon} />
-    </Badge>
-  );
+    const itemIcon = (
+      <Badge
+        color="secondary"
+        variant="dot"
+        overlap="circle"
+        invisible={!hasNotifications}
+      >
+        <Icon fontSize="small" className={classes.icon} />
+      </Badge>
+    );
 
-  const childProps = {
-    onClick,
-    className: clsx(classes.root, isOpen ? classes.open : classes.closed),
-  };
+    const childProps = {
+      onClick,
+      className: clsx(classes.root, isOpen ? classes.open : classes.closed),
+    };
+
+    if (!isOpen) {
+      if (to === undefined) {
+        return (
+          <div {...childProps} ref={ref}>
+            {itemIcon}
+          </div>
+        );
+      }
+
+      return (
+        <NavLink
+          {...childProps}
+          activeClassName={classes.selected}
+          to={to}
+          end
+          ref={ref}
+        >
+          {itemIcon}
+        </NavLink>
+      );
+    }
+
+    const content = (
+      <>
+        <div data-testid="login-button" className={classes.iconContainer}>
+          {itemIcon}
+        </div>
+        {text && (
+          <Typography variant="subtitle2" className={classes.label}>
+            {text}
+          </Typography>
+        )}
+        <div className={classes.secondaryAction}>{children}</div>
+      </>
+    );
 
-  if (!isOpen) {
     if (to === undefined) {
-      return <div {...childProps}>{itemIcon}</div>;
+      return (
+        <div {...childProps} ref={ref}>
+          {content}
+        </div>
+      );
     }
 
     return (
-      <NavLink {...childProps} activeClassName={classes.selected} to={to} end>
-        {itemIcon}
+      <NavLink
+        {...childProps}
+        activeClassName={classes.selected}
+        to={to}
+        end
+        ref={ref}
+      >
+        {content}
       </NavLink>
     );
-  }
-
-  const content = (
-    <>
-      <div data-testid="login-button" className={classes.iconContainer}>
-        {itemIcon}
-      </div>
-      {text && (
-        <Typography variant="subtitle2" className={classes.label}>
-          {text}
-        </Typography>
-      )}
-      <div className={classes.secondaryAction}>{children}</div>
-    </>
-  );
-
-  if (to === undefined) {
-    return <div {...childProps}>{content}</div>;
-  }
-
-  return (
-    <NavLink {...childProps} activeClassName={classes.selected} to={to} end>
-      {content}
-    </NavLink>
-  );
-};
+  },
+);
 
 type SidebarSearchFieldProps = {
   onSearch: (input: string) => void;

packages/create-app/templates/default-app/packages/backend/src/index.ts

@@ -17,7 +17,7 @@ import {
   SingleHostDiscovery,
   UrlReaders,
 } from '@backstage/backend-common';
-import { ConfigReader } from '@backstage/config';
+import { Config } from '@backstage/config';
 import auth from './plugins/auth';
 import catalog from './plugins/catalog';
 import scaffolder from './plugins/scaffolder';
@@ -25,7 +25,7 @@ import proxy from './plugins/proxy';
 import techdocs from './plugins/techdocs';
 import { PluginEnvironment } from './types';
 
-function makeCreateEnv(config: ConfigReader) {
+function makeCreateEnv(config: Config) {
   const root = getRootLogger();
   const reader = UrlReaders.default({ logger: root, config });
   const discovery = SingleHostDiscovery.fromConfig(config);
@@ -42,9 +42,8 @@ function makeCreateEnv(config: ConfigReader) {
 }
 
 async function main() {
-  const configs = await loadBackendConfig();
-  const configReader = ConfigReader.fromConfigs(configs);
-  const createEnv = makeCreateEnv(configReader);
+  const config = await loadBackendConfig({logger: getRootLogger()});
+  const createEnv = makeCreateEnv(config);
 
   const catalogEnv = useHotMemoize(module, () => createEnv('catalog'));
   const scaffolderEnv = useHotMemoize(module, () => createEnv('scaffolder'));
@@ -61,7 +60,7 @@ async function main() {
   apiRouter.use(notFoundHandler());
 
   const service = createServiceBuilder(module)
-    .loadConfig(configReader)
+    .loadConfig(config)
     .addRouter('/api', apiRouter)
 
   await service.start().catch(err => {

plugins/auth-backend/src/service/standaloneServer.ts

@@ -17,7 +17,6 @@
 import Knex from 'knex';
 import { Server } from 'http';
 import { Logger } from 'winston';
-import { ConfigReader } from '@backstage/config';
 import { createRouter } from './router';
 import {
   createServiceBuilder,
@@ -34,7 +33,7 @@ export async function startStandaloneServer(
   options: ServerOptions,
 ): Promise<Server> {
   const logger = options.logger.child({ service: 'auth-backend' });
-  const config = ConfigReader.fromConfigs(await loadBackendConfig());
+  const config = await loadBackendConfig({ logger });
   const discovery = SingleHostDiscovery.fromConfig(config);
 
   const database = useHotMemoize(module, () => {

plugins/catalog-backend/src/service/standaloneServer.ts

@@ -20,7 +20,6 @@ import {
   UrlReaders,
   useHotMemoize,
 } from '@backstage/backend-common';
-import { ConfigReader } from '@backstage/config';
 import { Server } from 'http';
 import { Logger } from 'winston';
 import { DatabaseManager } from '../database';
@@ -37,7 +36,7 @@ export async function startStandaloneServer(
   options: ServerOptions,
 ): Promise<Server> {
   const logger = options.logger.child({ service: 'catalog-backend' });
-  const config = ConfigReader.fromConfigs(await loadBackendConfig());
+  const config = await loadBackendConfig({ logger });
   const reader = UrlReaders.default({ logger, config });
   const db = useHotMemoize(module, () =>
     DatabaseManager.createInMemoryDatabaseConnection(),

plugins/proxy-backend/src/service/router.test.ts

@@ -16,7 +16,6 @@
 
 import { buildMiddleware, createRouter } from './router';
 import * as winston from 'winston';
-import { ConfigReader } from '@backstage/config';
 import {
   loadBackendConfig,
   SingleHostDiscovery,
@@ -42,7 +41,7 @@ const mockCreateProxyMiddleware = createProxyMiddleware as jest.MockedFunction<
 describe('createRouter', () => {
   it('works', async () => {
     const logger = winston.createLogger();
-    const config = ConfigReader.fromConfigs(await loadBackendConfig());
+    const config = await loadBackendConfig({ logger });
     const discovery = SingleHostDiscovery.fromConfig(config);
     const router = await createRouter({
       config,

plugins/proxy-backend/src/service/standaloneServer.ts

@@ -22,7 +22,6 @@ import {
 import { Server } from 'http';
 import { Logger } from 'winston';
 import { createRouter } from './router';
-import { ConfigReader } from '@backstage/config';
 
 export interface ServerOptions {
   port: number;
@@ -37,7 +36,7 @@ export async function startStandaloneServer(
 
   logger.debug('Creating application...');
 
-  const config = ConfigReader.fromConfigs(await loadBackendConfig());
+  const config = await loadBackendConfig({ logger });
   const discovery = SingleHostDiscovery.fromConfig(config);
   const router = await createRouter({
     config,

plugins/rollbar-backend/src/service/standaloneServer.ts

@@ -20,7 +20,6 @@ import {
   createServiceBuilder,
   loadBackendConfig,
 } from '@backstage/backend-common';
-import { ConfigReader } from '@backstage/config';
 import { createRouter } from './router';
 
 export interface ServerOptions {
@@ -33,7 +32,7 @@ export async function startStandaloneServer(
   options: ServerOptions,
 ): Promise<Server> {
   const logger = options.logger.child({ service: 'rollbar-backend' });
-  const config = ConfigReader.fromConfigs(await loadBackendConfig());
+  const config = await loadBackendConfig({ logger });
 
   logger.debug('Creating application...');
 

plugins/techdocs-backend/src/default-branch.ts

@@ -15,8 +15,8 @@
  */
 import fetch from 'cross-fetch';
 import parseGitUrl from 'git-url-parse';
-import { ConfigReader, Config } from '@backstage/config';
-import { loadBackendConfig } from '@backstage/backend-common';
+import { Config } from '@backstage/config';
+import { getRootLogger, loadBackendConfig } from '@backstage/backend-common';
 
 interface IGitlabBranch {
   name: string;
@@ -230,7 +230,8 @@ async function getAzureDefaultBranch(
 export const getDefaultBranch = async (
   repositoryUrl: string,
 ): Promise<string> => {
-  const config = ConfigReader.fromConfigs(await loadBackendConfig());
+  // TODO(Rugvip): Config should not be loaded here, pass it in instead
+  const config = await loadBackendConfig({ logger: getRootLogger() });
   const typeMapping = [
     { url: /github*/g, type: 'github' },
     { url: /gitlab*/g, type: 'gitlab' },