From b35c1770cf61beb8177314db8510092282da7852 Mon Sep 17 00:00:00 2001 From: Juan Pablo Garcia Ripa Date: Mon, 5 Dec 2022 19:25:40 +0100 Subject: [PATCH] change path argument to options and allow csv and glob for paths Signed-off-by: Juan Pablo Garcia Ripa --- .changeset/lemon-coats-camp.md | 7 +- package.json | 2 +- packages/repo-tools/cli-report.md | 9 +- packages/repo-tools/package.json | 2 + .../src/commands/api-reports/api-extractor.ts | 40 ++---- .../src/commands/api-reports/api-reports.ts | 122 ++++++++++++------ packages/repo-tools/src/commands/index.ts | 14 +- packages/repo-tools/src/lib/paths.test.ts | 51 +++++++- packages/repo-tools/src/lib/paths.ts | 25 ++++ yarn.lock | 2 + 10 files changed, 187 insertions(+), 87 deletions(-) diff --git a/.changeset/lemon-coats-camp.md b/.changeset/lemon-coats-camp.md index a2d87476f6..ae31098e2f 100644 --- a/.changeset/lemon-coats-camp.md +++ b/.changeset/lemon-coats-camp.md @@ -4,8 +4,9 @@ Add new command options to the `api-report` -- added `--allowWarnings` to continue processing packages if some packages have warnings -- added `--omitMessages` to pass some warnings messages code to be omitted from the api-report.md files +- added `--allow-warnings`, `-a` to continue processing packages if some packages have warnings +- added `--omit-messages`, `-o` to pass some warnings messages code to be omitted from the api-report.md files +- added `--paths`, `-p` to select packages path to process - The `paths` argument for this command now takes as default the value on `workspaces.packages` inside the root package.json -- The `paths` argument now allow glob patterns +- Removed the `paths` argument replaced by the option `--paths` - change the path resolution to use the `@backstage/cli-common` packages instead diff --git a/package.json b/package.json index 497a652cdf..5182436762 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,7 @@ "build:backend": "yarn workspace backend build", "build:all": "backstage-cli repo build --all", "build:api-reports": "yarn build:api-reports:only --tsc", - "build:api-reports:only": "backstage-repo-tools api-reports --allow-warnings packages/core-components plugins/catalog plugins/catalog-import plugins/git-release-manager plugins/jenkins plugins/kubernetes", + "build:api-reports:only": "backstage-repo-tools api-reports --allow-warnings 'packages/core-components,plugins/+(catalog|catalog-import|git-release-manager|jenkins|kubernetes)'", "build:api-docs": "LANG=en_EN yarn build:api-reports --docs", "tsc": "tsc", "tsc:full": "backstage-cli repo clean && tsc --skipLibCheck false --incremental false", diff --git a/packages/repo-tools/cli-report.md b/packages/repo-tools/cli-report.md index ba15ecc674..d3446e0262 100644 --- a/packages/repo-tools/cli-report.md +++ b/packages/repo-tools/cli-report.md @@ -12,7 +12,7 @@ Options: -h, --help Commands: - api-reports [options] [paths...] + api-reports [options] type-deps help [command] ``` @@ -20,14 +20,15 @@ Commands: ### `backstage-repo-tools api-reports` ``` -Usage: backstage-repo-tools api-reports [options] [paths...] +Usage: backstage-repo-tools api-reports [options] Options: + -p --paths [paths...] --ci --tsc --docs - --allow-warnings [allowWarningsPaths...] - --omitMessages + -a, --allow-warnings [allowWarningsPaths...] + -o, --omit-messages -h, --help ``` diff --git a/packages/repo-tools/package.json b/packages/repo-tools/package.json index 9b51d35922..6025af9bce 100644 --- a/packages/repo-tools/package.json +++ b/packages/repo-tools/package.json @@ -40,7 +40,9 @@ "chalk": "^4.0.0", "commander": "^9.1.0", "fs-extra": "10.1.0", + "glob": "^8.0.3", "is-glob": "^4.0.3", + "minimatch": "^5.1.1", "ts-node": "^10.0.0" }, "devDependencies": { diff --git a/packages/repo-tools/src/commands/api-reports/api-extractor.ts b/packages/repo-tools/src/commands/api-reports/api-extractor.ts index b2fab40ca5..46f4ea984b 100644 --- a/packages/repo-tools/src/commands/api-reports/api-extractor.ts +++ b/packages/repo-tools/src/commands/api-reports/api-extractor.ts @@ -65,14 +65,9 @@ import { } from '@microsoft/api-documenter/lib/markdown/CustomMarkdownEmitter'; import { IMarkdownEmitterContext } from '@microsoft/api-documenter/lib/markdown/MarkdownEmitter'; import { AstDeclaration } from '@microsoft/api-extractor/lib/analyzer/AstDeclaration'; -import { paths as cliPaths, resolvePackagePath } from '../../lib/paths'; +import { paths as cliPaths } from '../../lib/paths'; -import g from 'glob'; -import isGlob from 'is-glob'; - -import { promisify } from 'util'; - -const glob = promisify(g); +import minimatch from 'minimatch'; const tmpDir = cliPaths.resolveTargetRoot( './node_modules/.cache/api-extractor', @@ -226,24 +221,6 @@ ApiReportGenerator.generateReviewFileContent = }); }; -export async function findPackageDirs(selectedPaths: string[]) { - const packageDirs = new Array(); - for (const packageRoot of selectedPaths) { - const fullPath = cliPaths.resolveTargetRoot(packageRoot); - - // if the path contain any glob notation we resolve all the paths to process one by one - const dirs = isGlob(fullPath) ? await glob(fullPath) : [fullPath]; - for (const dir of dirs) { - const packageDir = await resolvePackagePath(dir); - if (!packageDir) { - continue; - } - packageDirs.push(packageDir); - } - } - return packageDirs; -} - export async function createTemporaryTsConfig(includedPackageDirs: string[]) { const path = cliPaths.resolveTargetRoot('tsconfig.tmp.json'); @@ -331,7 +308,7 @@ interface ApiExtractionOptions { outputDir: string; isLocalBuild: boolean; tsconfigFilePath: string; - allowWarnings: boolean | string[]; + allowWarnings?: boolean | string[]; omitMessages?: string[]; } @@ -340,7 +317,7 @@ export async function runApiExtraction({ outputDir, isLocalBuild, tsconfigFilePath, - allowWarnings, + allowWarnings = false, omitMessages = [], }: ApiExtractionOptions) { await fs.remove(outputDir); @@ -366,7 +343,7 @@ export async function runApiExtraction({ for (const packageDir of packageDirs) { console.log(`## Processing ${packageDir}`); const noBail = Array.isArray(allowWarnings) - ? allowWarnings.includes(packageDir) + ? allowWarnings.some(aw => aw === packageDir || minimatch(packageDir, aw)) : allowWarnings; const projectFolder = cliPaths.resolveTargetRoot(packageDir); @@ -514,6 +491,9 @@ export async function runApiExtraction({ } const warningCountAfter = await countApiReportWarnings(projectFolder); + if (noBail) { + console.log(`Skipping warnings check for ${packageDir}`); + } if (warningCountAfter > 0 && !noBail) { throw new Error( `The API Report for ${packageDir} is not allowed to have warnings`, @@ -1289,13 +1269,11 @@ function generateCliReport(name: string, models: CliModel[]): string { } interface CliExtractionOptions { - projectRoot: string; packageDirs: string[]; isLocalBuild: boolean; } export async function runCliExtraction({ - projectRoot, packageDirs, isLocalBuild, }: CliExtractionOptions) { @@ -1344,7 +1322,7 @@ export async function runCliExtraction({ console.log(''); console.log( `The conflicting file is ${relativePath( - projectRoot, + cliPaths.targetRoot, reportPath, )}, expecting the following content:`, ); diff --git a/packages/repo-tools/src/commands/api-reports/api-reports.ts b/packages/repo-tools/src/commands/api-reports/api-reports.ts index 9b0d78d6a8..11b65823c7 100644 --- a/packages/repo-tools/src/commands/api-reports/api-reports.ts +++ b/packages/repo-tools/src/commands/api-reports/api-reports.ts @@ -15,49 +15,42 @@ */ import { OptionValues } from 'commander'; -import { resolve as resolvePath } from 'path'; import fs from 'fs-extra'; import { spawnSync } from 'child_process'; import { createTemporaryTsConfig, - findPackageDirs, categorizePackageDirs, runApiExtraction, runCliExtraction, buildDocs, } from './api-extractor'; -import { paths as cliPaths } from '../../lib/paths'; +import { findPackageDirs, paths as cliPaths } from '../../lib/paths'; -export default async (paths: string[], opts: OptionValues) => { - const tmpDir = resolvePath( - cliPaths.targetRoot, +export default async (opts: OptionValues) => { + const tmpDir = cliPaths.resolveTargetRoot( './node_modules/.cache/api-extractor', ); - const projectRoot = resolvePath(cliPaths.targetRoot); const isCiBuild = opts.ci; const isDocsBuild = opts.docs; const runTsc = opts.tsc; - const selectedPaths = paths.length ? paths : await getWorkspacePkgs(); - const allowWarnings: boolean | string[] = opts.allowWarnings; - const omitMessages = opts.omitMessages; + const parsedPaths = parseArrayOption(opts.paths); + const isAllPackages = !Array.isArray(parsedPaths) || !parsedPaths?.length; + const selectedPaths = isAllPackages ? await getWorkspacePkgs() : parsedPaths; const selectedPackageDirs = await findPackageDirs(selectedPaths); - if (paths.length && isCiBuild) { - // TODO @sarabadu we can remove this validation to allow `/plugins/*` on CI?? - throw new Error( - 'Package path arguments are not supported together with the --ci flag', - ); - } - if (!paths.length && !isCiBuild && !isDocsBuild) { + const allowWarnings = parseArrayOption(opts.allowWarnings); + const omitMessages = parseArrayOption(opts.omitMessages); + + if (isAllPackages && !isCiBuild && !isDocsBuild) { console.log(''); console.log( 'TIP: You can generate api-reports for select packages by passing package paths:', ); console.log(''); console.log( - ' yarn build:api-reports packages/config packages/core-plugin-api plugins/*', + ' yarn build:api-reports -p packages/config -p packages/core-plugin-api,plugins/*', ); console.log(''); } @@ -67,27 +60,11 @@ export default async (paths: string[], opts: OptionValues) => { temporaryTsConfigPath = await createTemporaryTsConfig(selectedPackageDirs); } const tsconfigFilePath = - temporaryTsConfigPath ?? resolvePath(projectRoot, 'tsconfig.json'); + temporaryTsConfigPath ?? cliPaths.resolveTargetRoot('tsconfig.json'); if (runTsc) { - await fs.remove(resolvePath(projectRoot, 'dist-types')); - const { status } = spawnSync( - 'yarn', - [ - 'tsc', - ['--project', tsconfigFilePath], - ['--skipLibCheck', 'false'], - ['--incremental', 'false'], - ].flat(), - { - stdio: 'inherit', - shell: true, - cwd: projectRoot, - }, - ); - if (status !== 0) { - process.exit(status || undefined); - } + console.log('# Compiling TypeScript'); + await generateTSC(tsconfigFilePath); } const { tsPackageDirs, cliPackageDirs } = await categorizePackageDirs( @@ -102,13 +79,12 @@ export default async (paths: string[], opts: OptionValues) => { isLocalBuild: !isCiBuild, tsconfigFilePath, allowWarnings, - omitMessages, + omitMessages: Array.isArray(omitMessages) ? omitMessages : [], }); } if (cliPackageDirs.length > 0) { console.log('# Generating package CLI reports'); await runCliExtraction({ - projectRoot, packageDirs: cliPackageDirs, isLocalBuild: !isCiBuild, }); @@ -118,10 +94,49 @@ export default async (paths: string[], opts: OptionValues) => { console.log('# Generating package documentation'); await buildDocs({ inputDir: tmpDir, - outputDir: resolvePath(projectRoot, 'docs/reference'), + outputDir: cliPaths.resolveTargetRoot('docs/reference'), }); } }; + +/** + * Generates the TypeScript declaration files for the specified project, using the provided `tsconfig.json` file. + * + * Any existing declaration files in the `dist-types` directory will be deleted before generating the new ones. + * + * If the `tsc` command exits with a non-zero exit code, the process will be terminated with the same exit code. + * + * @param tsconfigFilePath {string} The path to the `tsconfig.json` file to use for generating the declaration files. + * @returns {Promise} A promise that resolves when the declaration files have been generated. + */ +export async function generateTSC(tsconfigFilePath: string) { + await fs.remove(cliPaths.resolveTargetRoot('dist-types')); + const { status } = spawnSync( + 'yarn', + [ + 'tsc', + ['--project', tsconfigFilePath], + ['--skipLibCheck', 'false'], + ['--incremental', 'false'], + ].flat(), + { + stdio: 'inherit', + shell: true, + cwd: cliPaths.targetRoot, + }, + ); + if (status !== 0) { + process.exit(status || undefined); + } +} + +/** + * Retrieves the list of package names in the "workspaces" field of the `package.json` file in the current workspace root. + * + * If the file does not exist, or the "workspaces" field is not present, returns `undefined`. + * + * @returns {Promise} The list of package names, or `undefined` if not found. + */ async function getWorkspacePkgs() { const pkgJson = await fs .readJson(cliPaths.resolveTargetRoot('package.json')) @@ -134,3 +149,30 @@ async function getWorkspacePkgs() { const workspaces = pkgJson?.workspaces?.packages; return workspaces; } + +/** + * Splits each string in the input array on comma, and returns an array of the resulting substrings. + * If the input array is `undefined`, returns `undefined`. If the input value is `true` or `false`, + * returns the value as-is. + * + * @param value An array of strings to be split on comma, or a boolean value (inherithed from commanderjs array args). + * @returns An array of the resulting substrings, the original boolean value, or `undefined` if the input value is `undefined`. + * + * @example + * parseOption(['foo,bar,baz']) + * // returns ['foo', 'bar', 'baz'] + * + * parseOption(true) + * // returns true + * + * parseOption() + * // returns undefined + */ +function parseArrayOption(value: string[] | boolean | undefined) { + if (typeof value === 'boolean') { + return value; + } + return value?.flatMap((str: string) => + str.includes(',') ? str.split(',') : str, + ); +} diff --git a/packages/repo-tools/src/commands/index.ts b/packages/repo-tools/src/commands/index.ts index bbc6773f83..5742ce7241 100644 --- a/packages/repo-tools/src/commands/index.ts +++ b/packages/repo-tools/src/commands/index.ts @@ -21,21 +21,21 @@ import { exitWithError } from '../lib/errors'; export function registerCommands(program: Command) { program .command('api-reports') - .argument( - '[paths...]', - 'path of package folder to extract API reports, `workspaces.packages` from root packages.json by default', + .option( + '-p --paths [paths...]', + 'paths of package folder to extract API reports, `workspaces.packages` from root packages.json by default. Allows glob patterns and comma separated values', ) .option('--ci', 'CI run checks that there is no changes on API reports') .option('--tsc', 'executes the tsc compilation before extracting the APIs') .option('--docs', 'generates the api documentation') .option( - '--allow-warnings [allowWarningsPaths...]', - 'continue processing packages after getting errors on selected packages', + '-a, --allow-warnings [allowWarningsPaths...]', + 'continue processing packages after getting errors on selected packages Allows glob patterns and comma separated values (i.e. packages/core,plugins/core-*)', false, ) .option( - '--omitMessages ', - 'select some message code to be omited on the API Extractor (i.e ae-cyclic-inherit-doc)', + '-o, --omit-messages ', + 'select some message code to be omited on the API Extractor (comma separated values i.e ae-cyclic-inherit-doc,ae-missing-getter )', ) .description('Generate an API report for selected packages') .action( diff --git a/packages/repo-tools/src/lib/paths.test.ts b/packages/repo-tools/src/lib/paths.test.ts index 90d84aec3c..c642900612 100644 --- a/packages/repo-tools/src/lib/paths.test.ts +++ b/packages/repo-tools/src/lib/paths.test.ts @@ -16,7 +16,7 @@ import mockFs from 'mock-fs'; import { resolve as resolvePath } from 'path'; -import { resolvePackagePath, paths } from './paths'; +import { resolvePackagePath, paths, findPackageDirs } from './paths'; describe('paths', () => { jest.spyOn(paths, 'targetRoot', 'get').mockReturnValue('/root'); @@ -38,6 +38,14 @@ describe('paths', () => { 'package-c': {}, 'README.md': 'Hello World', }, + plugins: { + 'plugin-a': { + 'package.json': '{}', + }, + 'plugin-b': { + 'package.json': '{}', + }, + }, }, }); }); @@ -59,8 +67,49 @@ describe('paths', () => { 'packages/package-b', ); }); + it('should work with absolute paths', async () => { + expect(await resolvePackagePath('/root/packages/package-a')).toBe( + 'packages/package-a', + ); + }); it('should return undefined if the pat is not a directory', async () => { expect(await resolvePackagePath('packages/README.md')).toBeUndefined(); }); }); + describe('findPackageDirs', () => { + it('should return only the given packages', async () => { + expect(await findPackageDirs(['packages/package-a'])).toEqual([ + 'packages/package-a', + ]); + }); + it('should return only the given packages when using glob patterns', async () => { + expect(await findPackageDirs(['packages/*'])).toEqual([ + 'packages/package-a', + 'packages/package-b', + ]); + expect(await findPackageDirs(['packages/*', 'plugins/*'])).toEqual([ + 'packages/package-a', + 'packages/package-b', + 'plugins/plugin-a', + 'plugins/plugin-b', + ]); + }); + it('should return only the given packages when using absolute paths', async () => { + expect( + await findPackageDirs([ + '/root/packages/package-a', + '/root/plugins/plugin-b', + ]), + ).toEqual(['packages/package-a', 'plugins/plugin-b']); + }); + it('should return only the given packages when using absolute paths with glob patterns', async () => { + expect( + await findPackageDirs(['/root/packages/*', '/root/plugins/*-a']), + ).toEqual([ + 'packages/package-a', + 'packages/package-b', + 'plugins/plugin-a', + ]); + }); + }); }); diff --git a/packages/repo-tools/src/lib/paths.ts b/packages/repo-tools/src/lib/paths.ts index f387e9be51..874bb48ea1 100644 --- a/packages/repo-tools/src/lib/paths.ts +++ b/packages/repo-tools/src/lib/paths.ts @@ -18,6 +18,13 @@ import { findPaths } from '@backstage/cli-common'; import { relative as relativePath, join } from 'path'; import fs from 'fs-extra'; +import g from 'glob'; +import isGlob from 'is-glob'; + +import { promisify } from 'util'; + +const glob = promisify(g); + /* eslint-disable-next-line no-restricted-syntax */ export const paths = findPaths(__dirname); @@ -41,3 +48,21 @@ export async function resolvePackagePath( } return relativePath(paths.targetRoot, fullPackageDir); } + +export async function findPackageDirs(selectedPaths: string[] = []) { + const packageDirs = new Array(); + for (const packageRoot of selectedPaths) { + const fullPath = paths.resolveTargetRoot(packageRoot); + + // if the path contain any glob notation we resolve all the paths to process one by one + const dirs = isGlob(fullPath) ? await glob(fullPath) : [fullPath]; + for (const dir of dirs) { + const packageDir = await resolvePackagePath(dir); + if (!packageDir) { + continue; + } + packageDirs.push(packageDir); + } + } + return packageDirs; +} diff --git a/yarn.lock b/yarn.lock index e92df381ca..cc9dcb0642 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8480,7 +8480,9 @@ __metadata: chalk: ^4.0.0 commander: ^9.1.0 fs-extra: 10.1.0 + glob: ^8.0.3 is-glob: ^4.0.3 + minimatch: ^5.1.1 mock-fs: ^5.1.0 ts-node: ^10.0.0 bin: