Skip to content

Commit 54a7551

Browse files
filipesilvahansl
filipesilva
authored and
hansl
committed
fix(@ngtools/webpack): remove app decorators on AOT
Based on @clydin's great work in #8456. I had to adapt it to use the StandardTransform model we use currently but the logic is his. Should addressed the bigger AOT non-prod builds and memory consumptions in prod build. Fix #8457 Partially adresses #5618 Supersedes #8456
1 parent abcf62f commit 54a7551

File tree

6 files changed

+266
-8
lines changed

6 files changed

+266
-8
lines changed

‎packages/@ngtools/webpack/src/angular_compiler_plugin.ts

+7
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
replaceBootstrap,
2222
exportNgFactory,
2323
exportLazyModuleMap,
24+
removeDecorators,
2425
registerLocaleData,
2526
findResources,
2627
replaceResources,
@@ -641,6 +642,12 @@ export class AngularCompilerPlugin implements Tapable {
641642
if (this._JitMode) {
642643
// Replace resources in JIT.
643644
this._transformers.push(replaceResources(isAppPath));
645+
} else {
646+
// Remove unneeded angular decorators.
647+
this._transformers.push(removeDecorators(
648+
() => this._getTsProgram().getTypeChecker(),
649+
isAppPath
650+
));
644651
}
645652

646653
if (this._platform === PLATFORM.Browser) {

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

+22-6
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,11 @@ export function getLastNode(sourceFile: ts.SourceFile): ts.Node | null {
3232
}
3333

3434

35-
export function transformTypescript(
36-
content: string,
37-
transformers: ts.TransformerFactory<ts.SourceFile>[]
38-
) {
35+
// Test transform helpers.
36+
const basePath = '/project/src/';
37+
const fileName = basePath + 'test-file.ts';
3938

39+
export function createTypescriptContext(content: string) {
4040
// Set compiler options.
4141
const compilerOptions: ts.CompilerOptions = {
4242
noEmitOnError: false,
@@ -49,15 +49,31 @@ export function transformTypescript(
4949
};
5050

5151
// Create compiler host.
52-
const basePath = '/project/src/';
5352
const compilerHost = new WebpackCompilerHost(compilerOptions, basePath);
5453

5554
// Add a dummy file to host content.
56-
const fileName = basePath + 'test-file.ts';
5755
compilerHost.writeFile(fileName, content, false);
5856

5957
// Create the TypeScript program.
6058
const program = ts.createProgram([fileName], compilerOptions, compilerHost);
59+
return { compilerHost, program };
60+
}
61+
62+
export function transformTypescript(
63+
content: string | undefined,
64+
transformers: ts.TransformerFactory<ts.SourceFile>[],
65+
program?: ts.Program,
66+
compilerHost?: WebpackCompilerHost
67+
) {
68+
69+
// Use given context or create a new one.
70+
if (content !== undefined) {
71+
const typescriptContext = createTypescriptContext(content);
72+
program = typescriptContext.program;
73+
compilerHost = typescriptContext.compilerHost;
74+
} else if (!program || !compilerHost) {
75+
throw new Error('transformTypescript needs either `content` or a `program` and `compilerHost');
76+
}
6177

6278
// Emit.
6379
const { emitSkipped, diagnostics } = program.emit(

‎packages/@ngtools/webpack/src/transformers/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,4 @@ export * from './export_ngfactory';
88
export * from './export_lazy_module_map';
99
export * from './register_locale_data';
1010
export * from './replace_resources';
11+
export * from './remove_decorators';

‎packages/@ngtools/webpack/src/transformers/make_transform.ts

+24-2
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,19 @@ export function makeTransform(
6666
}
6767
};
6868

69-
// Only visit source files we have ops for.
70-
return ops.length > 0 ? ts.visitNode(sf, visitor) : sf;
69+
// Don't visit the sourcefile at all if we don't have ops for it.
70+
if (ops.length === 0) {
71+
return sf;
72+
}
73+
74+
const result = ts.visitNode(sf, visitor);
75+
76+
// If we removed any decorators, we need to clean up the decorator arrays.
77+
if (removeOps.some((op) => op.target.kind === ts.SyntaxKind.Decorator)) {
78+
cleanupDecorators(result);
79+
}
80+
81+
return result;
7182
};
7283

7384
return transformer;
@@ -104,3 +115,14 @@ function visitEachChildWorkaround(node: ts.Node, visitor: ts.Visitor,
104115

105116
return ts.visitEachChild(node, visitor, context);
106117
}
118+
119+
120+
// If TS sees an empty decorator array, it will still emit a `__decorate` call.
121+
// This seems to be a TS bug.
122+
function cleanupDecorators(node: ts.Node) {
123+
if (node.decorators && node.decorators.length == 0) {
124+
node.decorators = undefined;
125+
}
126+
127+
ts.forEachChild(node, node => cleanupDecorators(node));
128+
}

‎packages/@ngtools/webpack/src/transformers/remove_decorators.spec.ts

+112
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
import { oneLine, stripIndent } from 'common-tags';
2+
import { createTypescriptContext, transformTypescript } from './ast_helpers';
3+
import { removeDecorators } from './remove_decorators';
4+
5+
describe('@ngtools/webpack transformers', () => {
6+
describe('decorator_remover', () => {
7+
it('should remove Angular decorators', () => {
8+
const input = stripIndent`
9+
import { Component } from '@angular/core';
10+
11+
@Component({
12+
selector: 'app-root',
13+
templateUrl: './app.component.html',
14+
styleUrls: ['./app.component.css']
15+
})
16+
export class AppComponent {
17+
title = 'app';
18+
}
19+
`;
20+
const output = stripIndent`
21+
export class AppComponent {
22+
constructor() {
23+
this.title = 'app';
24+
}
25+
}
26+
`;
27+
28+
const { program, compilerHost } = createTypescriptContext(input);
29+
const transformer = removeDecorators(
30+
() => program.getTypeChecker(),
31+
() => true,
32+
);
33+
const result = transformTypescript(undefined, [transformer], program, compilerHost);
34+
35+
expect(oneLine`${result}`).toEqual(oneLine`${output}`);
36+
});
37+
38+
it('should not remove non-Angular decorators', () => {
39+
const input = stripIndent`
40+
import { Component } from 'another-lib';
41+
42+
@Component({
43+
selector: 'app-root',
44+
templateUrl: './app.component.html',
45+
styleUrls: ['./app.component.css']
46+
})
47+
export class AppComponent {
48+
title = 'app';
49+
}
50+
`;
51+
const output = `
52+
import * as tslib_1 from "tslib";
53+
import { Component } from 'another-lib';
54+
let AppComponent = class AppComponent {
55+
constructor() {
56+
this.title = 'app';
57+
}
58+
};
59+
AppComponent = tslib_1.__decorate([
60+
Component({
61+
selector: 'app-root',
62+
templateUrl: './app.component.html',
63+
styleUrls: ['./app.component.css']
64+
})
65+
], AppComponent);
66+
export { AppComponent };
67+
`;
68+
69+
const { program, compilerHost } = createTypescriptContext(input);
70+
const transformer = removeDecorators(
71+
() => program.getTypeChecker(),
72+
() => true,
73+
);
74+
const result = transformTypescript(undefined, [transformer], program, compilerHost);
75+
76+
expect(oneLine`${result}`).toEqual(oneLine`${output}`);
77+
});
78+
79+
it('should remove imports for identifiers within the decorator', () => {
80+
const input = stripIndent`
81+
import { Component } from '@angular/core';
82+
import { ChangeDetectionStrategy } from '@angular/core';
83+
84+
@Component({
85+
selector: 'app-root',
86+
changeDetection: ChangeDetectionStrategy.OnPush,
87+
templateUrl: './app.component.html',
88+
styleUrls: ['./app.component.css']
89+
})
90+
export class AppComponent {
91+
title = 'app';
92+
}
93+
`;
94+
const output = stripIndent`
95+
export class AppComponent {
96+
constructor() {
97+
this.title = 'app';
98+
}
99+
}
100+
`;
101+
102+
const { program, compilerHost } = createTypescriptContext(input);
103+
const transformer = removeDecorators(
104+
() => program.getTypeChecker(),
105+
() => true,
106+
);
107+
const result = transformTypescript(undefined, [transformer], program, compilerHost);
108+
109+
expect(oneLine`${result}`).toEqual(oneLine`${output}`);
110+
});
111+
});
112+
});

‎packages/@ngtools/webpack/src/transformers/remove_decorators.ts

+100
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
import * as ts from 'typescript';
2+
3+
import { collectDeepNodes } from './ast_helpers';
4+
import { removeImport } from './remove_import';
5+
import { StandardTransform, TransformOperation, RemoveNodeOperation } from './interfaces';
6+
import { makeTransform } from './make_transform';
7+
8+
9+
export function removeDecorators(
10+
getTypeChecker: () => ts.TypeChecker,
11+
shouldTransform: (fileName: string) => boolean
12+
): ts.TransformerFactory<ts.SourceFile> {
13+
14+
const standardTransform: StandardTransform = function (sourceFile: ts.SourceFile) {
15+
const ops: TransformOperation[] = [];
16+
17+
if (!shouldTransform(sourceFile.fileName)) {
18+
return ops;
19+
}
20+
21+
collectDeepNodes<ts.Decorator>(sourceFile, ts.SyntaxKind.Decorator)
22+
.filter((decorator) => shouldRemove(decorator, getTypeChecker()))
23+
.forEach((decorator) => {
24+
// Remove the decorator node.
25+
ops.push(new RemoveNodeOperation(sourceFile, decorator));
26+
// Also remove imports for identifiers used in the decorator.
27+
// TS doesn't automatically elide imports for things removed in transformers so we have
28+
// to do it manually.
29+
collectDeepNodes<ts.Identifier>(decorator, ts.SyntaxKind.Identifier)
30+
.forEach((identifier) => {
31+
// This is finding a lot of things that might not be imports at all, but we can't
32+
// use the type checker since previous transforms might have modified things
33+
// Worst case scenario, some imports will be left over because there was another
34+
// identifier somewhere in the file as a property access or something with the same
35+
// name as the identifer we want to remove.
36+
// TODO: figure out if there's a better way to elide imports.
37+
ops.push(...removeImport(sourceFile, [identifier]));
38+
});
39+
});
40+
41+
return ops;
42+
};
43+
44+
return makeTransform(standardTransform);
45+
}
46+
47+
function shouldRemove(decorator: ts.Decorator, typeChecker: ts.TypeChecker): boolean {
48+
const origin = getDecoratorOrigin(decorator, typeChecker);
49+
50+
return origin && origin.module === '@angular/core';
51+
}
52+
53+
// Decorator helpers.
54+
interface DecoratorOrigin {
55+
name: string;
56+
module: string;
57+
}
58+
59+
function getDecoratorOrigin(
60+
decorator: ts.Decorator,
61+
typeChecker: ts.TypeChecker
62+
): DecoratorOrigin | null {
63+
if (!ts.isCallExpression(decorator.expression)) {
64+
return null;
65+
}
66+
67+
let identifier: ts.Node;
68+
let name: string;
69+
if (ts.isPropertyAccessExpression(decorator.expression.expression)) {
70+
identifier = decorator.expression.expression.expression;
71+
name = decorator.expression.expression.name.text;
72+
} else if (ts.isIdentifier(decorator.expression.expression)) {
73+
identifier = decorator.expression.expression;
74+
} else {
75+
return null;
76+
}
77+
78+
// NOTE: resolver.getReferencedImportDeclaration would work as well but is internal
79+
const symbol = typeChecker.getSymbolAtLocation(identifier);
80+
if (symbol && symbol.declarations && symbol.declarations.length > 0) {
81+
const declaration = symbol.declarations[0];
82+
let module: string;
83+
if (ts.isImportSpecifier(declaration)) {
84+
name = (declaration.propertyName || declaration.name).text;
85+
module = (declaration.parent.parent.parent.moduleSpecifier as ts.StringLiteral).text;
86+
} else if (ts.isNamespaceImport(declaration)) {
87+
// Use the name from the decorator namespace property access
88+
module = (declaration.parent.parent.moduleSpecifier as ts.StringLiteral).text;
89+
} else if (ts.isImportClause(declaration)) {
90+
name = declaration.name.text;
91+
module = (declaration.parent.moduleSpecifier as ts.StringLiteral).text;
92+
} else {
93+
return null;
94+
}
95+
96+
return { name, module };
97+
}
98+
99+
return null;
100+
}

0 commit comments

Comments
 (0)