From 16689f4d503040c7d5f15e3da2b2e9d31b679e0d Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Thu, 21 Nov 2024 11:43:23 +0900 Subject: [PATCH 1/7] Fix api-reports space-after-comma bug Signed-off-by: Juan Escalada --- .../repo-tools/src/commands/api-reports/api-reports.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) 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 637cd86671..0603188dac 100644 --- a/packages/repo-tools/src/commands/api-reports/api-reports.ts +++ b/packages/repo-tools/src/commands/api-reports/api-reports.ts @@ -45,7 +45,15 @@ export const buildApiReports = async (paths: string[] = [], opts: Options) => { const runTsc = opts.tsc; const allowWarnings = parseArrayOption(opts.allowWarnings); const allowAllWarnings = opts.allowAllWarnings; - const omitMessages = parseArrayOption(opts.omitMessages); + const omitMessagesRaw = opts.omitMessages; + + if (omitMessagesRaw && omitMessagesRaw.endsWith(',')) { + throw new Error( + `Invalid value for --omit-messages: ${omitMessagesRaw}\nMust be a comma-separated list of strings without spaces or wrapped in quotations.`, + ); + } + + const omitMessages = parseArrayOption(omitMessagesRaw); const isAllPackages = !paths?.length; const selectedPackageDirs = await resolvePackagePaths({ From 5e25365997abdd5460cb6d7d98365dc247b734f0 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Thu, 21 Nov 2024 11:56:05 +0900 Subject: [PATCH 2/7] Add unit test for checking invalid comma parsing Signed-off-by: Juan Escalada --- .../src/commands/api-reports/api-reports.test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/repo-tools/src/commands/api-reports/api-reports.test.ts b/packages/repo-tools/src/commands/api-reports/api-reports.test.ts index 955720c52e..6401f64439 100644 --- a/packages/repo-tools/src/commands/api-reports/api-reports.test.ts +++ b/packages/repo-tools/src/commands/api-reports/api-reports.test.ts @@ -473,6 +473,17 @@ describe('buildApiReports', () => { outputDir: mockDir.resolve('node_modules/.cache/api-extractor'), }); }); + + it('should throw an error if omitMessages ends with a comma', async () => { + const opts = { + omitMessages: 'ae-missing-release-tag,', + }; + const paths = ['packages/*']; + + await expect(buildApiReports(paths, opts)).rejects.toThrow( + `Invalid value for --omit-messages: ${opts.omitMessages}\nMust be a comma-separated list of strings without spaces or wrapped in quotations.`, + ); + }); }); describe('isCI', () => { it('should set localBuild to false if CI option is passed', async () => { From c1eccd6220b4664128faa29129afcca23c48e6a8 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Thu, 21 Nov 2024 12:00:00 +0900 Subject: [PATCH 3/7] Add changeset Signed-off-by: Juan Escalada --- .changeset/new-pandas-perform.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/new-pandas-perform.md diff --git a/.changeset/new-pandas-perform.md b/.changeset/new-pandas-perform.md new file mode 100644 index 0000000000..d306e8cf0c --- /dev/null +++ b/.changeset/new-pandas-perform.md @@ -0,0 +1,5 @@ +--- +'@backstage/repo-tools': minor +--- + +Fix malformed -o flag bug in api-reports.ts From 58ec127c52256bda7dd2829e73feda2b79ea7445 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Tue, 26 Nov 2024 11:18:31 +0900 Subject: [PATCH 4/7] Add path validation and remove -o flag check Signed-off-by: Juan Escalada --- .../src/commands/api-reports/api-reports.ts | 39 ++++++++++++++----- 1 file changed, 29 insertions(+), 10 deletions(-) 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 0603188dac..b8b9f9fb23 100644 --- a/packages/repo-tools/src/commands/api-reports/api-reports.ts +++ b/packages/repo-tools/src/commands/api-reports/api-reports.ts @@ -24,6 +24,7 @@ import { } from './api-extractor'; import { paths as cliPaths, resolvePackagePaths } from '../../lib/paths'; import { generateTypeDeclarations } from './generateTypeDeclarations'; +import fs from 'fs/promises'; type Options = { ci?: boolean; @@ -45,17 +46,14 @@ export const buildApiReports = async (paths: string[] = [], opts: Options) => { const runTsc = opts.tsc; const allowWarnings = parseArrayOption(opts.allowWarnings); const allowAllWarnings = opts.allowAllWarnings; - const omitMessagesRaw = opts.omitMessages; - - if (omitMessagesRaw && omitMessagesRaw.endsWith(',')) { - throw new Error( - `Invalid value for --omit-messages: ${omitMessagesRaw}\nMust be a comma-separated list of strings without spaces or wrapped in quotations.`, - ); - } - - const omitMessages = parseArrayOption(omitMessagesRaw); + const omitMessages = parseArrayOption(opts.omitMessages); const isAllPackages = !paths?.length; + + if (!isAllPackages) { + checkValidPaths(paths); + } + const selectedPackageDirs = await resolvePackagePaths({ paths, include: opts.include, @@ -138,5 +136,26 @@ export const buildApiReports = async (paths: string[] = [], opts: Options) => { * // returns [] */ function parseArrayOption(value: string | undefined) { - return value ? value.split(',').map(s => s.trim()) : []; + return value + ? value + .split(',') + .map(s => s.trim()) + .filter(Boolean) + : []; +} + +/** + * Checks if the provided paths are valid. + * + * @throws Error if any of the paths are invalid. + * @param paths An array of paths to check. + */ +function checkValidPaths(paths: string[]) { + paths.forEach(async path => { + try { + await fs.access(path); + } catch (error) { + throw new Error(`Invalid path provided: ${path}`); + } + }); } From da94fabdac542f26689e6c32e2d2a2a1f0ea6d47 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Tue, 26 Nov 2024 12:15:11 +0900 Subject: [PATCH 5/7] Move validation into resolvePackagePaths Signed-off-by: Juan Escalada --- .../src/commands/api-reports/api-reports.ts | 21 ------------------- packages/repo-tools/src/lib/paths.ts | 17 +++++++++++++++ 2 files changed, 17 insertions(+), 21 deletions(-) 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 b8b9f9fb23..4e568102d9 100644 --- a/packages/repo-tools/src/commands/api-reports/api-reports.ts +++ b/packages/repo-tools/src/commands/api-reports/api-reports.ts @@ -24,7 +24,6 @@ import { } from './api-extractor'; import { paths as cliPaths, resolvePackagePaths } from '../../lib/paths'; import { generateTypeDeclarations } from './generateTypeDeclarations'; -import fs from 'fs/promises'; type Options = { ci?: boolean; @@ -50,10 +49,6 @@ export const buildApiReports = async (paths: string[] = [], opts: Options) => { const isAllPackages = !paths?.length; - if (!isAllPackages) { - checkValidPaths(paths); - } - const selectedPackageDirs = await resolvePackagePaths({ paths, include: opts.include, @@ -143,19 +138,3 @@ function parseArrayOption(value: string | undefined) { .filter(Boolean) : []; } - -/** - * Checks if the provided paths are valid. - * - * @throws Error if any of the paths are invalid. - * @param paths An array of paths to check. - */ -function checkValidPaths(paths: string[]) { - paths.forEach(async path => { - try { - await fs.access(path); - } catch (error) { - throw new Error(`Invalid path provided: ${path}`); - } - }); -} diff --git a/packages/repo-tools/src/lib/paths.ts b/packages/repo-tools/src/lib/paths.ts index 96fc92f035..0dcec32ff1 100644 --- a/packages/repo-tools/src/lib/paths.ts +++ b/packages/repo-tools/src/lib/paths.ts @@ -36,6 +36,23 @@ export async function resolvePackagePaths( const { paths: providedPaths, include, exclude } = options; let packages = await PackageGraph.listTargetPackages(); + if (providedPaths && providedPaths.length > 0) { + const invalidPaths: string[] = []; + for (const path of providedPaths) { + const matches = packages.some( + ({ dir }) => + new Minimatch(path).match(relativePath(paths.targetRoot, dir)) || + isChildPath(dir, path), + ); + if (!matches) { + invalidPaths.push(path); + } + } + if (invalidPaths.length > 0) { + throw new Error(`Invalid paths provided: ${invalidPaths.join(', ')}`); + } + } + if (providedPaths && providedPaths.length > 0) { packages = packages.filter(({ dir }) => providedPaths.some( From 6d6402a9748563c238187ab873fbdcb7395ccaa0 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Tue, 26 Nov 2024 12:17:58 +0900 Subject: [PATCH 6/7] Fix tests Signed-off-by: Juan Escalada --- .../commands/api-reports/api-reports.test.ts | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/packages/repo-tools/src/commands/api-reports/api-reports.test.ts b/packages/repo-tools/src/commands/api-reports/api-reports.test.ts index 6401f64439..94ae42d715 100644 --- a/packages/repo-tools/src/commands/api-reports/api-reports.test.ts +++ b/packages/repo-tools/src/commands/api-reports/api-reports.test.ts @@ -323,6 +323,24 @@ describe('buildApiReports', () => { expect(buildDocs).not.toHaveBeenCalled(); }); + it('should throw an error if a path does not exist', async () => { + const paths = ['packages/package-a', 'packages/package-c']; + const opts = {}; + + await expect(buildApiReports(paths, opts)).rejects.toThrow( + 'Invalid paths provided: packages/package-c', + ); + }); + it('should throw an error if an option is malformed', async () => { + const paths = ['ae-undocumented']; + const opts = { + omitMessages: 'ae-wrong-input-file-type,', + }; + + await expect(buildApiReports(paths, opts)).rejects.toThrow( + 'Invalid paths provided: ae-undocumented', + ); + }); }); describe('allowWarnings', () => { it('should accept single path value', async () => { @@ -473,17 +491,6 @@ describe('buildApiReports', () => { outputDir: mockDir.resolve('node_modules/.cache/api-extractor'), }); }); - - it('should throw an error if omitMessages ends with a comma', async () => { - const opts = { - omitMessages: 'ae-missing-release-tag,', - }; - const paths = ['packages/*']; - - await expect(buildApiReports(paths, opts)).rejects.toThrow( - `Invalid value for --omit-messages: ${opts.omitMessages}\nMust be a comma-separated list of strings without spaces or wrapped in quotations.`, - ); - }); }); describe('isCI', () => { it('should set localBuild to false if CI option is passed', async () => { From ce46791e2a428fd3e6de75c5aafdc48d2117ffe8 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Tue, 26 Nov 2024 12:19:34 +0900 Subject: [PATCH 7/7] Update changeset Signed-off-by: Juan Escalada --- .changeset/new-pandas-perform.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/new-pandas-perform.md b/.changeset/new-pandas-perform.md index d306e8cf0c..797f34d6df 100644 --- a/.changeset/new-pandas-perform.md +++ b/.changeset/new-pandas-perform.md @@ -2,4 +2,4 @@ '@backstage/repo-tools': minor --- -Fix malformed -o flag bug in api-reports.ts +Fix invalid path and malformed flags bugs in api-reports.ts