cli: refactor role functions to work around simpler PackageRole

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>

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

@@ -16,19 +16,20 @@
 
 import { Command } from 'commander';
 import { buildPackage, Output } from '../../lib/builder';
-import { PackageRoleName, readRoleForCommand } from '../../lib/role';
+import { PackageRole, findRoleFromCommand, getRoleInfo } from '../../lib/role';
 
-const bundledRoles: PackageRoleName[] = ['app', 'backend'];
+const bundledRoles: PackageRole[] = ['app', 'backend'];
 
 const esmPlatforms = ['web', 'common'];
 const cjsPlatforms = ['node', 'common'];
 
 export async function command(cmd: Command): Promise<void> {
-  const roleInfo = await readRoleForCommand(cmd);
+  const role = await findRoleFromCommand(cmd);
+  const roleInfo = getRoleInfo(role);
 
-  if (bundledRoles.includes(roleInfo.role)) {
+  if (bundledRoles.includes(role)) {
     throw new Error(
-      `Build command is not supported for package role '${roleInfo.role}'`,
+      `Build command is not supported for package role '${role}'`,
     );
   }
 
@@ -40,7 +41,7 @@ export async function command(cmd: Command): Promise<void> {
   if (esmPlatforms.includes(roleInfo.platform)) {
     outputs.add(Output.esm);
   }
-  if (roleInfo.role !== 'cli') {
+  if (role !== 'cli') {
     outputs.add(Output.types);
   }
 

packages/cli/src/commands/bundle/command.ts

@@ -17,10 +17,10 @@
 import { Command } from 'commander';
 import { bundleApp } from './bundleApp';
 import { bundleBackend } from './bundleBackend';
-import { readRoleForCommand } from '../../lib/role';
+import { findRoleFromCommand } from '../../lib/role';
 
 export async function command(cmd: Command): Promise<void> {
-  const roleInfo = await readRoleForCommand(cmd);
+  const role = await findRoleFromCommand(cmd);
 
   const options = {
     configPaths: cmd.config as string[],
@@ -28,13 +28,11 @@ export async function command(cmd: Command): Promise<void> {
     skipBuildDependencies: Boolean(cmd.skipBuildDependencies),
   };
 
-  if (roleInfo.role === 'app') {
+  if (role === 'app') {
     return bundleApp(options);
-  } else if (roleInfo.role === 'backend') {
+  } else if (role === 'backend') {
     return bundleBackend(options);
   }
 
-  throw new Error(
-    `Bundle command is not supported for package role '${roleInfo.role}'`,
-  );
+  throw new Error(`Bundle command is not supported for package role '${role}'`);
 }

packages/cli/src/commands/migrate/packageRole.ts

@@ -18,7 +18,7 @@ import fs from 'fs-extra';
 import { resolve as resolvePath } from 'path';
 import { getPackages } from '@manypkg/get-packages';
 import { paths } from '../../lib/paths';
-import { readPackageRole, detectPackageRole } from '../../lib/role';
+import { getRoleFromPackage, detectRoleFromPackage } from '../../lib/role';
 
 export default async () => {
   const { packages } = await getPackages(paths.targetDir);
@@ -26,18 +26,18 @@ export default async () => {
   await Promise.all(
     packages.map(async ({ dir, packageJson: pkg }) => {
       const { name } = pkg;
-      const existingRole = readPackageRole(pkg);
+      const existingRole = getRoleFromPackage(pkg);
       if (existingRole) {
         return;
       }
 
-      const detectedRole = detectPackageRole(pkg);
+      const detectedRole = detectRoleFromPackage(pkg);
       if (!detectedRole) {
         console.error(`No role detected for package ${name}`);
         return;
       }
 
-      console.log(`Detected package role of ${name} as ${detectedRole.role}`);
+      console.log(`Detected package role of ${name} as ${detectedRole}`);
 
       let newPkg = pkg as any;
 
@@ -45,7 +45,7 @@ export default async () => {
       if (pkgKeys.includes('backstage')) {
         newPkg.backstage = {
           ...newPkg.backstage,
-          role: detectedRole.role,
+          role: detectedRole,
         };
       } else {
         // We insert the backstage field after one of these fields, otherwise at the end
@@ -57,7 +57,7 @@ export default async () => {
           ) + 1 || pkgKeys.length;
 
         const pkgEntries = Object.entries(pkg);
-        pkgEntries.splice(index, 0, ['backstage', { role: detectedRole.role }]);
+        pkgEntries.splice(index, 0, ['backstage', { role: detectedRole }]);
         newPkg = Object.fromEntries(pkgEntries);
       }
 

packages/cli/src/commands/migrate/packageScripts.ts

@@ -17,23 +17,23 @@
 import fs from 'fs-extra';
 import { resolve as resolvePath } from 'path';
 import { PackageGraph } from '../../lib/monorepo';
-import { readPackageRole, PackageRoleName } from '../../lib/role';
+import { getRoleFromPackage, PackageRole } from '../../lib/role';
 
-const bundledRoles: PackageRoleName[] = ['app', 'backend'];
-const noStartRoles: PackageRoleName[] = ['cli', 'common-library'];
+const bundledRoles: PackageRole[] = ['app', 'backend'];
+const noStartRoles: PackageRole[] = ['cli', 'common-library'];
 
 export async function command() {
   const packages = await PackageGraph.listTargetPackages();
 
   await Promise.all(
     packages.map(async ({ dir, packageJson }) => {
-      const roleInfo = readPackageRole(packageJson);
-      if (!roleInfo) {
+      const role = getRoleFromPackage(packageJson);
+      if (!role) {
         return;
       }
 
-      const hasStart = !noStartRoles.includes(roleInfo.role);
-      const isBundled = bundledRoles.includes(roleInfo.role);
+      const hasStart = !noStartRoles.includes(role);
+      const isBundled = bundledRoles.includes(role);
 
       const expectedScripts = {
         ...(hasStart && { start: 'backstage-cli script start' }),

packages/cli/src/commands/start/command.ts

@@ -17,10 +17,10 @@
 import { Command } from 'commander';
 import { startBackend } from './startBackend';
 import { startFrontend } from './startFrontend';
-import { readRoleForCommand } from '../../lib/role';
+import { findRoleFromCommand } from '../../lib/role';
 
 export async function command(cmd: Command): Promise<void> {
-  const roleInfo = await readRoleForCommand(cmd);
+  const role = await findRoleFromCommand(cmd);
 
   const options = {
     configPaths: cmd.config as string[],
@@ -29,7 +29,7 @@ export async function command(cmd: Command): Promise<void> {
     inspectBrkEnabled: Boolean(cmd.inspectBrk),
   };
 
-  switch (roleInfo.role) {
+  switch (role) {
     case 'backend':
     case 'plugin-backend':
     case 'plugin-backend-module':
@@ -47,7 +47,7 @@ export async function command(cmd: Command): Promise<void> {
       return startFrontend({ entry: 'dev/index', ...options });
     default:
       throw new Error(
-        `Start command is not supported for package role '${roleInfo.role}'`,
+        `Start command is not supported for package role '${role}'`,
       );
   }
 }

packages/cli/src/lib/monorepo/PackageGraph.ts

@@ -16,7 +16,7 @@
 
 import { getPackages, Package } from '@manypkg/get-packages';
 import { paths } from '../paths';
-import { PackageRoleName } from '../role';
+import { PackageRole } from '../role';
 
 type PackageJSON = Package['packageJson'];
 
@@ -29,7 +29,7 @@ export interface ExtendedPackageJSON extends PackageJSON {
   bundled?: boolean;
 
   backstage?: {
-    role?: PackageRoleName;
+    role?: PackageRole;
   };
 }
 

packages/cli/src/lib/role/index.ts

@@ -14,14 +14,10 @@
  * limitations under the License.
  */
 
-export type {
-  PackageRoleInfo,
-  PackagePlatform,
-  PackageRoleName,
-} from './types';
+export type { PackageRoleInfo, PackagePlatform, PackageRole } from './types';
 export {
   getRoleInfo,
-  readPackageRole,
-  readRoleForCommand,
-  detectPackageRole,
+  getRoleFromPackage,
+  findRoleFromCommand,
+  detectRoleFromPackage,
 } from './packageRoles';

packages/cli/src/lib/role/packageRoles.test.ts

@@ -18,9 +18,9 @@ import mockFs from 'mock-fs';
 import { Command } from 'commander';
 import {
   getRoleInfo,
-  readPackageRole,
-  readRoleForCommand,
-  detectPackageRole,
+  getRoleFromPackage,
+  findRoleFromCommand,
+  detectRoleFromPackage,
 } from './packageRoles';
 
 describe('getRoleInfo', () => {
@@ -28,11 +28,13 @@ describe('getRoleInfo', () => {
     expect(getRoleInfo('web-library')).toEqual({
       role: 'web-library',
       platform: 'web',
+      bundled: false,
     });
 
     expect(getRoleInfo('app')).toEqual({
       role: 'app',
       platform: 'web',
+      bundled: true,
     });
 
     expect(() => getRoleInfo('invalid')).toThrow(
@@ -41,39 +43,33 @@ describe('getRoleInfo', () => {
   });
 });
 
-describe('readPackageRole', () => {
+describe('getRoleFromPackage', () => {
   it('reads explicit package roles', () => {
     expect(
-      readPackageRole({
+      getRoleFromPackage({
         backstage: {
           role: 'web-library',
         },
       }),
-    ).toEqual({
-      role: 'web-library',
-      platform: 'web',
-    });
+    ).toEqual('web-library');
 
     expect(
-      readPackageRole({
+      getRoleFromPackage({
         backstage: {
           role: 'app',
         },
       }),
-    ).toEqual({
-      role: 'app',
-      platform: 'web',
-    });
+    ).toEqual('app');
 
     expect(() =>
-      readPackageRole({
+      getRoleFromPackage({
         name: 'test',
         backstage: {},
       }),
     ).toThrow('Package test must specify a role in the "backstage" field');
 
     expect(() =>
-      readPackageRole({
+      getRoleFromPackage({
         name: 'test',
         backstage: { role: 'invalid' },
       }),
@@ -81,7 +77,7 @@ describe('readPackageRole', () => {
   });
 });
 
-describe('readRoleForCommand', () => {
+describe('findRoleFromCommand', () => {
   function mkCommand(args: string) {
     return new Command()
       .option('--role <role>', 'test role')
@@ -104,28 +100,24 @@ describe('readRoleForCommand', () => {
   });
 
   it('provides role info by role', async () => {
-    await expect(readRoleForCommand(mkCommand(''))).resolves.toEqual({
-      role: 'web-library',
-      platform: 'web',
-    });
+    await expect(findRoleFromCommand(mkCommand(''))).resolves.toEqual(
+      'web-library',
+    );
 
     await expect(
-      readRoleForCommand(mkCommand('--role node-library')),
-    ).resolves.toEqual({
-      role: 'node-library',
-      platform: 'node',
-    });
+      findRoleFromCommand(mkCommand('--role node-library')),
+    ).resolves.toEqual('node-library');
 
     await expect(
-      readRoleForCommand(mkCommand('--role invalid')),
+      findRoleFromCommand(mkCommand('--role invalid')),
     ).rejects.toThrow(`Unknown package role 'invalid'`);
   });
 });
 
-describe('detectPackageRole', () => {
+describe('detectRoleFromPackage', () => {
   it('detects the role of example-app', () => {
     expect(
-      detectPackageRole({
+      detectRoleFromPackage({
         name: 'example-app',
         private: true,
         bundled: true,
@@ -143,15 +135,12 @@ describe('detectPackageRole', () => {
           'cy:run': 'cypress run',
         },
       }),
-    ).toEqual({
-      role: 'app',
-      platform: 'web',
-    });
+    ).toEqual('app');
   });
 
   it('detects the role of example-backend', () => {
     expect(
-      detectPackageRole({
+      detectRoleFromPackage({
         name: 'example-backend',
         main: 'dist/index.cjs.js',
         types: 'src/index.ts',
@@ -166,15 +155,12 @@ describe('detectPackageRole', () => {
           'migrate:create': 'knex migrate:make -x ts',
         },
       }),
-    ).toEqual({
-      role: 'backend',
-      platform: 'node',
-    });
+    ).toEqual('backend');
   });
 
   it('detects the role of @backstage/plugin-catalog', () => {
     expect(
-      detectPackageRole({
+      detectRoleFromPackage({
         name: '@backstage/plugin-catalog',
         main: 'src/index.ts',
         types: 'src/index.ts',
@@ -194,15 +180,12 @@ describe('detectPackageRole', () => {
           clean: 'backstage-cli clean',
         },
       }),
-    ).toEqual({
-      role: 'plugin-frontend',
-      platform: 'web',
-    });
+    ).toEqual('plugin-frontend');
   });
 
   it('detects the role of @backstage/plugin-catalog-backend', () => {
     expect(
-      detectPackageRole({
+      detectRoleFromPackage({
         name: '@backstage/plugin-catalog-backend',
         main: 'src/index.ts',
         types: 'src/index.ts',
@@ -221,15 +204,12 @@ describe('detectPackageRole', () => {
           clean: 'backstage-cli clean',
         },
       }),
-    ).toEqual({
-      role: 'plugin-backend',
-      platform: 'node',
-    });
+    ).toEqual('plugin-backend');
   });
 
   it('detects the role of @backstage/plugin-catalog-react', () => {
     expect(
-      detectPackageRole({
+      detectRoleFromPackage({
         name: '@backstage/plugin-catalog-react',
         main: 'src/index.ts',
         types: 'src/index.ts',
@@ -247,15 +227,12 @@ describe('detectPackageRole', () => {
           clean: 'backstage-cli clean',
         },
       }),
-    ).toEqual({
-      role: 'web-library',
-      platform: 'web',
-    });
+    ).toEqual('web-library');
   });
 
   it('detects the role of @backstage/plugin-catalog-common', () => {
     expect(
-      detectPackageRole({
+      detectRoleFromPackage({
         name: '@backstage/plugin-catalog-common',
         main: 'src/index.ts',
         types: 'src/index.ts',
@@ -274,15 +251,12 @@ describe('detectPackageRole', () => {
           clean: 'backstage-cli clean',
         },
       }),
-    ).toEqual({
-      role: 'common-library',
-      platform: 'common',
-    });
+    ).toEqual('common-library');
   });
 
   it('detects the role of @backstage/plugin-catalog-backend-module-ldap', () => {
     expect(
-      detectPackageRole({
+      detectRoleFromPackage({
         name: '@backstage/plugin-catalog-backend-module-ldap',
         main: 'src/index.ts',
         types: 'src/index.ts',
@@ -300,15 +274,12 @@ describe('detectPackageRole', () => {
           clean: 'backstage-cli clean',
         },
       }),
-    ).toEqual({
-      role: 'plugin-backend-module',
-      platform: 'node',
-    });
+    ).toEqual('plugin-backend-module');
   });
 
   it('detects the role of @backstage/plugin-permission-node', () => {
     expect(
-      detectPackageRole({
+      detectRoleFromPackage({
         name: '@backstage/plugin-permission-node',
         main: 'src/index.ts',
         types: 'src/index.ts',
@@ -327,15 +298,12 @@ describe('detectPackageRole', () => {
           clean: 'backstage-cli clean',
         },
       }),
-    ).toEqual({
-      role: 'node-library',
-      platform: 'node',
-    });
+    ).toEqual('node-library');
   });
 
   it('detects the role of @backstage/plugin-analytics-module-ga', () => {
     expect(
-      detectPackageRole({
+      detectRoleFromPackage({
         name: '@backstage/plugin-analytics-module-ga',
         main: 'src/index.ts',
         types: 'src/index.ts',
@@ -355,9 +323,6 @@ describe('detectPackageRole', () => {
           clean: 'backstage-cli clean',
         },
       }),
-    ).toEqual({
-      role: 'plugin-frontend-module',
-      platform: 'web',
-    });
+    ).toEqual('plugin-frontend-module');
   });
 });

packages/cli/src/lib/role/packageRoles.ts

@@ -18,24 +18,23 @@ import { z } from 'zod';
 import fs from 'fs-extra';
 import { Command } from 'commander';
 import { paths } from '../paths';
-import { PackageRoleInfo } from './types';
+import { PackageRole, PackageRoleInfo } from './types';
 
-const packageRoles: PackageRoleInfo[] = [
-  { role: 'app', platform: 'web' },
-  { role: 'backend', platform: 'node' },
-  { role: 'cli', platform: 'node' },
-  { role: 'web-library', platform: 'web' },
-  { role: 'node-library', platform: 'node' },
-  { role: 'common-library', platform: 'common' },
-  { role: 'plugin-frontend', platform: 'web' },
-  { role: 'plugin-frontend-module', platform: 'web' },
-  { role: 'plugin-backend', platform: 'node' },
-  { role: 'plugin-backend-module', platform: 'node' },
+const packageRoleInfos: PackageRoleInfo[] = [
+  { role: 'app', bundled: true, platform: 'web' },
+  { role: 'backend', bundled: true, platform: 'node' },
+  { role: 'cli', bundled: false, platform: 'node' },
+  { role: 'web-library', bundled: false, platform: 'web' },
+  { role: 'node-library', bundled: false, platform: 'node' },
+  { role: 'common-library', bundled: false, platform: 'common' },
+  { role: 'plugin-frontend', bundled: false, platform: 'web' },
+  { role: 'plugin-frontend-module', bundled: false, platform: 'web' },
+  { role: 'plugin-backend', bundled: false, platform: 'node' },
+  { role: 'plugin-backend-module', bundled: false, platform: 'node' },
 ];
-const roleMap = Object.fromEntries(packageRoles.map(i => [i.role, i]));
 
 export function getRoleInfo(role: string): PackageRoleInfo {
-  const roleInfo = packageRoles.find(r => r.role === role);
+  const roleInfo = packageRoleInfos.find(r => r.role === role);
   if (!roleInfo) {
     throw new Error(`Unknown package role '${role}'`);
   }
@@ -51,7 +50,7 @@ const readSchema = z.object({
     .optional(),
 });
 
-export function readPackageRole(pkgJson: unknown): PackageRoleInfo | undefined {
+export function getRoleFromPackage(pkgJson: unknown): PackageRole | undefined {
   const pkg = readSchema.parse(pkgJson);
 
   // If there's an explicit role, use that.
@@ -63,21 +62,19 @@ export function readPackageRole(pkgJson: unknown): PackageRoleInfo | undefined {
       );
     }
 
-    return getRoleInfo(role);
+    return getRoleInfo(role).role;
   }
 
   return undefined;
 }
 
-export async function readRoleForCommand(
-  cmd: Command,
-): Promise<PackageRoleInfo> {
+export async function findRoleFromCommand(cmd: Command): Promise<PackageRole> {
   if (cmd.role) {
-    return getRoleInfo(cmd.role);
+    return getRoleInfo(cmd.role)?.role;
   }
 
   const pkg = await fs.readJson(paths.resolveTarget('package.json'));
-  const info = readPackageRole(pkg);
+  const info = getRoleFromPackage(pkg);
   if (!info) {
     throw new Error(`Target package must have 'backstage.role' set`);
   }
@@ -104,28 +101,28 @@ const detectionSchema = z.object({
   module: z.string().optional(),
 });
 
-export function detectPackageRole(
+export function detectRoleFromPackage(
   pkgJson: unknown,
-): PackageRoleInfo | undefined {
+): PackageRole | undefined {
   const pkg = detectionSchema.parse(pkgJson);
 
   if (pkg.scripts?.start?.includes('app:serve')) {
-    return roleMap.app;
+    return 'app';
   }
   if (pkg.scripts?.build?.includes('backend:bundle')) {
-    return roleMap.backend;
+    return 'backend';
   }
   if (pkg.name?.includes('plugin-') && pkg.name?.includes('-backend-module-')) {
-    return roleMap['plugin-backend-module'];
+    return 'plugin-backend-module';
   }
   if (pkg.name?.includes('plugin-') && pkg.name?.includes('-module-')) {
-    return roleMap['plugin-frontend-module'];
+    return 'plugin-frontend-module';
   }
   if (pkg.scripts?.start?.includes('plugin:serve')) {
-    return roleMap['plugin-frontend'];
+    return 'plugin-frontend';
   }
   if (pkg.scripts?.start?.includes('backend:dev')) {
-    return roleMap['plugin-backend'];
+    return 'plugin-backend';
   }
 
   const mainEntry = pkg.publishConfig?.main || pkg.main;
@@ -133,16 +130,16 @@ export function detectPackageRole(
   const typesEntry = pkg.publishConfig?.types || pkg.types;
   if (typesEntry) {
     if (mainEntry && moduleEntry) {
-      return roleMap['common-library'];
+      return 'common-library';
     }
     if (moduleEntry || mainEntry?.endsWith('.esm.js')) {
-      return roleMap['web-library'];
+      return 'web-library';
     }
     if (mainEntry) {
-      return roleMap['node-library'];
+      return 'node-library';
     }
   } else if (mainEntry) {
-    return roleMap.cli;
+    return 'cli';
   }
 
   return undefined;

packages/cli/src/lib/role/types.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-export type PackageRoleName =
+export type PackageRole =
   | 'app'
   | 'backend'
   | 'cli'
@@ -29,6 +29,7 @@ export type PackageRoleName =
 export type PackagePlatform = 'node' | 'web' | 'common';
 
 export interface PackageRoleInfo {
-  role: PackageRoleName;
+  role: PackageRole;
+  bundled: boolean;
   platform: PackagePlatform;
 }