diff --git a/.changeset/hip-cars-repair.md b/.changeset/hip-cars-repair.md new file mode 100644 index 0000000000..de46860b03 --- /dev/null +++ b/.changeset/hip-cars-repair.md @@ -0,0 +1,11 @@ +--- +'@backstage/repo-tools': patch +--- + +The `api-reports` command now checks for api report files that no longer apply. +If it finds such files, it's treated basically the same as report errors do, and +the check fails. + +For example, if you had an `api-report-alpha.md` but then removed the alpha +export, the reports generator would now report that this file needs to be +deleted. 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 c978e49300..8f7a298764 100644 --- a/packages/repo-tools/src/commands/api-reports/api-extractor.ts +++ b/packages/repo-tools/src/commands/api-reports/api-extractor.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { groupBy } from 'lodash'; import { resolve as resolvePath, relative as relativePath, @@ -363,7 +364,9 @@ export async function runApiExtraction({ } const warnings = new Array(); - for (const { packageDir, name } of packageEntryPoints) { + for (const [packageDir, group] of Object.entries( + groupBy(packageEntryPoints, ep => ep.packageDir), + )) { console.log(`## Processing ${packageDir}`); const noBail = Array.isArray(allowWarnings) ? allowWarnings.some(aw => aw === packageDir || minimatch(packageDir, aw)) @@ -374,209 +377,237 @@ export async function runApiExtraction({ './dist-types', packageDir, ); + const names = group.map(ep => ep.name); - const suffix = name === 'index' ? '' : `-${name}`; - const reportFileName = `api-report${suffix}.md`; - const reportPath = resolvePath(projectFolder, reportFileName); - - const warningCountBefore = await countApiReportWarnings(reportPath); - - const extractorConfig = ExtractorConfig.prepare({ - configObject: { - mainEntryPointFilePath: resolvePath(packageFolder, `src/${name}.d.ts`), - bundledPackages: [], - - compiler: { - tsconfigFilePath, - }, - - apiReport: { - enabled: true, - reportFileName, - reportFolder: projectFolder, - reportTempFolder: resolvePath( - outputDir, - `${suffix}`, - ), - }, - - docModel: { - // TODO(Rugvip): This skips docs for non-index entry points. We can try to work around it, but - // most likely it makes sense to wait for API Extractor to natively support exports. - enabled: name === 'index', - apiJsonFilePath: resolvePath( - outputDir, - `${suffix}.api.json`, - ), - }, - - dtsRollup: { - enabled: false, - }, - - tsdocMetadata: { - enabled: false, - }, - - messages: { - // Silence compiler warnings, as these will prevent the CI build to work - compilerMessageReporting: { - default: { - logLevel: 'none' as ExtractorLogLevel.None, - // These contain absolute file paths, so can't be included in the report - // addToApiReportFile: true, - }, - }, - extractorMessageReporting: { - default: { - logLevel: 'warning' as ExtractorLogLevel.Warning, - addToApiReportFile: true, - }, - ...messagesConf, - }, - tsdocMessageReporting: { - default: { - logLevel: 'warning' as ExtractorLogLevel.Warning, - addToApiReportFile: true, - }, - }, - }, - - newlineKind: 'lf', - - projectFolder, - }, - configObjectFullPath: projectFolder, - packageJsonFullPath: resolvePath(projectFolder, 'package.json'), - tsdocConfigFile: await getTsDocConfig(), - ignoreMissingEntryPoint: true, + const staleReportFiles = fs.readdirSync(projectFolder).filter(filename => { + const match = + filename.match(/^(.+)-api-report\.md$/) || + filename.match(/^api-report-(.+)\.md$/); + return match && !names.includes(match[1]); }); - // The `packageFolder` needs to point to the location within `dist-types` in order for relative - // paths to be logged. Unfortunately the `prepare` method above derives it from the `packageJsonFullPath`, - // which needs to point to the actual file, so we override `packageFolder` afterwards. - ( - extractorConfig as { - packageFolder: string; - } - ).packageFolder = packageFolder; + for (const name of names) { + const suffix = name === 'index' ? '' : `-${name}`; + const reportFileName = `api-report${suffix}.md`; + const reportPath = resolvePath(projectFolder, reportFileName); - if (!compilerState) { - compilerState = CompilerState.create(extractorConfig, { - additionalEntryPoints: entryPoints, + const warningCountBefore = await countApiReportWarnings(reportPath); + + const extractorConfig = ExtractorConfig.prepare({ + configObject: { + mainEntryPointFilePath: resolvePath( + packageFolder, + `src/${name}.d.ts`, + ), + bundledPackages: [], + + compiler: { + tsconfigFilePath, + }, + + apiReport: { + enabled: true, + reportFileName, + reportFolder: projectFolder, + reportTempFolder: resolvePath( + outputDir, + `${suffix}`, + ), + }, + + docModel: { + // TODO(Rugvip): This skips docs for non-index entry points. We can try to work around it, but + // most likely it makes sense to wait for API Extractor to natively support exports. + enabled: name === 'index', + apiJsonFilePath: resolvePath( + outputDir, + `${suffix}.api.json`, + ), + }, + + dtsRollup: { + enabled: false, + }, + + tsdocMetadata: { + enabled: false, + }, + + messages: { + // Silence compiler warnings, as these will prevent the CI build to work + compilerMessageReporting: { + default: { + logLevel: 'none' as ExtractorLogLevel.None, + // These contain absolute file paths, so can't be included in the report + // addToApiReportFile: true, + }, + }, + extractorMessageReporting: { + default: { + logLevel: 'warning' as ExtractorLogLevel.Warning, + addToApiReportFile: true, + }, + ...messagesConf, + }, + tsdocMessageReporting: { + default: { + logLevel: 'warning' as ExtractorLogLevel.Warning, + addToApiReportFile: true, + }, + }, + }, + + newlineKind: 'lf', + + projectFolder, + }, + configObjectFullPath: projectFolder, + packageJsonFullPath: resolvePath(projectFolder, 'package.json'), + tsdocConfigFile: await getTsDocConfig(), + ignoreMissingEntryPoint: true, }); - } - // Message verbosity can't be configured, so just skip the check instead - (Extractor as any)._checkCompilerCompatibility = () => {}; - - let shouldLogInstructions = false; - let conflictingFile: undefined | string = undefined; - - // Invoke API Extractor - const extractorResult = Extractor.invoke(extractorConfig, { - localBuild: isLocalBuild, - showVerboseMessages: false, - showDiagnostics: false, - messageCallback(message) { - if (message.text.includes('The API report file is missing')) { - shouldLogInstructions = true; + // The `packageFolder` needs to point to the location within `dist-types` in order for relative + // paths to be logged. Unfortunately the `prepare` method above derives it from the `packageJsonFullPath`, + // which needs to point to the actual file, so we override `packageFolder` afterwards. + ( + extractorConfig as { + packageFolder: string; } - if ( - message.text.includes( - 'You have changed the public API signature for this project.', - ) - ) { - shouldLogInstructions = true; - const match = message.text.match( - /Please copy the file "(.*)" to "api-report\.md"/, - ); - if (match) { - conflictingFile = match[1]; + ).packageFolder = packageFolder; + + if (!compilerState) { + compilerState = CompilerState.create(extractorConfig, { + additionalEntryPoints: entryPoints, + }); + } + + // Message verbosity can't be configured, so just skip the check instead + (Extractor as any)._checkCompilerCompatibility = () => {}; + + let shouldLogInstructions = false; + let conflictingFile: undefined | string = undefined; + + // Invoke API Extractor + const extractorResult = Extractor.invoke(extractorConfig, { + localBuild: isLocalBuild, + showVerboseMessages: false, + showDiagnostics: false, + messageCallback(message) { + if (message.text.includes('The API report file is missing')) { + shouldLogInstructions = true; } - } - }, - compilerState, - }); - - // This release tag validation makes sure that the release tag of known entry points match expectations. - // The root index entrypoint is only allowed @public exports, while /alpha and /beta only allow @alpha and @beta. - if ( - validateReleaseTags && - fs.pathExistsSync(extractorConfig.reportFilePath) - ) { - if (['index', 'alpha', 'beta'].includes(name)) { - const report = await fs.readFile( - extractorConfig.reportFilePath, - 'utf8', - ); - const lines = report.split(/\r?\n/); - const expectedTag = name === 'index' ? 'public' : name; - for (let i = 0; i < lines.length; i += 1) { - const line = lines[i]; - const match = line.match(/^\/\/ @(alpha|beta|public)/); - if (match && match[1] !== expectedTag) { - // Because of limitations in the type script rollup logic we need to allow public exports from the other release stages - // TODO(Rugvip): Try to work around the need for this exception - if (expectedTag !== 'public' && match[1] === 'public') { - continue; - } - throw new Error( - `Unexpected release tag ${match[1]} in ${ - extractorConfig.reportFilePath - } at line ${i + 1}`, + if ( + message.text.includes( + 'You have changed the public API signature for this project.', + ) + ) { + shouldLogInstructions = true; + const match = message.text.match( + /Please copy the file "(.*)" to "api-report\.md"/, ); + if (match) { + conflictingFile = match[1]; + } + } + }, + compilerState, + }); + + // This release tag validation makes sure that the release tag of known entry points match expectations. + // The root index entrypoint is only allowed @public exports, while /alpha and /beta only allow @alpha and @beta. + if ( + validateReleaseTags && + fs.pathExistsSync(extractorConfig.reportFilePath) + ) { + if (['index', 'alpha', 'beta'].includes(name)) { + const report = await fs.readFile( + extractorConfig.reportFilePath, + 'utf8', + ); + const lines = report.split(/\r?\n/); + const expectedTag = name === 'index' ? 'public' : name; + for (let i = 0; i < lines.length; i += 1) { + const line = lines[i]; + const match = line.match(/^\/\/ @(alpha|beta|public)/); + if (match && match[1] !== expectedTag) { + // Because of limitations in the type script rollup logic we need to allow public exports from the other release stages + // TODO(Rugvip): Try to work around the need for this exception + if (expectedTag !== 'public' && match[1] === 'public') { + continue; + } + throw new Error( + `Unexpected release tag ${match[1]} in ${ + extractorConfig.reportFilePath + } at line ${i + 1}`, + ); + } } } } - } - - if (!extractorResult.succeeded) { - if (shouldLogInstructions) { - logApiReportInstructions(); - - if (conflictingFile) { - console.log(''); - console.log( - `The conflicting file is ${relativePath( - tmpDir, - conflictingFile, - )}, with the following content:`, - ); - console.log(''); - - const content = await fs.readFile(conflictingFile, 'utf8'); - console.log(content); + if (!extractorResult.succeeded) { + if (shouldLogInstructions) { logApiReportInstructions(); + + if (conflictingFile) { + console.log(''); + console.log( + `The conflicting file is ${relativePath( + tmpDir, + conflictingFile, + )}, with the following content:`, + ); + console.log(''); + + const content = await fs.readFile(conflictingFile, 'utf8'); + console.log(content); + + logApiReportInstructions(); + } } + + throw new Error( + `API Extractor completed with ${extractorResult.errorCount} errors` + + ` and ${extractorResult.warningCount} warnings`, + ); } - throw new Error( - `API Extractor completed with ${extractorResult.errorCount} errors` + - ` and ${extractorResult.warningCount} warnings`, - ); - } - - const warningCountAfter = await countApiReportWarnings(reportPath); - 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`, - ); - } - if (warningCountAfter === 0 && allowWarningPkg.includes(packageDir)) { - console.log( - `No need to allow warnings for ${packageDir}, it does not have any`, - ); - } - if (warningCountAfter > warningCountBefore) { - warnings.push( - `The API Report for ${packageDir} introduces new warnings. ` + - 'Please fix these warnings in order to keep the API Reports tidy.', - ); + const warningCountAfter = await countApiReportWarnings(reportPath); + 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`, + ); + } + if (warningCountAfter === 0 && allowWarningPkg.includes(packageDir)) { + console.log( + `No need to allow warnings for ${packageDir}, it does not have any`, + ); + } + if (warningCountAfter > warningCountBefore) { + warnings.push( + `The API Report for ${packageDir} introduces new warnings. ` + + 'Please fix these warnings in order to keep the API Reports tidy.', + ); + } + if (staleReportFiles.length) { + if (noBail) { + for (const f of staleReportFiles) { + fs.rmSync(resolvePath(projectFolder, f)); + console.log(`Deleted deprecated API report ${f}`); + } + } else { + const staleList = staleReportFiles + .map(f => join(packageDir, f)) + .join(', '); + throw new Error( + `The API Report(s) ${staleList} are no longer relevant and should be deleted`, + ); + } + } } }