Skip to content

Commit 2c3295d

Browse files
alan-agius4mgechev
authored andcommitted
fix(@ngtools/webpack): don't elide imports for type references that are needed for decorator metadata
When `emitDecoratorMetadata` is set to true we don't elide type references imports that are used for decorated nodes. Fixes #16808
1 parent 8817385 commit 2c3295d

File tree

6 files changed

+312
-29
lines changed

6 files changed

+312
-29
lines changed

packages/ngtools/webpack/src/transformers/ast_helpers.ts

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,14 @@ export function getLastNode(sourceFile: ts.SourceFile): ts.Node | null {
5050
// Test transform helpers.
5151
const basePath = '/project/src/';
5252
const fileName = basePath + 'test-file.ts';
53+
const typeScriptLibFiles = loadTypeScriptLibFiles();
5354
const tsLibFiles = loadTsLibFiles();
5455

5556
export function createTypescriptContext(
5657
content: string,
5758
additionalFiles?: Record<string, string>,
5859
useLibs = false,
59-
importHelpers = true,
60+
extraCompilerOptions: ts.CompilerOptions = {},
6061
) {
6162
// Set compiler options.
6263
const compilerOptions: ts.CompilerOptions = {
@@ -68,7 +69,9 @@ export function createTypescriptContext(
6869
target: ts.ScriptTarget.ESNext,
6970
skipLibCheck: true,
7071
sourceMap: false,
71-
importHelpers,
72+
importHelpers: true,
73+
experimentalDecorators: true,
74+
...extraCompilerOptions,
7275
};
7376

7477
// Create compiler host.
@@ -86,11 +89,17 @@ export function createTypescriptContext(
8689
// Write the default libs.
8790
// These are needed for tests that use import(), because it relies on a Promise being there.
8891
const compilerLibFolder = dirname(compilerHost.getDefaultLibFileName(compilerOptions));
89-
for (const [k, v] of Object.entries(tsLibFiles)) {
92+
for (const [k, v] of Object.entries(typeScriptLibFiles)) {
9093
compilerHost.writeFile(join(compilerLibFolder, k), v, false);
9194
}
9295
}
9396

97+
if (compilerOptions.importHelpers) {
98+
for (const [k, v] of Object.entries(tsLibFiles)) {
99+
compilerHost.writeFile(k, v, false);
100+
}
101+
}
102+
94103
if (additionalFiles) {
95104
for (const key in additionalFiles) {
96105
compilerHost.writeFile(basePath + key, additionalFiles[key], false);
@@ -108,8 +117,7 @@ export function transformTypescript(
108117
transformers: ts.TransformerFactory<ts.SourceFile>[],
109118
program?: ts.Program,
110119
compilerHost?: WebpackCompilerHost,
111-
) {
112-
120+
): string | undefined {
113121
// Use given context or create a new one.
114122
if (content !== undefined) {
115123
const typescriptContext = createTypescriptContext(content);
@@ -137,18 +145,29 @@ export function transformTypescript(
137145
return compilerHost.readFile(fileName.replace(/\.tsx?$/, '.js'));
138146
}
139147

140-
function loadTsLibFiles() {
148+
function loadTypeScriptLibFiles(): Record<string, string> {
141149
const libFolderPath = dirname(require.resolve('typescript/lib/lib.d.ts'));
142150
const libFolderFiles = readdirSync(libFolderPath);
143151
const libFileNames = libFolderFiles.filter(f => f.startsWith('lib.') && f.endsWith('.d.ts'));
144152

145153
// Return a map of the lib names to their content.
146-
return libFileNames.reduce(
147-
(map, f) => {
148-
map[f] = readFileSync(join(libFolderPath, f), 'utf-8');
154+
const libs: Record<string, string> = {};
155+
for (const f of libFileNames) {
156+
libs[f] = readFileSync(join(libFolderPath, f), 'utf-8');
157+
}
158+
159+
return libs;
160+
}
161+
162+
function loadTsLibFiles(): Record<string, string> {
163+
const libFolderPath = dirname(require.resolve('tslib/package.json'));
164+
const libFolderFiles = readdirSync(libFolderPath);
165+
166+
// Return a map of the lib names to their content.
167+
const libs: Record<string, string> = {};
168+
for (const f of libFolderFiles) {
169+
libs[join('node_modules/tslib', f)] = readFileSync(join(libFolderPath, f), 'utf-8');
170+
}
149171

150-
return map;
151-
},
152-
{} as { [k: string]: string },
153-
);
172+
return libs;
154173
}

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

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ export function elideImports(
1919
sourceFile: ts.SourceFile,
2020
removedNodes: ts.Node[],
2121
getTypeChecker: () => ts.TypeChecker,
22+
compilerOptions: ts.CompilerOptions,
2223
): TransformOperation[] {
2324
const ops: TransformOperation[] = [];
2425

@@ -33,8 +34,8 @@ export function elideImports(
3334
const imports: ts.ImportDeclaration[] = [];
3435

3536
ts.forEachChild(sourceFile, function visit(node) {
36-
// Skip type references and removed nodes. We consider both unused.
37-
if (node.kind == ts.SyntaxKind.TypeReference || removedNodes.includes(node)) {
37+
// Skip removed nodes.
38+
if (removedNodes.includes(node)) {
3839
return;
3940
}
4041

@@ -46,17 +47,48 @@ export function elideImports(
4647
}
4748

4849
let symbol: ts.Symbol | undefined;
50+
if (ts.isTypeReferenceNode(node)) {
51+
if (!compilerOptions.emitDecoratorMetadata) {
52+
// Skip and mark as unused if emitDecoratorMetadata is disabled.
53+
return;
54+
}
4955

50-
switch (node.kind) {
51-
case ts.SyntaxKind.Identifier:
56+
const parent = node.parent;
57+
let isTypeReferenceForDecoratoredNode = false;
58+
59+
switch (parent.kind) {
60+
case ts.SyntaxKind.GetAccessor:
61+
case ts.SyntaxKind.PropertyDeclaration:
62+
case ts.SyntaxKind.MethodDeclaration:
63+
isTypeReferenceForDecoratoredNode = !!parent.decorators?.length;
64+
break;
65+
case ts.SyntaxKind.Parameter:
66+
// - A constructor parameter can be decorated or the class itself is decorated.
67+
// - The parent of the parameter is decorated example a method declaration or a set accessor.
68+
// In all cases we need the type reference not to be elided.
69+
isTypeReferenceForDecoratoredNode = !!(parent.decorators?.length ||
70+
(ts.isSetAccessor(parent.parent) && !!parent.parent.decorators?.length) ||
71+
(ts.isConstructorDeclaration(parent.parent) && !!parent.parent.parent.decorators?.length));
72+
break;
73+
}
74+
if (isTypeReferenceForDecoratoredNode) {
5275
symbol = typeChecker.getSymbolAtLocation(node);
53-
break;
54-
case ts.SyntaxKind.ExportSpecifier:
55-
symbol = typeChecker.getExportSpecifierLocalTargetSymbol(node as ts.ExportSpecifier);
56-
break;
57-
case ts.SyntaxKind.ShorthandPropertyAssignment:
58-
symbol = typeChecker.getShorthandAssignmentValueSymbol(node);
59-
break;
76+
} else {
77+
// If type reference is not for Decorator skip and marked as unused.
78+
return;
79+
}
80+
} else {
81+
switch (node.kind) {
82+
case ts.SyntaxKind.Identifier:
83+
symbol = typeChecker.getSymbolAtLocation(node);
84+
break;
85+
case ts.SyntaxKind.ExportSpecifier:
86+
symbol = typeChecker.getExportSpecifierLocalTargetSymbol(node as ts.ExportSpecifier);
87+
break;
88+
case ts.SyntaxKind.ShorthandPropertyAssignment:
89+
symbol = typeChecker.getShorthandAssignmentValueSymbol(node);
90+
break;
91+
}
6092
}
6193

6294
if (symbol) {

0 commit comments

Comments
 (0)