Skip to content

Commit 07a8016

Browse files
petebacondarwinatscott
authored andcommitted
fix(ngcc): capture dynamic import expressions as well as declarations (angular#37075)
Previously we only checked for static import declaration statements. This commit also finds import paths from dynamic import expressions. Also this commit should speed up processing: Previously we were parsing the source code contents into a `ts.SourceFile` and then walking the parsed AST to find import paths. Generating an AST is unnecessary work and it is faster and creates less memory pressure to just scan the source code contents with the TypeScript scanner, identifying import paths from the tokens. PR Close angular#37075
1 parent 4d69da5 commit 07a8016

File tree

2 files changed

+233
-9
lines changed

2 files changed

+233
-9
lines changed

packages/compiler-cli/ngcc/src/dependencies/esm_dependency_host.ts

Lines changed: 215 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,20 +13,204 @@ import {DependencyHostBase} from './dependency_host';
1313
* Helper functions for computing dependencies.
1414
*/
1515
export class EsmDependencyHost extends DependencyHostBase {
16+
// By skipping trivia here we don't have to account for it in the processing below
17+
// It has no relevance to capturing imports.
18+
private scanner = ts.createScanner(ts.ScriptTarget.Latest, /* skipTrivia */ true);
19+
1620
protected canSkipFile(fileContents: string): boolean {
1721
return !hasImportOrReexportStatements(fileContents);
1822
}
1923

2024
protected extractImports(file: AbsoluteFsPath, fileContents: string): Set<string> {
21-
const imports: string[] = [];
22-
// Parse the source into a TypeScript AST and then walk it looking for imports and re-exports.
23-
const sf =
24-
ts.createSourceFile(file, fileContents, ts.ScriptTarget.ES2015, false, ts.ScriptKind.JS);
25-
return new Set(sf.statements
26-
// filter out statements that are not imports or reexports
27-
.filter(isStringImportOrReexport)
28-
// Grab the id of the module that is being imported
29-
.map(stmt => stmt.moduleSpecifier.text));
25+
const imports = new Set<string>();
26+
const templateStack: ts.SyntaxKind[] = [];
27+
let lastToken: ts.SyntaxKind = ts.SyntaxKind.Unknown;
28+
let currentToken: ts.SyntaxKind = ts.SyntaxKind.Unknown;
29+
30+
this.scanner.setText(fileContents);
31+
32+
while ((currentToken = this.scanner.scan()) !== ts.SyntaxKind.EndOfFileToken) {
33+
switch (currentToken) {
34+
case ts.SyntaxKind.TemplateHead:
35+
templateStack.push(currentToken);
36+
break;
37+
case ts.SyntaxKind.OpenBraceToken:
38+
if (templateStack.length > 0) {
39+
templateStack.push(currentToken);
40+
}
41+
break;
42+
case ts.SyntaxKind.CloseBraceToken:
43+
if (templateStack.length > 0) {
44+
const templateToken = templateStack[templateStack.length - 1];
45+
if (templateToken === ts.SyntaxKind.TemplateHead) {
46+
currentToken = this.scanner.reScanTemplateToken(/* isTaggedTemplate */ false);
47+
if (currentToken === ts.SyntaxKind.TemplateTail) {
48+
templateStack.pop();
49+
}
50+
} else {
51+
templateStack.pop();
52+
}
53+
}
54+
break;
55+
case ts.SyntaxKind.SlashToken:
56+
case ts.SyntaxKind.SlashEqualsToken:
57+
if (canPrecedeARegex(lastToken)) {
58+
currentToken = this.scanner.reScanSlashToken();
59+
}
60+
break;
61+
case ts.SyntaxKind.ImportKeyword:
62+
const importPath = this.extractImportPath();
63+
if (importPath !== null) {
64+
imports.add(importPath);
65+
}
66+
break;
67+
case ts.SyntaxKind.ExportKeyword:
68+
const reexportPath = this.extractReexportPath();
69+
if (reexportPath !== null) {
70+
imports.add(reexportPath);
71+
}
72+
break;
73+
}
74+
lastToken = currentToken;
75+
}
76+
77+
// Clear the text from the scanner.
78+
this.scanner.setText('');
79+
80+
return imports;
81+
}
82+
83+
84+
/**
85+
* We have found an `import` token so now try to identify the import path.
86+
*
87+
* This method will use the current state of `this.scanner` to extract a string literal module
88+
* specifier. It expects that the current state of the scanner is that an `import` token has just
89+
* been scanned.
90+
*
91+
* The following forms of import are matched:
92+
*
93+
* * `import "module-specifier";`
94+
* * `import("module-specifier")`
95+
* * `import defaultBinding from "module-specifier";`
96+
* * `import defaultBinding, * as identifier from "module-specifier";`
97+
* * `import defaultBinding, {...} from "module-specifier";`
98+
* * `import * as identifier from "module-specifier";`
99+
* * `import {...} from "module-specifier";`
100+
*
101+
* @returns the import path or null if there is no import or it is not a string literal.
102+
*/
103+
protected extractImportPath(): string|null {
104+
// Check for side-effect import
105+
let sideEffectImportPath = this.tryStringLiteral();
106+
if (sideEffectImportPath !== null) {
107+
return sideEffectImportPath;
108+
}
109+
110+
let kind: ts.SyntaxKind|null = this.scanner.getToken();
111+
112+
// Check for dynamic import expression
113+
if (kind === ts.SyntaxKind.OpenParenToken) {
114+
return this.tryStringLiteral();
115+
}
116+
117+
// Check for defaultBinding
118+
if (kind === ts.SyntaxKind.Identifier) {
119+
// Skip default binding
120+
kind = this.scanner.scan();
121+
if (kind === ts.SyntaxKind.CommaToken) {
122+
// Skip comma that indicates additional import bindings
123+
kind = this.scanner.scan();
124+
}
125+
}
126+
127+
// Check for namespace import clause
128+
if (kind === ts.SyntaxKind.AsteriskToken) {
129+
kind = this.skipNamespacedClause();
130+
if (kind === null) {
131+
return null;
132+
}
133+
}
134+
// Check for named imports clause
135+
else if (kind === ts.SyntaxKind.OpenBraceToken) {
136+
kind = this.skipNamedClause();
137+
}
138+
139+
// Expect a `from` clause, if not bail out
140+
if (kind !== ts.SyntaxKind.FromKeyword) {
141+
return null;
142+
}
143+
144+
return this.tryStringLiteral();
145+
}
146+
147+
/**
148+
* We have found an `export` token so now try to identify a re-export path.
149+
*
150+
* This method will use the current state of `this.scanner` to extract a string literal module
151+
* specifier. It expects that the current state of the scanner is that an `export` token has
152+
* just been scanned.
153+
*
154+
* There are three forms of re-export that are matched:
155+
*
156+
* * `export * from '...';
157+
* * `export * as alias from '...';
158+
* * `export {...} from '...';
159+
*/
160+
protected extractReexportPath(): string|null {
161+
// Skip the `export` keyword
162+
let token: ts.SyntaxKind|null = this.scanner.scan();
163+
if (token === ts.SyntaxKind.AsteriskToken) {
164+
token = this.skipNamespacedClause();
165+
if (token === null) {
166+
return null;
167+
}
168+
} else if (token === ts.SyntaxKind.OpenBraceToken) {
169+
token = this.skipNamedClause();
170+
}
171+
// Expect a `from` clause, if not bail out
172+
if (token !== ts.SyntaxKind.FromKeyword) {
173+
return null;
174+
}
175+
return this.tryStringLiteral();
176+
}
177+
178+
protected skipNamespacedClause(): ts.SyntaxKind|null {
179+
// Skip past the `*`
180+
let token = this.scanner.scan();
181+
// Check for a `* as identifier` alias clause
182+
if (token === ts.SyntaxKind.AsKeyword) {
183+
// Skip past the `as` keyword
184+
token = this.scanner.scan();
185+
// Expect an identifier, if not bail out
186+
if (token !== ts.SyntaxKind.Identifier) {
187+
return null;
188+
}
189+
// Skip past the identifier
190+
token = this.scanner.scan();
191+
}
192+
return token;
193+
}
194+
195+
protected skipNamedClause(): ts.SyntaxKind {
196+
let braceCount = 1;
197+
// Skip past the initial opening brace `{`
198+
let token = this.scanner.scan();
199+
// Search for the matching closing brace `}`
200+
while (braceCount > 0 && token !== ts.SyntaxKind.EndOfFileToken) {
201+
if (token === ts.SyntaxKind.OpenBraceToken) {
202+
braceCount++;
203+
} else if (token === ts.SyntaxKind.CloseBraceToken) {
204+
braceCount--;
205+
}
206+
token = this.scanner.scan();
207+
}
208+
return token;
209+
}
210+
211+
protected tryStringLiteral(): string|null {
212+
return this.scanner.scan() === ts.SyntaxKind.StringLiteral ? this.scanner.getTokenValue() :
213+
null;
30214
}
31215
}
32216

@@ -56,3 +240,25 @@ export function isStringImportOrReexport(stmt: ts.Statement): stmt is ts.ImportD
56240
ts.isExportDeclaration(stmt) && !!stmt.moduleSpecifier &&
57241
ts.isStringLiteral(stmt.moduleSpecifier);
58242
}
243+
244+
245+
function canPrecedeARegex(kind: ts.SyntaxKind): boolean {
246+
switch (kind) {
247+
case ts.SyntaxKind.Identifier:
248+
case ts.SyntaxKind.StringLiteral:
249+
case ts.SyntaxKind.NumericLiteral:
250+
case ts.SyntaxKind.BigIntLiteral:
251+
case ts.SyntaxKind.RegularExpressionLiteral:
252+
case ts.SyntaxKind.ThisKeyword:
253+
case ts.SyntaxKind.PlusPlusToken:
254+
case ts.SyntaxKind.MinusMinusToken:
255+
case ts.SyntaxKind.CloseParenToken:
256+
case ts.SyntaxKind.CloseBracketToken:
257+
case ts.SyntaxKind.CloseBraceToken:
258+
case ts.SyntaxKind.TrueKeyword:
259+
case ts.SyntaxKind.FalseKeyword:
260+
return false;
261+
default:
262+
return true;
263+
}
264+
}

packages/compiler-cli/ngcc/test/dependencies/esm_dependency_host_spec.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,17 @@ runInEachFileSystem(() => {
5757
expect(dependencies.has(_('/node_modules/lib-1/sub-1'))).toBe(true);
5858
});
5959

60+
it('should resolve all the external dynamic imports of the source file', () => {
61+
const {dependencies, missing, deepImports} = createDependencyInfo();
62+
host.collectDependencies(
63+
_('/external/dynamic/index.js'), {dependencies, missing, deepImports});
64+
expect(dependencies.size).toBe(2);
65+
expect(missing.size).toBe(0);
66+
expect(deepImports.size).toBe(0);
67+
expect(dependencies.has(_('/node_modules/lib-1'))).toBe(true);
68+
expect(dependencies.has(_('/node_modules/lib-1/sub-1'))).toBe(true);
69+
});
70+
6071
it('should capture missing external imports', () => {
6172
const {dependencies, missing, deepImports} = createDependencyInfo();
6273
host.collectDependencies(
@@ -184,6 +195,13 @@ runInEachFileSystem(() => {
184195
},
185196
{name: _('/external/imports/package.json'), contents: '{"esm2015": "./index.js"}'},
186197
{name: _('/external/imports/index.metadata.json'), contents: 'MOCK METADATA'},
198+
{
199+
name: _('/external/dynamic/index.js'),
200+
contents:
201+
`async function foo() { await const x = import('lib-1');\n const promise = import('lib-1/sub-1'); }`
202+
},
203+
{name: _('/external/dynamic/package.json'), contents: '{"esm2015": "./index.js"}'},
204+
{name: _('/external/dynamic/index.metadata.json'), contents: 'MOCK METADATA'},
187205
{
188206
name: _('/external/re-exports/index.js'),
189207
contents: `export {X} from 'lib-1';\nexport {\n Y,\n Z\n} from 'lib-1/sub-1';`

0 commit comments

Comments
 (0)