8000 build: rework symbol extractor to support lazy/common chunks (#61566) · angular/angular@6dade56 · GitHub
[go: up one dir, main page]
Skip to content

Commit 6dade56

Browse files
devversionkirjs
devversion
authored and
kirjs
committed
build: rework symbol extractor to support lazy/common chunks (#61566)
Since we are going to replace our `app_bundle` rule (custom ESBuild + Terser pipeline) with the real Angular CLI where shared/lazy/common chunks may exist, we need to update the symbol extractor to support multiple files. We could have just merged all symbols, but this commit tries to do better by detecting what symbols are loaded eagerly vs. lazily. This will be very useful for e.g. defer tests or other lazy features we are introducing in the feature. PR Close #61566
1 parent 7bc39a8 commit 6dade56

File tree

7 files changed

+136
-82
lines changed

7 files changed

+136
-82
lines changed

‎tools/symbol-extractor/BUILD.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ ts_project(
1515
),
1616
tsconfig = "//tools:tsconfig_build",
1717
deps = [
18-
"//:node_modules/@bazel/runfiles",
1918
"//:node_modules/@types/jasmine",
19+
"//:node_modules/tinyglobby",
2020
"//:node_modules/typescript",
2121
],
2222
)

‎tools/symbol-extractor/cli.mts

Lines changed: 83 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,23 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import {runfiles} from '@bazel/runfiles';
109
import * as fs from 'fs';
1110
import * as path from 'path';
11+
import {globSync} from 'tinyglobby';
1212

1313
import {SymbolExtractor} from './symbol_extractor.mjs';
14+
import assert from 'assert';
1415

1516
const args = process.argv.slice(2) as [string, string];
1617
process.exitCode = main(args) ? 0 : 1;
1718

19+
interface GoldenFile {
20+
chunks: {
21+
main: string[];
22+
lazy: string[];
23+
};
24+
}
25+
1826
/**
1927
* CLI main method.
2028
*
@@ -23,29 +31,85 @@ process.exitCode = main(args) ? 0 : 1;
2331
* ```
2432
*/
2533
function main(argv: [string, string, string] | [string, string]): boolean {
26-
const javascriptFilePath = runfiles.resolveWorkspaceRelative(argv[0]);
27-
const goldenFilePath = runfiles.resolveWorkspaceRelative(argv[1]);
34+
const bundlesDir = path.resolve(argv[0]);
35+
const goldenFilePath = path.resolve(argv[1]);
2836
const doUpdate = argv[2] === '--accept';
37+
const bundles = globSync('**/*.js', {cwd: bundlesDir});
38+
const goldenContent = fs.readFileSync(goldenFilePath).toString();
39+
const golden = JSON.parse(goldenContent) as GoldenFile;
2940

30-
console.info('Input javascript file:', javascriptFilePath);
41+
console.info('Input bundles directory:', bundlesDir);
3142

32-
const javascriptContent = fs.readFileSync(javascriptFilePath).toString();
33-
const goldenContent = fs.readFileSync(goldenFilePath).toString();
34-
const symbolExtractor = new SymbolExtractor(javascriptFilePath, javascriptContent);
43+
const importEdges = new Map<string, string[]>();
44+
const bundleSymbols = new Map<string, SymbolExtractor>();
45+
46+
for (const bundleFile of bundles) {
47+
console.info('Processing bundle file:', bundleFile);
48+
49+
const javascriptContent = fs.readFileSync(path.join(bundlesDir, bundleFile)).toString();
50+
const symbolExtractor = new SymbolExtractor(javascriptContent);
51+
52+
// Keep track of import edges, so we can determine what is loaded lazily vs eagerly.
53+
importEdges.set(bundleFile, symbolExtractor.eagerlyLoadedRelativeSpecifiers);
54+
55+
bundleSymbols.set(bundleFile, symbolExtractor);
56+
}
57+
58+
// Find all bundles that are eagerly loaded by the `main.js` bundle.
59+
const eagerlyLoadedBundles = new Set<string>();
60+
const queue: string[] = ['main.js'];
61+
while (queue.length !== 0) {
62+
const entry = queue.pop()!;
63+
64+
if (eagerlyLoadedBundles.has(entry)) {
65+
continue;
66+
}
67+
eagerlyLoadedBundles.add(entry);
68+
69+
for (const edge of importEdges.get(entry) ?? []) {
70+
queue.push(edge);
71+
}
72+
}
73+
74+
const eagerlyLoadedSymbols: string[] = [];
75+
const lazySymbols: string[] = [];
76+
77+
for (const bundleFile of bundles) {
78+
const extractor = bundleSymbols.get(bundleFile);
79+
assert(extractor, `Expected symbol extractor to exist for bundle: ${bundleFile}`);
80+
81+
(eagerlyLoadedBundles.has(bundleFile) ? eagerlyLoadedSymbols : lazySymbols).push(
82+
...extractor.actual,
83+
);
84+
}
3585

36-
let passed = false;
3786
if (doUpdate) {
87+
const newGolden: GoldenFile = {
88+
chunks: {
89+
main: eagerlyLoadedSymbols,
90+
lazy: lazySymbols,
91+
},
92+
};
93+
3894
const goldenOutFilePath = path.join(process.env['BUILD_WORKING_DIRECTORY']!, argv[1]);
39-
fs.writeFileSync(goldenOutFilePath, JSON.stringify(symbolExtractor.actual, undefined, 2));
40-
console.error('Updated gold file:', goldenOutFilePath);
41-
passed = true;
42-
} else {
43-
passed = symbolExtractor.compareAndPrintError(goldenContent);
44-
if (!passed) {
45-
console.error(`TEST FAILED!`);
46-
console.error(` To update the golden file run: `);
47-
console.error(` yarn bazel run ${process.env['TEST_TARGET']}.accept`);
48-
}
95+
fs.writeFileSync(goldenOutFilePath, JSON.stringify(newGolden, undefined, 2));
96+
console.error('Updated golden file:', goldenOutFilePath);
97+
return true;
98+
}
99+
100+
const success =
101+
SymbolExtractor.compareAndPrintError(goldenFilePath, golden?.chunks?.lazy ?? [], lazySymbols) &&
102+
SymbolExtractor.compareAndPrintError(
103+
goldenFilePath,
104+
golden?.chunks?.main ?? [],
105+
eagerlyLoadedSymbols,
106+
);
107+
108+
if (!success) {
109+
console.error(`TEST FAILED!`);
110+
console.error(` To update the golden file run: `);
111+
console.error(` yarn bazel run ${process.env['TEST_TARGET']}.accept`);
49112
}
50-
return passed;
113+
114+
return success;
51115
}

‎tools/symbol-extractor/index.bzl

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@ load("@aspect_rules_js//js:defs.bzl", "js_binary", "js_test")
99
This test verifies that a set of top level symbols from a javascript file match a gold file.
1010
"""
1111

12-
def js_expected_symbol_test(name, src, golden, data = [], **kwargs):
12+
def js_expected_symbol_test(name, bundles_dir, golden, data = [], **kwargs):
1313
"""This test verifies that a set of top level symbols from a javascript file match a gold file.
1414
"""
1515
all_data = data + [
1616
Label("//tools/symbol-extractor:lib_rjs"),
17-
src,
17+
bundles_dir,
1818
golden,
1919
]
2020
entry_point = "//tools/symbol-extractor:cli.mjs"
@@ -24,7 +24,7 @@ def js_expected_symbol_test(name, src, golden, data = [], **kwargs):
2424
data = all_data,
2525
entry_point = entry_point,
2626
tags = kwargs.pop("tags", []) + ["symbol_extractor"],
27-
fixed_args = ["$(rootpath %s)" % src, "$(rootpath %s)" % golden],
27+
fixed_args = ["$(rootpath %s)" % bundles_dir, "$(rootpath %s)" % golden],
2828
**kwargs
2929
)
3030

@@ -33,6 +33,6 @@ def js_expected_symbol_test(name, src, golden, data = [], **kwargs):
3333
testonly = True,
3434
data = all_data,
3535
entry_point = entry_point,
36-
fixed_args = ["$(rootpath %s)" % src, "$(rootpath %s)" % golden, "--accept"],
36+
fixed_args = ["$(rootpath %s)" % bundles_dir, "$(rootpath %s)" % golden, "--accept"],
3737
**kwargs
3838
)

‎tools/symbol-extractor/symbol_extractor.mts

Lines changed: 37 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,18 @@ import ts from 'typescript';
1212

1313
export class SymbolExtractor {
1414
public actual: string[];
15+
public eagerlyLoadedRelativeSpecifiers: string[];
16+
17+
static parse(contents: string): {symbols: string[]; eagerlyLoadedRelativeSpecifiers: string[]} {
18+
const eagerlyLoadedRelativeSpecifiers: string[] = [];
1519

16-
static parse(path: string, contents: string): string[] {
1720
const symbols: string[] = [];
18-
const source: ts.SourceFile = ts.createSourceFile(path, contents, ts.ScriptTarget.Latest, true);
21+
const source: ts.SourceFile = ts.createSourceFile(
22+
'_test_file.js',
23+
contents,
24+
ts.ScriptTarget.Latest,
25+
true,
26+
);
1927
let fnRecurseDepth = 0;
2028
function visitor(child: ts.Node) {
2129
// Left for easier debugging.
@@ -45,11 +53,7 @@ export class SymbolExtractor {
4553
// Terser optimizes variable declarations with `undefined` as initializer
4654
// by omitting the initializer completely. We capture such declarations as well.
4755
// https://github.com/terser/terser/blob/86ea74d5c12ae51b64468/CHANGELOG.md#v540.
48-
if (fnRecurseDepth !== 0) {
49-
if (!isEsmInitFunction(varDecl)) {
50-
symbols.push(stripSuffix(varDecl.name.getText()));
51-
}
52-
}
56+
symbols.push(stripSuffix(varDecl.name.getText()));
5357
break;
5458
case ts.SyntaxKind.FunctionDeclaration:
5559
const funcDecl = child as ts.FunctionDeclaration;
@@ -59,20 +63,29 @@ export class SymbolExtractor {
5963
const classDecl = child as ts.ClassDeclaration;
6064
classDecl.name && symbols.push(stripSuffix(classDecl.name.getText()));
6165
break;
66+
case ts.SyntaxKind.ImportDeclaration:
67+
// Keep track of eagerly loaded modules, so that can determine which
68+
// chunks of a set of bundles is eagerly loaded vs. lazily.
69+
const {moduleSpecifier} = child as ts.ImportDeclaration;
70+
if (ts.isStringLiteral(moduleSpecifier) && moduleSpecifier.text.startsWith('.')) {
71+
eagerlyLoadedRelativeSpecifiers.push(moduleSpecifier.text);
72+
}
73+
6274
default:
6375
// Left for easier debugging.
6476
// console.log('###', ts.SyntaxKind[child.kind], child.getText());
6577
}
6678
}
6779
visitor(source);
6880
symbols.sort();
69-
return symbols;
81+
82+
return {
83+
symbols,
84+
eagerlyLoadedRelativeSpecifiers,
85+
};
7086
}
7187

72-
static diff(actual: string[], expected: string | string[]): {[name: string]: number} {
73-
if (typeof expected == 'string') {
74-
expected = JSON.parse(expected) as string[];
75-
}
88+
static diff(actual: string[], expected: string[]): {[name: string]: number} {
7689
const diff: {[name: string]: number} = {};
7790

7891
// All symbols in the golden file start out with a count corresponding to the number of symbols
@@ -92,23 +105,27 @@ export class SymbolExtractor {
92105
return diff;
93106
}
94107

95-
constructor(
96-
private path: string,
97-
private contents: string,
98-
) {
99-
this.actual = SymbolExtractor.parse(path, contents);
108+
constructor(contents: string) {
109+
const res = SymbolExtractor.parse(contents);
110+
111+
this.actual = res.symbols;
112+
this.eagerlyLoadedRelativeSpecifiers = res.eagerlyLoadedRelativeSpecifiers;
100113
}
101114

102115
expect(expectedSymbols: string[]) {
103116
expect(SymbolExtractor.diff(this.actual, expectedSymbols)).toEqual({});
104117
}
105118

106-
compareAndPrintError(expected: string | string[]): boolean {
119+
static compareAndPrintError(
120+
goldenFilename: string,
121+
expected: string[],
122+
actual: string[],
123+
): boolean {
107124
let passed = true;
108-
const diff = SymbolExtractor.diff(this.actual, expected);
125+
const diff = SymbolExtractor.diff(actual, expected);
109126
Object.keys(diff).forEach((key) => {
110127
if (passed) {
111-
console.error(`Expected symbols in '${this.path}' did not match gold file.`);
128+
console.error(`Expected symbols in '${goldenFilename}' did not match gold file.`);
112129
passed = false;
113130
}
114131
const missingOrExtra = diff[key] > 0 ? 'extra' : 'missing';
@@ -124,26 +141,3 @@ function stripSuffix(text: string): string {
124141
const index = text.lastIndexOf('$');
125142
return index > -1 ? text.substring(0, index) : text;
126143
}
127-
128-
/**
129-
* This function detects a specific pattern that represents ESM modules
130-
* in the generated code. Those symbols are not really needed for the purposes
131-
* of symbol checking, since they only represent a module graph and all
132-
* nested symbols are being captured by the logic already. The pattern that
133-
* this function detects looks like this:
134-
* ```
135-
* var init_testability = __esm({
136-
* "packages/core/src/testability/testability.mjs"() {
137-
* // ...
138-
* }
139-
* });
140-
* ```
141-
*/
142-
function isEsmInitFunction(varDecl: ts.VariableDeclaration) {
143-
return (
144-
varDecl.name.getText().startsWith('init_') &&
145-
varDecl.initializer &&
146-
ts.isCallExpression(varDecl.initializer) &&
147-
(varDecl.initializer.expression as ts.Identifier).escapedText === '___esm'
148-
);
149-
}

‎tools/symbol-extractor/symbol_extractor_spec.mts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {SymbolExtractor} from './symbol_extractor.mjs';
1414

1515
describe('scenarios', () => {
1616
const symbolExtractorSpecDir = path.dirname(
17-
runfiles.resolve('angular/tools/symbol-extractor/symbol_extractor_spec/empty.json'),
17+
runfiles.resolve('angular/tools/symbol-extractor/symbol_extractor_spec/simple.json'),
1818
);
1919
const scenarioFiles = fs.readdirSync(symbolExtractorSpecDir);
2020
for (let i = 0; i < scenarioFiles.length; i++) {
@@ -37,8 +37,8 @@ describe('scenarios', () => {
3737
it(testName, () => {
3838
const jsFileContent = fs.readFileSync(path.join(symbolExtractorSpecDir, filePath)).toString();
3939
const jsonFileContent = fs.readFileSync(goldenFilePath).toString();
40-
const symbols = SymbolExtractor.parse(testName, jsFileContent);
41-
const diff = SymbolExtractor.diff(symbols, jsonFileContent);
40+
const {symbols} = SymbolExtractor.parse(jsFileContent);
41+
const diff = SymbolExtractor.diff(symbols, JSON.parse(jsonFileContent));
4242
expect(diff).toEqual({});
4343
});
4444
}
@@ -56,8 +56,14 @@ describe('scenarios', () => {
5656
const jsonFileContent = fs
5757
.readFileSync(path.join(symbolExtractorSpecDir, 'es2015_class_output.json'))
5858
.toString();
59-
const symbols = SymbolExtractor.parse('es2015_class_output', jsFileContent);
60-
const diff = SymbolExtractor.diff(symbols, jsonFileContent);
59+
const {symbols} = SymbolExtractor.parse(jsFileContent);
60+
const diff = SymbolExtractor.diff(symbols, JSON.parse(jsonFileContent) as string[]);
6161
expect(diff).toEqual({});
6262
});
63+
64+
it('should properly detect eaglery loaded files', () => {
65+
const jsFileContent = `import {} from './other_chunk';`;
66+
const {eagerlyLoadedRelativeSpecifiers} = SymbolExtractor.parse(jsFileContent);
67+
expect(eagerlyLoadedRelativeSpecifiers).toEqual(['./other_chunk']);
68+
});
6369
});

‎tools/symbol-extractor/symbol_extractor_spec/empty.js

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

‎tools/symbol-extractor/symbol_extractor_spec/empty.json

Lines changed: 0 additions & 1 deletion
This file was deleted.

0 commit comments

Comments
 (0)
0