Skip to content

Commit 174eb34

Browse files
clydinalan-agius4
authored andcommitted
fix(@angular/build): elide non-value aliases and remaining type-only AST constructs
Follow up to #34582 to apply the same import elision fixes made to `@ngtools/webpack` to the `replaceBootstrap` transformer used by the application builder: - Elide imported alias symbols that do not resolve to a runtime value (or traverse a type-only alias declaration), handling non-`type` interface re-exports (`export { Foo }`) and interface parameter types under `emitDecoratorMetadata`. - Only rewrite a partially elided import declaration when a runtime value import must be removed or when the declaration was already replaced by a previous transformer, so TypeScript's own import elision continues to apply to all other declarations. - Skip all non-`ExpressionWithTypeArguments` `TypeNode`s when `emitDecoratorMetadata` is disabled, and unconditionally skip `TypeQueryNode` (`typeof X`), `ImportTypeNode`, `IndexSignatureDeclaration`, ambient `declare` statements, and external `export ... from '...'` declarations. - Recursively visit child AST nodes when applying import removals in `replaceBootstrap` so individual `ImportSpecifier`s are removed when `platformBrowserDynamic` shares an import with other used runtime exports.
1 parent 9e6c1ce commit 174eb34

2 files changed

Lines changed: 198 additions & 12 deletions

File tree

‎packages/angular/build/src/tools/angular/transformers/jit-bootstrap-transformer.ts‎

Lines changed: 95 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,17 @@ export function replaceBootstrap(
7171
if (removals.size > 0) {
7272
updatedSourceFile = ts.visitEachChild(
7373
updatedSourceFile,
74-
(node) => (removals.has(node) ? undefined : node),
74+
function visitForRemoval(node): ts.Node | undefined {
75+
if (removals.has(node)) {
76+
return undefined;
77+
}
78+
79+
return ts.isImportDeclaration(node) ||
80+
ts.isImportClause(node) ||
81+
ts.isNamedImports(node)
82+
? ts.visitEachChild(node, visitForRemoval, context)
83+
: node;
84+
},
7585
context,
7686
);
7787
}
@@ -137,17 +147,30 @@ export function elideImports(
137147
return;
138148
}
139149

140-
// Type reference imports do not need to be emitted when emitDecoratorMetadata is disabled.
141-
if (ts.isTypeReferenceNode(node) && !compilerOptions.emitDecoratorMetadata) {
150+
// Type nodes do not need to be emitted when `emitDecoratorMetadata` is disabled.
151+
// `ExpressionWithTypeArguments` is excluded as it also represents runtime values such as
152+
// `class ... extends Base<T>` heritage clauses and instantiation expressions (`Map<string, T>`).
153+
if (
154+
ts.isTypeNode(node) &&
155+
!ts.isExpressionWithTypeArguments(node) &&
156+
!compilerOptions.emitDecoratorMetadata
157+
) {
142158
return;
143159
}
144160

145-
// Pure type declarations and type-only exports are erased at runtime.
161+
// Pure type declarations, type queries, ambient statements, external re-exports,
162+
// and type-only exports are erased or do not reference local imports at runtime.
146163
if (
147164
ts.isInterfaceDeclaration(node) ||
148165
ts.isTypeAliasDeclaration(node) ||
149166
ts.isTypeParameterDeclaration(node) ||
150-
(ts.isExportDeclaration(node) && node.isTypeOnly) ||
167+
ts.isTypeQueryNode(node) ||
168+
ts.isImportTypeNode(node) ||
169+
ts.isIndexSignatureDeclaration(node) ||
170+
(ts.isStatement(node) &&
171+
ts.canHaveModifiers(node) &&
172+
ts.getModifiers(node)?.some((m) => m.kind === ts.SyntaxKind.DeclareKeyword)) ||
173+
(ts.isExportDeclaration(node) && (node.isTypeOnly || node.moduleSpecifier !== undefined)) ||
151174
(ts.isExportSpecifier(node) && node.isTypeOnly)
152175
) {
153176
return;
@@ -184,15 +207,30 @@ export function elideImports(
184207
return importNodeRemovals;
185208
}
186209

187-
const isUnused = (node: ts.Identifier) => {
210+
/**
211+
* Determines how an imported binding should be handled:
212+
* - `keep`: the binding is still referenced.
213+
* - `value`: the binding is a runtime value that is no longer referenced.
214+
* - `type`: the binding does not exist at runtime and is always elided by TypeScript.
215+
*/
216+
const getRemovalKind = (node: ts.Identifier): 'keep' | 'value' | 'type' => {
188217
// Do not remove JSX factory imports
189218
if (node.text === compilerOptions.jsxFactory) {
190-
return false;
219+
return 'keep';
191220
}
192221

193222
const symbol = typeChecker.getSymbolAtLocation(node);
223+
if (!symbol) {
224+
return 'keep';
225+
}
226+
227+
// Note: with `verbatimModuleSyntax`, TypeScript reports an error for imports of non-value aliases
228+
// that do not use a `type` modifier (TS1484/TS1485), so no special handling is needed for it.
229+
if ((symbol.flags & ts.SymbolFlags.Alias) !== 0 && !isValueAlias(symbol, typeChecker)) {
230+
return 'type';
231+
}
194232

195-
return symbol && !usedSymbols.has(symbol);
233+
return usedSymbols.has(symbol) ? 'keep' : 'value';
196234
};
197235

198236
for (const node of imports) {
@@ -205,20 +243,27 @@ export function elideImports(
205243

206244
if (namedBindings && ts.isNamespaceImport(namedBindings)) {
207245
// "import * as XYZ from 'abc';"
208-
if (isUnused(namedBindings.name)) {
246+
if (getRemovalKind(namedBindings.name) !== 'keep') {
209247
importNodeRemovals.add(node);
210248
}
211249
} else {
212250
const specifierNodeRemovals = [];
213251
let clausesCount = 0;
252+
// TypeScript skips its own import elision for declarations replaced by a transformer.
253+
// A declaration is therefore only partially rewritten when a runtime value import must be
254+
// removed, or when it was already replaced by a previous transformer. Otherwise, it is left
255+
// untouched so that TypeScript continues to elide any type-only specifiers itself.
256+
let requiresRewrite = !ts.isParseTreeNode(node);
214257

215258
// "import { XYZ, ... } from 'abc';"
216259
if (namedBindings && ts.isNamedImports(namedBindings)) {
217260
let removedClausesCount = 0;
218261
clausesCount += namedBindings.elements.length;
219262

220263
for (const specifier of namedBindings.elements) {
221-
if (specifier.isTypeOnly || isUnused(specifier.name)) {
264+
const removalKind = specifier.isTypeOnly ? 'type' : getRemovalKind(specifier.name);
265+
if (removalKind !== 'keep') {
266+
requiresRewrite ||= removalKind === 'value';
222267
removedClausesCount++;
223268
// in case we don't have any more namedImports we should remove the parent ie the {}
224269
const nodeToRemove =
@@ -233,14 +278,18 @@ export function elideImports(
233278
if (node.importClause.name) {
234279
clausesCount++;
235280

236-
if (node.importClause.isTypeOnly || isUnused(node.importClause.name)) {
281+
const removalKind = node.importClause.isTypeOnly
282+
? 'type'
283+
: getRemovalKind(node.importClause.name);
284+
if (removalKind !== 'keep') {
285+
requiresRewrite ||= removalKind === 'value';
237286
specifierNodeRemovals.push(node.importClause.name);
238287
}
239288
}
240289

241290
if (specifierNodeRemovals.length === clausesCount) {
242291
importNodeRemovals.add(node);
243-
} else {
292+
} else if (requiresRewrite) {
244293
for (const specifierNodeRemoval of specifierNodeRemovals) {
245294
importNodeRemovals.add(specifierNodeRemoval);
246295
}
@@ -250,3 +299,37 @@ export function elideImports(
250299

251300
return importNodeRemovals;
252301
}
302+
303+
/**
304+
* Determines whether an import alias resolves to a runtime value. Aliases that only resolve to
305+
* types, or that pass through a type-only import or export declaration anywhere along the alias
306+
* chain, are always elided by TypeScript.
307+
*/
308+
function isValueAlias(symbol: ts.Symbol, typeChecker: ts.TypeChecker): boolean {
309+
const seen = new Set<ts.Symbol>();
310+
let current = symbol;
311+
312+
while ((current.flags & ts.SymbolFlags.Alias) !== 0) {
313+
if (seen.has(current)) {
314+
// Circular alias (a TypeScript error) — assume runtime value so we don't over-elide.
315+
return true;
316+
}
317+
seen.add(current);
318+
319+
if (
320+
current.declarations?.length &&
321+
current.declarations.every(ts.isTypeOnlyImportOrExportDeclaration)
322+
) {
323+
return false;
324+
}
325+
326+
const next = typeChecker.getImmediateAliasedSymbol(current);
327+
if (!next) {
328+
// Unresolved alias (e.g., missing module) — assume runtime value so we don't over-elide.
329+
return true;
330+
}
331+
current = next;
332+
}
333+
334+
return (current.flags & ts.SymbolFlags.Value) !== 0;
335+
}
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
import ts from 'typescript';
10+
import { replaceBootstrap } from './jit-bootstrap-transformer';
11+
12+
describe('replaceBootstrap', () => {
13+
function transformCode(
14+
files: Record<string, string>,
15+
entryFile = '/project/src/main.ts',
16+
): string {
17+
const compilerOptions: ts.CompilerOptions = {
18+
target: ts.ScriptTarget.ES2022,
19+
module: ts.ModuleKind.ES2022,
20+
moduleResolution: ts.ModuleResolutionKind.Bundler,
21+
noLib: true,
22+
};
23+
24+
const host: ts.CompilerHost = {
25+
getSourceFile: (fileName, languageVersion) =>
26+
files[fileName] !== undefined
27+
? ts.createSourceFile(fileName, files[fileName], languageVersion, true)
28+
: undefined,
29+
getDefaultLibFileName: () => '/lib.d.ts',
30+
writeFile: () => undefined,
31+
getCurrentDirectory: () => '/project',
32+
getCanonicalFileName: (fileName) => fileName,
33+
useCaseSensitiveFileNames: () => true,
34+
getNewLine: () => '\n',
35+
fileExists: (fileName) => files[fileName] !== undefined,
36+
readFile: (fileName) => files[fileName],
37+
};
38+
39+
const program = ts.createProgram([entryFile], compilerOptions, host);
40+
let output = '';
41+
const sourceFile = program.getSourceFile(entryFile);
42+
expect(sourceFile).toBeDefined();
43+
44+
program.emit(
45+
sourceFile,
46+
(_fileName, text) => {
47+
output = text;
48+
},
49+
undefined,
50+
false,
51+
{ before: [replaceBootstrap(() => program.getTypeChecker())] },
52+
);
53+
54+
return output;
55+
}
56+
57+
const platformBrowserDynamicStub = `
58+
export declare function platformBrowserDynamic(extraProviders?: unknown[]): {
59+
bootstrapModule(moduleType: unknown): Promise<unknown>;
60+
};
61+
export const VERSION = '0.0.0';
62+
export interface PlatformHelper {}
63+
`;
64+
65+
it('replaces platformBrowserDynamic and removes its import when no other specifiers remain', () => {
66+
const output = transformCode({
67+
'/project/node_modules/@angular/platform-browser-dynamic/index.d.ts':
68+
platformBrowserDynamicStub,
69+
'/project/src/app.module.ts': 'export class AppModule {}',
70+
'/project/src/main.ts': `
71+
import { platformBrowserDynamic, PlatformHelper } from '@angular/platform-browser-dynamic';
72+
import { AppModule } from './app.module';
73+
74+
export { PlatformHelper };
75+
platformBrowserDynamic().bootstrapModule(AppModule);
76+
`,
77+
});
78+
79+
expect(output).toContain(`import * as __NgCli_bootstrap_1 from "@angular/platform-browser";`);
80+
expect(output).toContain(`__NgCli_bootstrap_1.platformBrowser().bootstrapModule(AppModule);`);
81+
expect(output).not.toContain('@angular/platform-browser-dynamic');
82+
});
83+
84+
it('removes only platformBrowserDynamic specifier when another runtime export from the same module is used', () => {
85+
const output = transformCode({
86+
'/project/node_modules/@angular/platform-browser-dynamic/index.d.ts':
87+
platformBrowserDynamicStub,
88+
'/project/src/app.module.ts': 'export class AppModule {}',
89+
'/project/src/main.ts': `
90+
import { platformBrowserDynamic, VERSION, PlatformHelper } from '@angular/platform-browser-dynamic';
91+
import { AppModule } from './app.module';
92+
93+
export type { PlatformHelper };
94+
export const version = VERSION;
95+
platformBrowserDynamic().bootstrapModule(AppModule);
96+
`,
97+
});
98+
99+
expect(output).toContain(`import { VERSION } from '@angular/platform-browser-dynamic';`);
100+
expect(output).not.toContain('PlatformHelper');
101+
expect(output).toContain(`__NgCli_bootstrap_1.platformBrowser().bootstrapModule(AppModule);`);
102+
});
103+
});

0 commit comments

Comments
 (0)