Skip to content

Commit 265ef0d

Browse files
committed
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 9ed0505 commit 265ef0d

2 files changed

Lines changed: 192 additions & 12 deletions

File tree

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

Lines changed: 89 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,11 @@ 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+
return removals.has(node)
76+
? undefined
77+
: ts.visitEachChild(node, visitForRemoval, context);
78+
},
7579
context,
7680
);
7781
}
@@ -137,17 +141,30 @@ export function elideImports(
137141
return;
138142
}
139143

140-
// Type reference imports do not need to be emitted when emitDecoratorMetadata is disabled.
141-
if (ts.isTypeReferenceNode(node) && !compilerOptions.emitDecoratorMetadata) {
144+
// Type nodes do not need to be emitted when `emitDecoratorMetadata` is disabled.
145+
// `ExpressionWithTypeArguments` is excluded as it also represents runtime values such as
146+
// `class ... extends Base<T>` heritage clauses and instantiation expressions (`Map<string, T>`).
147+
if (
148+
ts.isTypeNode(node) &&
149+
!ts.isExpressionWithTypeArguments(node) &&
150+
!compilerOptions.emitDecoratorMetadata
151+
) {
142152
return;
143153
}
144154

145-
// Pure type declarations and type-only exports are erased at runtime.
155+
// Pure type declarations, type queries, ambient statements, external re-exports,
156+
// and type-only exports are erased or do not reference local imports at runtime.
146157
if (
147158
ts.isInterfaceDeclaration(node) ||
148159
ts.isTypeAliasDeclaration(node) ||
149160
ts.isTypeParameterDeclaration(node) ||
150-
(ts.isExportDeclaration(node) && node.isTypeOnly) ||
161+
ts.isTypeQueryNode(node) ||
162+
ts.isImportTypeNode(node) ||
163+
ts.isIndexSignatureDeclaration(node) ||
164+
(ts.isStatement(node) &&
165+
ts.canHaveModifiers(node) &&
166+
ts.getModifiers(node)?.some((m) => m.kind === ts.SyntaxKind.DeclareKeyword)) ||
167+
(ts.isExportDeclaration(node) && (node.isTypeOnly || node.moduleSpecifier !== undefined)) ||
151168
(ts.isExportSpecifier(node) && node.isTypeOnly)
152169
) {
153170
return;
@@ -184,15 +201,30 @@ export function elideImports(
184201
return importNodeRemovals;
185202
}
186203

187-
const isUnused = (node: ts.Identifier) => {
204+
/**
205+
* Determines how an imported binding should be handled:
206+
* - `keep`: the binding is still referenced.
207+
* - `value`: the binding is a runtime value that is no longer referenced.
208+
* - `type`: the binding does not exist at runtime and is always elided by TypeScript.
209+
*/
210+
const getRemovalKind = (node: ts.Identifier): 'keep' | 'value' | 'type' => {
188211
// Do not remove JSX factory imports
189212
if (node.text === compilerOptions.jsxFactory) {
190-
return false;
213+
return 'keep';
191214
}
192215

193216
const symbol = typeChecker.getSymbolAtLocation(node);
217+
if (!symbol) {
218+
return 'keep';
219+
}
220+
221+
// Note: with `verbatimModuleSyntax`, TypeScript reports an error for imports of non-value aliases
222+
// that do not use a `type` modifier (TS1484/TS1485), so no special handling is needed for it.
223+
if (symbol.flags & ts.SymbolFlags.Alias && !isValueAlias(symbol, typeChecker)) {
224+
return 'type';
225+
}
194226

195-
return symbol && !usedSymbols.has(symbol);
227+
return usedSymbols.has(symbol) ? 'keep' : 'value';
196228
};
197229

198230
for (const node of imports) {
@@ -205,20 +237,27 @@ export function elideImports(
205237

206238
if (namedBindings && ts.isNamespaceImport(namedBindings)) {
207239
// "import * as XYZ from 'abc';"
208-
if (isUnused(namedBindings.name)) {
240+
if (getRemovalKind(namedBindings.name) !== 'keep') {
209241
importNodeRemovals.add(node);
210242
}
211243
} else {
212244
const specifierNodeRemovals = [];
213245
let clausesCount = 0;
246+
// TypeScript skips its own import elision for declarations replaced by a transformer.
247+
// A declaration is therefore only partially rewritten when a runtime value import must be
248+
// removed, or when it was already replaced by a previous transformer. Otherwise, it is left
249+
// untouched so that TypeScript continues to elide any type-only specifiers itself.
250+
let requiresRewrite = !ts.isParseTreeNode(node);
214251

215252
// "import { XYZ, ... } from 'abc';"
216253
if (namedBindings && ts.isNamedImports(namedBindings)) {
217254
let removedClausesCount = 0;
218255
clausesCount += namedBindings.elements.length;
219256

220257
for (const specifier of namedBindings.elements) {
221-
if (specifier.isTypeOnly || isUnused(specifier.name)) {
258+
const removalKind = specifier.isTypeOnly ? 'type' : getRemovalKind(specifier.name);
259+
if (removalKind !== 'keep') {
260+
requiresRewrite ||= removalKind === 'value';
222261
removedClausesCount++;
223262
// in case we don't have any more namedImports we should remove the parent ie the {}
224263
const nodeToRemove =
@@ -233,14 +272,18 @@ export function elideImports(
233272
if (node.importClause.name) {
234273
clausesCount++;
235274

236-
if (node.importClause.isTypeOnly || isUnused(node.importClause.name)) {
275+
const removalKind = node.importClause.isTypeOnly
276+
? 'type'
277+
: getRemovalKind(node.importClause.name);
278+
if (removalKind !== 'keep') {
279+
requiresRewrite ||= removalKind === 'value';
237280
specifierNodeRemovals.push(node.importClause.name);
238281
}
239282
}
240283

241284
if (specifierNodeRemovals.length === clausesCount) {
242285
importNodeRemovals.add(node);
243-
} else {
286+
} else if (requiresRewrite) {
244287
for (const specifierNodeRemoval of specifierNodeRemovals) {
245288
importNodeRemovals.add(specifierNodeRemoval);
246289
}
@@ -250,3 +293,37 @@ export function elideImports(
250293

251294
return importNodeRemovals;
252295
}
296+
297+
/**
298+
* Determines whether an import alias resolves to a runtime value. Aliases that only resolve to
299+
* types, or that pass through a type-only import or export declaration anywhere along the alias
300+
* chain, are always elided by TypeScript.
301+
*/
302+
function isValueAlias(symbol: ts.Symbol, typeChecker: ts.TypeChecker): boolean {
303+
const seen = new Set<ts.Symbol>();
304+
let current = symbol;
305+
306+
while (current.flags & ts.SymbolFlags.Alias) {
307+
if (seen.has(current)) {
308+
// Circular alias (a TypeScript error) — assume runtime value so we don't over-elide.
309+
return true;
310+
}
311+
seen.add(current);
312+
313+
if (
314+
current.declarations?.length &&
315+
current.declarations.every(ts.isTypeOnlyImportOrExportDeclaration)
316+
) {
317+
return false;
318+
}
319+
320+
const next = typeChecker.getImmediateAliasedSymbol(current);
321+
if (!next) {
322+
// Unresolved alias (e.g., missing module) — assume runtime value so we don't over-elide.
323+
return true;
324+
}
325+
current = next;
326+
}
327+
328+
return (current.flags & ts.SymbolFlags.Value) !== 0;
329+
}
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)