Skip to content

Commit 9e6c1ce

Browse files
clydinalan-agius4
authored andcommitted
fix(@ngtools/webpack): elide non-value aliases and remaining type-only AST constructs
Follow up to #34582 to cover remaining pure-type import elision cases and partial specifier removal in `replaceBootstrap`: - 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. TypeScript skips its own import elision for declarations replaced by a transformer, so leaving other declarations untouched avoids retaining specifiers that TypeScript would have otherwise elided (such as inlined `const enum` imports). - 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 `elideImports` removals in `replaceBootstrap` so individual `ImportSpecifier`s are removed when `platformBrowserDynamic` shares an import with other used runtime exports.
1 parent 2958bf8 commit 9e6c1ce

4 files changed

Lines changed: 542 additions & 12 deletions

File tree

‎packages/ngtools/webpack/src/ivy/transformation.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,17 @@ export function replaceBootstrap(
149149
if (removals.size > 0) {
150150
updatedSourceFile = ts.visitEachChild(
151151
updatedSourceFile,
152-
(node) => (removals.has(node) ? undefined : node),
152+
function visitForRemoval(node): ts.Node | undefined {
153+
if (removals.has(node)) {
154+
return undefined;
155+
}
156+
157+
return ts.isImportDeclaration(node) ||
158+
ts.isImportClause(node) ||
159+
ts.isNamedImports(node)
160+
? ts.visitEachChild(node, visitForRemoval, context)
161+
: node;
162+
},
153163
context,
154164
);
155165
}
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 './transformation';
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+
});

‎packages/ngtools/webpack/src/transformers/elide_imports.ts‎

Lines changed: 84 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -54,17 +54,30 @@ export function elideImports(
5454
return;
5555
}
5656

57-
// Type reference imports do not need to be emitted when emitDecoratorMetadata is disabled.
58-
if (ts.isTypeReferenceNode(node) && !compilerOptions.emitDecoratorMetadata) {
57+
// Type nodes do not need to be emitted when `emitDecoratorMetadata` is disabled.
58+
// `ExpressionWithTypeArguments` is excluded as it also represents runtime values such as
59+
// `class ... extends Base<T>` heritage clauses and instantiation expressions (`Map<string, T>`).
60+
if (
61+
ts.isTypeNode(node) &&
62+
!ts.isExpressionWithTypeArguments(node) &&
63+
!compilerOptions.emitDecoratorMetadata
64+
) {
5965
return;
6066
}
6167

62-
// Pure type declarations and type-only exports are erased at runtime.
68+
// Pure type declarations, type queries, ambient statements, external re-exports,
69+
// and type-only exports are erased or do not reference local imports at runtime.
6370
if (
6471
ts.isInterfaceDeclaration(node) ||
6572
ts.isTypeAliasDeclaration(node) ||
6673
ts.isTypeParameterDeclaration(node) ||
67-
(ts.isExportDeclaration(node) && node.isTypeOnly) ||
74+
ts.isTypeQueryNode(node) ||
75+
ts.isImportTypeNode(node) ||
76+
ts.isIndexSignatureDeclaration(node) ||
77+
(ts.isStatement(node) &&
78+
ts.canHaveModifiers(node) &&
79+
ts.getModifiers(node)?.some((m) => m.kind === ts.SyntaxKind.DeclareKeyword)) ||
80+
(ts.isExportDeclaration(node) && (node.isTypeOnly || node.moduleSpecifier !== undefined)) ||
6881
(ts.isExportSpecifier(node) && node.isTypeOnly)
6982
) {
7083
return;
@@ -101,15 +114,30 @@ export function elideImports(
101114
return importNodeRemovals;
102115
}
103116

104-
const isUnused = (node: ts.Identifier) => {
117+
/**
118+
* Determines how an imported binding should be handled:
119+
* - `keep`: the binding is still referenced.
120+
* - `value`: the binding is a runtime value that is no longer referenced.
121+
* - `type`: the binding does not exist at runtime and is always elided by TypeScript.
122+
*/
123+
const getRemovalKind = (node: ts.Identifier): 'keep' | 'value' | 'type' => {
105124
// Do not remove JSX factory imports
106125
if (node.text === compilerOptions.jsxFactory) {
107-
return false;
126+
return 'keep';
108127
}
109128

110129
const symbol = typeChecker.getSymbolAtLocation(node);
130+
if (!symbol) {
131+
return 'keep';
132+
}
133+
134+
// Note: with `verbatimModuleSyntax`, TypeScript reports an error for imports of non-value aliases
135+
// that do not use a `type` modifier (TS1484/TS1485), so no special handling is needed for it.
136+
if ((symbol.flags & ts.SymbolFlags.Alias) !== 0 && !isValueAlias(symbol, typeChecker)) {
137+
return 'type';
138+
}
111139

112-
return symbol && !usedSymbols.has(symbol);
140+
return usedSymbols.has(symbol) ? 'keep' : 'value';
113141
};
114142

115143
for (const node of imports) {
@@ -122,20 +150,27 @@ export function elideImports(
122150

123151
if (namedBindings && ts.isNamespaceImport(namedBindings)) {
124152
// "import * as XYZ from 'abc';"
125-
if (isUnused(namedBindings.name)) {
153+
if (getRemovalKind(namedBindings.name) !== 'keep') {
126154
importNodeRemovals.add(node);
127155
}
128156
} else {
129157
const specifierNodeRemovals = [];
130158
let clausesCount = 0;
159+
// TypeScript skips its own import elision for declarations replaced by a transformer.
160+
// A declaration is therefore only partially rewritten when a runtime value import must be
161+
// removed, or when it was already replaced by a previous transformer. Otherwise, it is left
162+
// untouched so that TypeScript continues to elide any type-only specifiers itself.
163+
let requiresRewrite = !ts.isParseTreeNode(node);
131164

132165
// "import { XYZ, ... } from 'abc';"
133166
if (namedBindings && ts.isNamedImports(namedBindings)) {
134167
let removedClausesCount = 0;
135168
clausesCount += namedBindings.elements.length;
136169

137170
for (const specifier of namedBindings.elements) {
138-
if (specifier.isTypeOnly || isUnused(specifier.name)) {
171+
const removalKind = specifier.isTypeOnly ? 'type' : getRemovalKind(specifier.name);
172+
if (removalKind !== 'keep') {
173+
requiresRewrite ||= removalKind === 'value';
139174
removedClausesCount++;
140175
// in case we don't have any more namedImports we should remove the parent ie the {}
141176
const nodeToRemove =
@@ -150,14 +185,18 @@ export function elideImports(
150185
if (node.importClause.name) {
151186
clausesCount++;
152187

153-
if (node.importClause.isTypeOnly || isUnused(node.importClause.name)) {
188+
const removalKind = node.importClause.isTypeOnly
189+
? 'type'
190+
: getRemovalKind(node.importClause.name);
191+
if (removalKind !== 'keep') {
192+
requiresRewrite ||= removalKind === 'value';
154193
specifierNodeRemovals.push(node.importClause.name);
155194
}
156195
}
157196

158197
if (specifierNodeRemovals.length === clausesCount) {
159198
importNodeRemovals.add(node);
160-
} else {
199+
} else if (requiresRewrite) {
161200
for (const specifierNodeRemoval of specifierNodeRemovals) {
162201
importNodeRemovals.add(specifierNodeRemoval);
163202
}
@@ -167,3 +206,37 @@ export function elideImports(
167206

168207
return importNodeRemovals;
169208
}
209+
210+
/**
211+
* Determines whether an import alias resolves to a runtime value. Aliases that only resolve to
212+
* types, or that pass through a type-only import or export declaration anywhere along the alias
213+
* chain, are always elided by TypeScript.
214+
*/
215+
function isValueAlias(symbol: ts.Symbol, typeChecker: ts.TypeChecker): boolean {
216+
const seen = new Set<ts.Symbol>();
217+
let current = symbol;
218+
219+
while ((current.flags & ts.SymbolFlags.Alias) !== 0) {
220+
if (seen.has(current)) {
221+
// Circular alias (a TypeScript error) — assume runtime value so we don't over-elide.
222+
return true;
223+
}
224+
seen.add(current);
225+
226+
if (
227+
current.declarations?.length &&
228+
current.declarations.every(ts.isTypeOnlyImportOrExportDeclaration)
229+
) {
230+
return false;
231+
}
232+
233+
const next = typeChecker.getImmediateAliasedSymbol(current);
234+
if (!next) {
235+
// Unresolved alias (e.g., missing module) — assume runtime value so we don't over-elide.
236+
return true;
237+
}
238+
current = next;
239+
}
240+
241+
return (current.flags & ts.SymbolFlags.Value) !== 0;
242+
}

0 commit comments

Comments
 (0)