Skip to content

Commit

Permalink
feat(scripts): improve API violation reporting (microsoft#25356)
Browse files Browse the repository at this point in the history
* fix(script): make copy-compiled task work with packages without 'module'
* fix(scripts): make internal api stripping work and add rolluped dts after check
  • Loading branch information
Hotell authored Nov 3, 2022
1 parent a1cb47e commit 6ce378e
Show file tree
Hide file tree
Showing 3 changed files with 154 additions and 48 deletions.
127 changes: 112 additions & 15 deletions scripts/tasks/api-extractor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ export function apiExtractor() {
const args: ReturnType<typeof getJustArgv> & Partial<ApiExtractorCliRunCommandArgs> = getJustArgv();

const { isUsingTsSolutionConfigs, packageJson, tsConfig, tsConfigPath } = getTsPathAliasesConfig();
const messages = {
TS7016: [] as string[],
TS2305: [] as string[],
};

return apiExtractorConfigsForExecution.length
? (series(
Expand All @@ -82,33 +86,54 @@ export function apiExtractor() {
typescriptCompilerFolder: args['typescript-compiler-folder'],
configJsonFilePath: args.config ?? configPath,
localBuild: args.local ?? !process.env.TF_BUILD,

messageCallback: message => {
onResult: result => {
if (!isUsingTsSolutionConfigs) {
return;
}
if (message.category !== messageCategories.Compiler) {
if (result.succeeded === true) {
return;
}

if (message.messageId === compilerMessages.TS2305) {
logger.error(
if (messages.TS2305.length) {
const errTitle = [
chalk.bgRed.white.bold(`api-extractor | API VIOLATION:`),
chalk.red(`Looks like your package public API surface uses \`@internal\` marked API's!`),
chalk.red(` Your package public API uses \`@internal\` marked API's from following packages:`),
'\n',
);
].join('');
const logErr = formatApiViolationMessage(messages.TS2305);

logger.error(errTitle, logErr, '\n');
}

if (message.messageId === compilerMessages.TS7016) {
logger.error(
if (messages.TS7016.length) {
const errTitle = [
chalk.bgRed.white.bold(`api-extractor | MISSING DEPENDENCY TYPE DECLARATIONS:`),
chalk.red(`Looks like your package dependencies don't have generated index.d.ts type definitions.`),
'\n',
chalk.blueBright(
`🛠 Fix this by running: ${chalk.italic(`yarn lage generate-api --to ${packageJson.name}`)}`,
),
chalk.red(` Package dependencies are missing index.d.ts type definitions:`),
'\n',
].join('');
const logErr = formatMissingApiViolationMessage(messages.TS7016);
const logFix = chalk.blueBright(
`${chalk.bold('🛠 FIX')}: run '${chalk.italic(`yarn lage generate-api --to ${packageJson.name}`)}'`,
);

logger.error(errTitle, logErr, '\n', logFix, '\n');
}
},

messageCallback: message => {
if (!isUsingTsSolutionConfigs) {
return;
}
if (message.category !== messageCategories.Compiler) {
return;
}

if (message.messageId === compilerMessages.TS2305) {
messages.TS2305.push(message.text);
}

if (message.messageId === compilerMessages.TS7016) {
messages.TS7016.push(message.text);
}
},
onConfigLoaded: config => {
Expand All @@ -117,7 +142,12 @@ export function apiExtractor() {
}

logger.info(`api-extractor: package is using TS path aliases. Overriding TS compiler settings.`);
const compilerConfig = getTsPathAliasesApiExtractorConfig({ tsConfig, tsConfigPath, packageJson });

const compilerConfig = getTsPathAliasesApiExtractorConfig({
tsConfig,
tsConfigPath,
packageJson,
});

config.compiler = compilerConfig;
},
Expand All @@ -138,3 +168,70 @@ export function apiExtractor() {
logger.info(`skipping api-extractor execution - no configs present`);
};
}

/**
*
* @example
*
* ```
(TS2305) Module '"@fluentui/react-shared-contexts"' has no exported member 'ThemeContextValue_unstable'.
(TS2305) Module '"@fluentui/react-shared-contexts"' has no exported member 'TooltipVisibilityContextValue_unstable'.
↓ ↓ ↓
@fluentui/react-shared-contexts:
- TooltipVisibilityContextValue_unstable
- ThemeContextValue_unstable
```
*/
function formatApiViolationMessage(messages: string[]) {
const regexPkg = /'"(@fluentui\/[a-z-]+)"'/i;
const exportedTokenRegex = /'([a-z-_]+)'/i;

const byPackage = messages.reduce((acc, curr) => {
const [, packageName] = regexPkg.exec(curr) ?? [];
const [, exportedToken] = exportedTokenRegex.exec(curr) ?? [];
if (acc[packageName]) {
acc[packageName].add(exportedToken);
return acc;
}
acc[packageName] = new Set([exportedToken]);
return acc;
}, {} as Record<string, Set<string>>);

return Object.entries(byPackage)
.map(([packageName, tokens]) => {
return [
chalk.red.underline(packageName) + ':',
Array.from(tokens)
.map(token => chalk.italic.red(' - ' + token))
.join('\n'),
].join('\n');
})
.join('\n');
}

/**
*
* @example
```
(TS7016) Could not find a declaration file for module '@fluentui/react-theme'
(TS7016) Could not find a declaration file for module '@fluentui/react-shared-contexts'
↓ ↓ ↓
- @fluentui/react-theme
- @fluentui/react-shared-contexts
```
*/
function formatMissingApiViolationMessage(messages: string[]) {
const regexPkg = /'(@fluentui\/[a-z-]+)'/i;

return Object.values(
messages.reduce((acc, curr) => {
const [, packageName] = regexPkg.exec(curr) ?? [];
acc[curr] = chalk.italic.red('\t- ' + packageName);
return acc;
}, {} as Record<string, string>),
).join('\n');
}
41 changes: 22 additions & 19 deletions scripts/tasks/copy.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as fs from 'fs-extra';
import * as path from 'path';
import { series, resolveCwd, copyTask, copyInstructionsTask, logger } from 'just-scripts';
import { series, resolveCwd, copyTask, copyInstructionsTask, logger, TaskFunction } from 'just-scripts';
import { getProjectMetadata, findGitRoot } from '../monorepo';
import { getTsPathAliasesConfig } from './utils';

Expand Down Expand Up @@ -61,20 +61,19 @@ export function copyCompiled() {
if (!projectMetadata.sourceRoot) {
throw new Error(`${packageJson.name} is missing 'sourceRoot' in workspace.json`);
}
if (!packageJson.module) {
throw new Error(`${packageJson.name} is missing 'module' property in package.json`);
}

const paths = {
esm: {
in: path.join(
packageDir,
tsConfig.compilerOptions.outDir as string,
path.dirname(packageJson.module),
projectMetadata.sourceRoot,
),
out: path.join(packageDir, path.dirname(packageJson.module)),
},
esm: packageJson.module
? {
in: path.join(
packageDir,
tsConfig.compilerOptions.outDir as string,
path.dirname(packageJson.module),
projectMetadata.sourceRoot,
),
out: path.join(packageDir, path.dirname(packageJson.module)),
}
: null,
commonJs: {
in: path.join(
packageDir,
Expand All @@ -86,17 +85,21 @@ export function copyCompiled() {
},
};

return series(
copyTask({
paths: [paths.esm.in],
dest: paths.esm.out,
}),
const tasks = [
paths.esm
? copyTask({
paths: [paths.esm.in],
dest: paths.esm.out,
})
: null,
copyTask({
paths: [paths.commonJs.in],

dest: paths.commonJs.out,
}),
);
].filter(Boolean) as TaskFunction[];

return series(...tasks);
}
export function copy() {
const configPath = path.resolve(process.cwd(), 'config/pre-copy.json');
Expand Down
34 changes: 20 additions & 14 deletions scripts/tasks/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ import * as fs from 'fs';
import * as path from 'path';
import * as jju from 'jju';
import type { TscTaskOptions } from 'just-scripts';
import { offsetFromRoot } from '@nrwl/devkit';
import { appRootPath } from '@nrwl/tao/src/utils/app-root';

export function getTsPathAliasesConfig() {
const cwd = process.cwd();
Expand Down Expand Up @@ -46,26 +44,34 @@ export function getTsPathAliasesApiExtractorConfig(options: {
tsConfigPath: string;
packageJson: PackageJson;
}) {
const rootOffset = offsetFromRoot(path.dirname(options.tsConfigPath.replace(appRootPath, '')));
/**
* This special TSConfig config is all that's needed for api-extractor so it has all type information used for package:
* Customized TSConfig that uses `tsconfig.lib.json` as base with some required overrides:
*
* NOTES:
* - `compilerOptions.paths` doesn't work, nor is possible to turn them off when `extends` is used
* - `extends` is properly resolved via api-extractor which uses TS api
* - `skipLibCheck` needs to be explicitly set to `false` so errors propagate to api-extractor
* - `paths` is set to `undefined` so api-extractor won't use source files rather rollup-ed declaration files only
*
*/
const apiExtractorTsConfig: TsConfig = {
include: options.tsConfig.include,
/**
* `files` might be used to specify additional `d.ts` or global type definitions. IF they exist in package tsconfig we need to include them
*/
...(options.tsConfig.files ? { files: options.tsConfig.files } : null),
...options.tsConfig,
compilerOptions: {
...options.tsConfig.compilerOptions,
...enableAllowSyntheticDefaultImports({ pkgJson: options.packageJson }),
strict: true,
lib: options.tsConfig.compilerOptions.lib,
typeRoots: ['node_modules/@types', `${rootOffset}typings`],
types: options.tsConfig.compilerOptions.types,
/**
* This option has no effect on type declarations '.d.ts' thus can be turned off. For more info see https://www.typescriptlang.org/tsconfig#non-module-files
*
* NOTE: Some v8 packages (font-icons-mdl2) use `preserveConstEnums: false` which clashes with isolateModules - TSC will error
*/
isolatedModules: false,
/**
* needs to be explicitly set to `false` so errors propagate to api-extractor
*/
skipLibCheck: false,
/**
* just-scripts provides invalid types for tsconfig, thus `paths` cannot be set to dictionary,nor null or `{}`
*/
paths: undefined,
},
};

Expand Down

0 comments on commit 6ce378e

Please sign in to comment.