Skip to content

Commit 744a2ca

Browse files
committed
refactor(bazel): simplify api-golden rule after windows changes
Now that for Windows contributors, we also can assume a proper sandbox and node modules directory, we no longer need to leverage our `rules_nodejs` tricks/hack. Very soon we'll be able to remove the full `interop_module_mappings.ts` file given our `rules_js` migration. Yay!
1 parent e00c1ff commit 744a2ca

File tree

7 files changed

+35
-102
lines changed

7 files changed

+35
-102
lines changed

bazel/api-golden/BUILD.bazel

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,7 @@ ts_project(
1212
srcs = [
1313
"find_entry_points.ts",
1414
"index_npm_packages.ts",
15-
"module_mappings.ts",
16-
"patch-host.ts",
15+
"interop_module_mappings.ts",
1716
"path-normalize.ts",
1817
"test_api_report.ts",
1918
],

bazel/api-golden/index.bzl

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,20 @@ load("@bazel_skylib//rules:write_file.bzl", "write_file")
22
load("@build_bazel_rules_nodejs//:index.bzl", "pkg_npm")
33
load("//bazel/api-golden:index_rjs.bzl", _rjs_api_golden_test_npm_package = "api_golden_test_npm_package")
44

5-
nodejs_test_args = [
6-
# Needed so that node doesn't walk back to the source directory.
7-
# From there, the relative imports would point to .ts files.
8-
"--node_options=--preserve-symlinks",
9-
"--nobazel_run_linker",
10-
]
11-
125
default_strip_export_pattern = "^ɵ(?!ɵdefineInjectable|ɵinject|ɵInjectableDef)"
136

14-
def extract_module_names_from_npm_targets(type_targets):
7+
def extract_names_from_npm_targets(type_targets):
158
types = {}
169

1710
for type_target in type_targets:
1811
type_label = Label(type_target)
1912
type_package = type_label.package
2013

21-
if type_label.workspace_name != "npm":
14+
if type_label.workspace_name != "npm" or not type_package.startswith("@types/"):
2215
fail("Expected type targets to be part of the `@npm` workspace." +
2316
"e.g. `@npm//@types/nodes`.")
2417

25-
types[type_target] = type_package
18+
types[type_target] = type_package[len("@types/"):]
2619

2720
return types
2821

@@ -60,7 +53,8 @@ def api_golden_test(
6053
data = [":%s_synthetic_package" % name] + data,
6154
npm_package = "%s/%s_synthetic_package" % (native.package_name(), name),
6255
strip_export_pattern = strip_export_pattern,
63-
types = extract_module_names_from_npm_targets(types),
56+
types = extract_names_from_npm_targets(types),
57+
interop_mode = True,
6458
**kwargs
6559
)
6660

@@ -79,7 +73,8 @@ def api_golden_test_npm_package(
7973
npm_package = fixup_path_for_rules_js(npm_package),
8074
data = data,
8175
strip_export_pattern = strip_export_pattern,
82-
types = extract_module_names_from_npm_targets(types),
76+
types = extract_names_from_npm_targets(types),
77+
interop_mode = True,
8378
**kwargs
8479
)
8580

bazel/api-golden/index_npm_packages.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ async function main(
3131
npmPackageDir: string,
3232
approveGolden: boolean,
3333
stripExportPattern: RegExp,
34-
typePackageNames: string[],
34+
typeNames: string[],
3535
) {
3636
// TODO(ESM) This can be replaced with an actual ESM import when `ts_library` is
3737
// guaranteed to be ESM-only and supports the `mts` extension.
@@ -57,7 +57,7 @@ async function main(
5757
const actual = await testApiGolden(
5858
typesEntryPointPath,
5959
stripExportPattern,
60-
typePackageNames,
60+
typeNames,
6161
packageJsonPath,
6262
moduleName,
6363
);
@@ -101,14 +101,14 @@ async function main(
101101
const npmPackageDir = path.resolve(args[1]);
102102
const approveGolden = args[2] === 'true';
103103
const stripExportPattern = new RegExp(args[3]);
104-
const typePackageNames = args.slice(4);
104+
const typeNames = args.slice(4);
105105

106106
// For approving, point to the real directory outside of the bazel-out.
107107
if (approveGolden) {
108108
goldenDir = path.join(process.env.BUILD_WORKSPACE_DIRECTORY!, args[0]);
109109
}
110110

111-
main(goldenDir, npmPackageDir, approveGolden, stripExportPattern, typePackageNames).catch((e) => {
111+
main(goldenDir, npmPackageDir, approveGolden, stripExportPattern, typeNames).catch((e) => {
112112
console.error(e);
113113
process.exit(1);
114114
});

bazel/api-golden/index_rjs.bzl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ def api_golden_test_npm_package(
1313
data = [],
1414
strip_export_pattern = default_strip_export_pattern,
1515
types = {},
16+
interop_mode = False,
1617
**kwargs):
1718
"""Builds an API report for all entry-points within the given NPM package and compares it.
1819
@@ -23,6 +24,7 @@ def api_golden_test_npm_package(
2324
data: Runtime dependenices needed for the rule (e.g. the tree artifact of the NPM package)
2425
strip_export_pattern: An optional regular expression to filter out exports from the golden.
2526
types: Optional list of type targets to make available in the API report generation.
27+
interop_mode: Whether we are compiling in `rules_nodejs` interop mode.
2628
**kwargs: Other arguments passed to `js_binary`/`js_test` (depending on approval mode)
2729
"""
2830

@@ -42,6 +44,7 @@ def api_golden_test_npm_package(
4244
data = data,
4345
entry_point = "@devinfra//bazel/api-golden:index_npm_packages.js",
4446
args = [golden_dir, npm_package, "false", quoted_export_pattern] + type_names,
47+
env = {"RJS_MODE": "true" if not interop_mode else "false"},
4548
**kwargs
4649
)
4750

@@ -51,5 +54,6 @@ def api_golden_test_npm_package(
5154
data = data,
5255
entry_point = "@devinfra//bazel/api-golden:index_npm_packages.js",
5356
args = [golden_dir, npm_package, "true", quoted_export_pattern] + type_names,
57+
env = {"RJS_MODE": "true" if not interop_mode else "false"},
5458
**kwargs
5559
)

bazel/api-golden/module_mappings.ts renamed to bazel/api-golden/interop_module_mappings.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,11 @@ const scopedTypesPackageRegex = /^@types\/([^_\/]+)__(.+)/;
1616
* Resolves type modules and returns corresponding path mappings and a
1717
* list of referenced files.
1818
*/
19-
export async function resolveTypePackages(typePackageNames: string[]): Promise<{
19+
export async function resolveTypePackages(typeNames: string[]): Promise<{
2020
paths: Record<string, string[]>;
2121
typeFiles: string[];
2222
}> {
23+
const typePackageNames = typeNames.map((t) => `@types/${t}`);
2324
const typeFiles = [];
2425
const paths: Record<string, string[]> = {};
2526

bazel/api-golden/patch-host.ts

Lines changed: 0 additions & 64 deletions
This file was deleted.

bazel/api-golden/test_api_report.ts

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,7 @@ import os from 'os';
2121

2222
import {AstModule} from '@microsoft/api-extractor/lib/analyzer/AstModule';
2323
import {ExportAnalyzer} from '@microsoft/api-extractor/lib/analyzer/ExportAnalyzer';
24-
import {resolveTypePackages} from './module_mappings.js';
25-
import {patchHostToSkipNodeModules} from './patch-host.js';
24+
import {resolveTypePackages} from './interop_module_mappings.js';
2625

2726
/**
2827
* Original definition of the `ExportAnalyzer#fetchAstModuleExportInfo` method.
@@ -51,30 +50,31 @@ const _origFetchAstModuleExportInfo = ExportAnalyzer.prototype.fetchAstModuleExp
5150
export async function testApiGolden(
5251
indexFilePath: string,
5352
stripExportPattern: RegExp,
54-
typePackageNames: string[],
53+
typeNames: string[],
5554
packageJsonPath: string,
5655
customPackageName: string,
5756
): Promise<string | null> {
5857
const tempDir =
5958
process.env.TEST_TMPDIR ?? fs.mkdtempSync(path.join(os.tmpdir(), 'api-golden-rule'));
60-
const {paths, typeFiles} = await resolveTypePackages(typePackageNames);
59+
const rjsMode = process.env['RJS_MODE'] === 'true';
60+
61+
let resolvedTypePackages: Awaited<ReturnType<typeof resolveTypePackages>> | null = null;
62+
if (!rjsMode) {
63+
resolvedTypePackages = await resolveTypePackages(typeNames);
64+
}
6165

6266
const configObject: IConfigFile = {
6367
compiler: {
64-
overrideTsconfig:
65-
// We disable automatic `@types` resolution as this throws-off API reports when the API
66-
// test is run outside sandbox. Instead we expect a list of hard-coded types that should
67-
// be added.This works in non-sandbox and Windows. Note that we include the type files
68-
// directly in the compilation, and additionally set up path mappings. This allows
69-
// for global type definitions and module-scoped types to work.
70-
{
71-
files: [indexFilePath, ...typeFiles],
72-
compilerOptions: {
73-
paths,
74-
types: [],
75-
lib: ['esnext', 'dom'],
76-
},
68+
overrideTsconfig: {
69+
// In interop/compat mode, the linker is not available and there is no `node_modules`
70+
// directory, so we need to manually wire up global types.
71+
files: [indexFilePath, ...(resolvedTypePackages?.typeFiles ?? [])],
72+
compilerOptions: {
73+
paths: resolvedTypePackages?.paths ?? {},
74+
types: rjsMode ? typeNames : [],
75+
lib: ['esnext', 'dom'],
7776
},
77+
},
7878
},
7979
projectFolder: path.dirname(packageJsonPath),
8080
mainEntryPointFilePath: indexFilePath,
@@ -116,8 +116,6 @@ export async function testApiGolden(
116116
configObjectFullPath: undefined,
117117
});
118118

119-
patchHostToSkipNodeModules();
120-
121119
// This patches the `ExportAnalyzer` of `api-extractor` so that we can filter out
122120
// exports that match a specified pattern. Ideally this would not be needed as the
123121
// TSDoc JSDoc annotations could be used to filter out exports from the API report,

0 commit comments

Comments
 (0)