Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(scripts): remove @internal stripping and enable no deps build needed for api generation #25575

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions scripts/api-extractor/api-extractor.common.v-next.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,7 @@
},
"dtsRollup": {
"enabled": true,
// TODO: replace following line with these 2 commented ones after all v9 is migrated to new build and .d.ts API stripping
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we wont strip @internal apis thus removing

"untrimmedFilePath": "<projectFolder>/dist/index.d.ts",
// "untrimmedFilePath": "<projectFolder>/dist/untrimmed.d.ts",
// "publicTrimmedFilePath": "<projectFolder>/dist/index.d.ts",
"omitTrimmingComments": false
},
"compiler": {
Expand Down
1 change: 1 addition & 0 deletions scripts/tasks/api-extractor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ export function apiExtractor() {
tsConfig,
tsConfigPath,
packageJson,
definitionsRootPath: 'dist/out-tsc/types',
});

config.compiler = compilerConfig;
Expand Down
2 changes: 1 addition & 1 deletion scripts/tasks/ts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ function prepareTsTaskConfig(options: TscTaskOptions) {
if (hasNewCompilationSetup) {
options.outDir = `${tsConfigOutDir}/${options.outDir}`;
} else {
// TODO: remove after all v9 is migrated to new build and .d.ts API stripping
// TODO: remove after all v9 is migrated to new tsc processing
options.baseUrl = '.';
options.rootDir = './src';
}
Expand Down
53 changes: 53 additions & 0 deletions scripts/tasks/utils.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { getTsPathAliasesApiExtractorConfig } from './utils';

type DeepPartial<T> = Partial<{ [P in keyof T]: DeepPartial<T[P]> }>;
describe(`utils`, () => {
describe(`#getTsPathAliasesApiExtractorConfig`, () => {
type Options = Parameters<typeof getTsPathAliasesApiExtractorConfig>[0];
function setup(options: DeepPartial<Options> = {}) {
const defaults = {
tsConfig: {
compilerOptions: {
outDir: '../../dist/out-tsc',
...options.tsConfig?.compilerOptions,
},
},
tsConfigPath: 'tsconfig.lib.json',
packageJson: {
name: '@proj/one',
version: '0.0.1',
main: 'lib/index.js',
dependencies: { ...options.packageJson?.dependencies },
peerDependencies: { ...options.packageJson?.peerDependencies },
},
definitionsRootPath: options.definitionsRootPath ?? 'dist/types',
};

return getTsPathAliasesApiExtractorConfig(defaults as Options);
}

it(`should set compilerOptions`, () => {
const actual = setup();

expect(actual.overrideTsconfig.compilerOptions).toEqual(
expect.objectContaining({ isolatedModules: false, skipLibCheck: false }),
);
});

it(`should override path aliases to emitted declaration files instead of source files`, () => {
const actual = setup({ definitionsRootPath: 'dist/for/types' });

const newPaths = (actual.overrideTsconfig.compilerOptions.paths as unknown) as Record<string, string[]>;

const newPath = Object.values(newPaths)[0][0];
expect(newPath).toMatch(new RegExp('^dist/for/types.+src/index.d.ts$', 'i'));
});

it(`should set allowSyntheticDefaultImports if package has invalid deps/peerDeps`, () => {
const actual = setup({ packageJson: { dependencies: { '@storybook/api': '6.5.0' } } });
expect(actual.overrideTsconfig.compilerOptions).toEqual(
expect.objectContaining({ allowSyntheticDefaultImports: true }),
);
});
});
});
29 changes: 27 additions & 2 deletions scripts/tasks/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,42 @@ function enableAllowSyntheticDefaultImports(options: { pkgJson: PackageJson }) {
return shouldEnable ? { allowSyntheticDefaultImports: true } : null;
}

const rootTsConfig = JSON.parse(
fs.readFileSync(path.resolve(__dirname, '../../tsconfig.base.json'), 'utf-8'),
) as TsConfig;

function createNormalizedTsPaths(options: { definitionsRootPath: string; rootTsConfig: TsConfig }) {
const paths = (options.rootTsConfig.compilerOptions.paths as unknown) as Record<string, string[]>;
Copy link
Contributor Author

@Hotell Hotell Nov 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just-scripts provides invalid types for CompilerOptions thus need of override


const normalizedPaths = Object.entries(paths).reduce((acc, [pkgName, pathAliases]) => {
acc[pkgName] = [path.join(options.definitionsRootPath, pathAliases[0].replace('index.ts', 'index.d.ts'))];
return acc;
}, {} as typeof paths);

return normalizedPaths;
}

export function getTsPathAliasesApiExtractorConfig(options: {
tsConfig: TsConfig;
tsConfigPath: string;
packageJson: PackageJson;
definitionsRootPath: string;
}) {
const hasNewCompilationSetup = ((options.tsConfig.compilerOptions as unknown) as { outDir: string }).outDir.includes(
'dist/out-tsc',
);
// TODO: after all v9 is migrated to new tsc processing use only createNormalizedTsPaths
const normalizedPaths = hasNewCompilationSetup
? createNormalizedTsPaths({ definitionsRootPath: options.definitionsRootPath, rootTsConfig })
: undefined;

/**
* Customized TSConfig that uses `tsconfig.lib.json` as base with some required overrides:
*
* NOTES:
* - `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
* - `paths` is overriden to path mapping that points to generated declaration files. This also enables creation of dts rollup without a need of generating rollups for all dependencies 🫡
*
*/
const apiExtractorTsConfig: TsConfig = {
Expand All @@ -71,7 +95,8 @@ export function getTsPathAliasesApiExtractorConfig(options: {
/**
* just-scripts provides invalid types for tsconfig, thus `paths` cannot be set to dictionary,nor null or `{}`
*/
paths: undefined,
// @ts-expect-error - just-scripts provides invalid types
paths: normalizedPaths,
},
};

Expand Down
8 changes: 1 addition & 7 deletions tools/generators/migrate-converged-pkg/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -853,11 +853,6 @@ describe('migrate-converged-pkg generator', () => {
expect(readJson(tree, apiExtractorConfigPath)).toMatchInlineSnapshot(`
Object {
"$schema": "https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json",
"dtsRollup": Object {
"enabled": true,
"publicTrimmedFilePath": "<projectFolder>/dist/index.d.ts",
"untrimmedFilePath": "",
},
"extends": "@fluentui/scripts/api-extractor/api-extractor.common.v-next.json",
}
`);
Expand Down Expand Up @@ -1493,8 +1488,7 @@ describe('migrate-converged-pkg generator', () => {
},
"dtsRollup": Object {
"enabled": true,
"publicTrimmedFilePath": "<projectFolder>/dist/unstable.d.ts",
"untrimmedFilePath": "<projectFolder>/dist/unstable-untrimmed.d.ts",
"untrimmedFilePath": "<projectFolder>/dist/unstable.d.ts",
},
"extends": "@fluentui/scripts/api-extractor/api-extractor.common.v-next.json",
"mainEntryPointFilePath": "<projectFolder>/../../../dist/out-tsc/types/packages/react-components/<unscopedPackageName>/src/unstable/index.d.ts",
Expand Down
27 changes: 14 additions & 13 deletions tools/generators/migrate-converged-pkg/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ function runMigrationOnProject(tree: Tree, schema: AssertedSchema, _userLog: Use

// update package npm scripts
updatePackageJson(tree, optionsWithTsConfigs);
updateApiExtractorForLocalBuilds(tree, optionsWithTsConfigs);
updateApiExtractor(tree, optionsWithTsConfigs);

// setup storybook
setupStorybook(tree, options);
Expand All @@ -155,12 +155,6 @@ const templates = {
main: {
$schema: 'https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json',
extends: '@fluentui/scripts/api-extractor/api-extractor.common.v-next.json',
// TODO: remove after all v9 is migrated to new build and .d.ts API stripping
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we wont stript @internal marked APIs thus removing

dtsRollup: {
enabled: true,
untrimmedFilePath: '',
publicTrimmedFilePath: '<projectFolder>/dist/index.d.ts',
},
},
unstable: {
$schema: 'https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json',
Expand All @@ -174,8 +168,7 @@ const templates = {
},
dtsRollup: {
enabled: true,
untrimmedFilePath: '<projectFolder>/dist/unstable-untrimmed.d.ts',
publicTrimmedFilePath: '<projectFolder>/dist/unstable.d.ts',
untrimmedFilePath: '<projectFolder>/dist/unstable.d.ts',
},
},
};
Expand Down Expand Up @@ -576,11 +569,11 @@ function setupUnstableApi(tree: Tree, options: NormalizedSchemaWithTsConfigs) {
}

updateUnstablePackageJson();
updateUnstableApiExtractorForLocalBuilds();
updateUnstableApiExtractor();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove Local from name as with this we have only 1 api-extractor config 🙌


return tree;

function updateUnstableApiExtractorForLocalBuilds() {
function updateUnstableApiExtractor() {
const apiExtractor = templates.apiExtractor();

writeJson(tree, joinPathFragments(options.paths.configRoot, 'api-extractor.unstable.json'), apiExtractor.unstable);
Expand Down Expand Up @@ -646,7 +639,6 @@ function updatePackageJson(tree: Tree, options: NormalizedSchemaWithTsConfigs) {

function setupScripts(json: PackageJson) {
const scripts = {
'generate-api': 'tsc -p ./tsconfig.lib.json --emitDeclarationOnly && just-scripts api-extractor',
test: 'jest --passWithNoTests',
'type-check': 'tsc -b tsconfig.json',
};
Expand Down Expand Up @@ -686,12 +678,21 @@ function updatePackageJson(tree: Tree, options: NormalizedSchemaWithTsConfigs) {
}
}

function updateApiExtractorForLocalBuilds(tree: Tree, options: NormalizedSchemaWithTsConfigs) {
function updateApiExtractor(tree: Tree, options: NormalizedSchemaWithTsConfigs) {
const apiExtractor = templates.apiExtractor();
const scripts = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

collocate to proper boundary so its possible to run only particular migrations if needed

'generate-api': 'tsc -p ./tsconfig.lib.json --emitDeclarationOnly && just-scripts api-extractor',
};

tree.delete(joinPathFragments(options.paths.configRoot, 'api-extractor.local.json'));
writeJson(tree, joinPathFragments(options.paths.configRoot, 'api-extractor.json'), apiExtractor.main);

updateJson(tree, options.paths.packageJson, (json: PackageJson) => {
Object.assign(json.scripts, scripts);

return json;
});

return tree;
}

Expand Down