From f2c5b7461773a2c9ced713c9bc3c86a83c912588 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 24 Jan 2022 00:34:20 +0100 Subject: [PATCH] cli: refactor role functions to work around simpler PackageRole Signed-off-by: Patrik Oldsberg --- packages/cli/src/commands/build/command.ts | 13 +- packages/cli/src/commands/bundle/command.ts | 12 +- .../cli/src/commands/migrate/packageRole.ts | 12 +- .../src/commands/migrate/packageScripts.ts | 14 +-- packages/cli/src/commands/start/command.ts | 8 +- packages/cli/src/lib/monorepo/PackageGraph.ts | 4 +- packages/cli/src/lib/role/index.ts | 12 +- .../cli/src/lib/role/packageRoles.test.ts | 111 ++++++------------ packages/cli/src/lib/role/packageRoles.ts | 63 +++++----- packages/cli/src/lib/role/types.ts | 5 +- 10 files changed, 106 insertions(+), 148 deletions(-) diff --git a/packages/cli/src/commands/build/command.ts b/packages/cli/src/commands/build/command.ts index 733faa9480..5db6baa34f 100644 --- a/packages/cli/src/commands/build/command.ts +++ b/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 { - 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 { if (esmPlatforms.includes(roleInfo.platform)) { outputs.add(Output.esm); } - if (roleInfo.role !== 'cli') { + if (role !== 'cli') { outputs.add(Output.types); } diff --git a/packages/cli/src/commands/bundle/command.ts b/packages/cli/src/commands/bundle/command.ts index 44b39e6797..4b8c718001 100644 --- a/packages/cli/src/commands/bundle/command.ts +++ b/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 { - 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 { 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}'`); } diff --git a/packages/cli/src/commands/migrate/packageRole.ts b/packages/cli/src/commands/migrate/packageRole.ts index 86898a6012..e8bfb8e7c8 100644 --- a/packages/cli/src/commands/migrate/packageRole.ts +++ b/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); } diff --git a/packages/cli/src/commands/migrate/packageScripts.ts b/packages/cli/src/commands/migrate/packageScripts.ts index 5d0033606f..c54d4ebb31 100644 --- a/packages/cli/src/commands/migrate/packageScripts.ts +++ b/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' }), diff --git a/packages/cli/src/commands/start/command.ts b/packages/cli/src/commands/start/command.ts index ff4d3b7c0b..6615d5fd46 100644 --- a/packages/cli/src/commands/start/command.ts +++ b/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 { - 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 { 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 { 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}'`, ); } } diff --git a/packages/cli/src/lib/monorepo/PackageGraph.ts b/packages/cli/src/lib/monorepo/PackageGraph.ts index db7a528494..89d5f79cd2 100644 --- a/packages/cli/src/lib/monorepo/PackageGraph.ts +++ b/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; }; } diff --git a/packages/cli/src/lib/role/index.ts b/packages/cli/src/lib/role/index.ts index 9becfa367b..a01be8d11e 100644 --- a/packages/cli/src/lib/role/index.ts +++ b/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'; diff --git a/packages/cli/src/lib/role/packageRoles.test.ts b/packages/cli/src/lib/role/packageRoles.test.ts index f3302d3256..db79143e50 100644 --- a/packages/cli/src/lib/role/packageRoles.test.ts +++ b/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 ', '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'); }); }); diff --git a/packages/cli/src/lib/role/packageRoles.ts b/packages/cli/src/lib/role/packageRoles.ts index 67bee0e64f..c12ca644b9 100644 --- a/packages/cli/src/lib/role/packageRoles.ts +++ b/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 { +export async function findRoleFromCommand(cmd: Command): Promise { 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; diff --git a/packages/cli/src/lib/role/types.ts b/packages/cli/src/lib/role/types.ts index 1f5afe7926..efe2c99fd5 100644 --- a/packages/cli/src/lib/role/types.ts +++ b/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; }