Skip to content

Commit e2a25c1

Browse files
clydindgp1130
authored andcommittedMay 5, 2020
fix(@ngtools/webpack): only emit import default helper when needed
Previously, the import default TypeScript helper was emitted for every file when in JIT mode. This was unused code in the majority of cases. The helper is now emitted only when needed. For this package that would be when an Angular component decorator's resource URL properties are adjusted to support JIT execution with Webpack. (cherry picked from commit 013d424)
1 parent 2571115 commit e2a25c1

File tree

2 files changed

+79
-49
lines changed

2 files changed

+79
-49
lines changed
 

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

+47-48
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,32 @@
55
* Use of this source code is governed by an MIT-style license that can be
66
* found in the LICENSE file at https://angular.io/license
77
*/
8-
import { tags } from '@angular-devkit/core';
98
import * as ts from 'typescript';
109

10+
// emit helper for `import Name from "foo"`
11+
// importName is marked as an internal property but is needed for the tslib import.
12+
const importDefaultHelper: ts.UnscopedEmitHelper & { importName?: string } = {
13+
name: 'typescript:commonjsimportdefault',
14+
importName: '__importDefault',
15+
scoped: false,
16+
text: `
17+
var __importDefault = (this && this.__importDefault) || function (mod) {
18+
return (mod && mod.__esModule) ? mod : { "default": mod };
19+
};`,
20+
};
21+
1122
export function replaceResources(
1223
shouldTransform: (fileName: string) => boolean,
1324
getTypeChecker: () => ts.TypeChecker,
1425
directTemplateLoading = false,
1526
): ts.TransformerFactory<ts.SourceFile> {
16-
1727
return (context: ts.TransformationContext) => {
1828
const typeChecker = getTypeChecker();
1929

2030
const visitNode: ts.Visitor = (node: ts.Node) => {
2131
if (ts.isClassDeclaration(node)) {
22-
const decorators = ts.visitNodes(
23-
node.decorators,
24-
(node: ts.Decorator) => visitDecorator(node, typeChecker, directTemplateLoading),
32+
const decorators = ts.visitNodes(node.decorators, (node: ts.Decorator) =>
33+
visitDecorator(context, node, typeChecker, directTemplateLoading),
2534
);
2635

2736
return ts.updateClassDeclaration(
@@ -38,22 +47,8 @@ export function replaceResources(
3847
return ts.visitEachChild(node, visitNode, context);
3948
};
4049

41-
// emit helper for `import Name from "foo"`
42-
// importName is marked as an internal property but is needed for the tslib import.
43-
const importDefaultHelper: ts.UnscopedEmitHelper & { importName?: string; } = {
44-
name: 'typescript:commonjsimportdefault',
45-
importName: '__importDefault',
46-
scoped: false,
47-
text: tags.stripIndent`
48-
var __importDefault = (this && this.__importDefault) || function (mod) {
49-
return (mod && mod.__esModule) ? mod : { "default": mod };
50-
};`,
51-
};
52-
5350
return (sourceFile: ts.SourceFile) => {
5451
if (shouldTransform(sourceFile.fileName)) {
55-
context.requestEmitHelper(importDefaultHelper);
56-
5752
return ts.visitNode(sourceFile, visitNode);
5853
}
5954

@@ -63,9 +58,11 @@ export function replaceResources(
6358
}
6459

6560
function visitDecorator(
61+
context: ts.TransformationContext,
6662
node: ts.Decorator,
6763
typeChecker: ts.TypeChecker,
68-
directTemplateLoading: boolean): ts.Decorator {
64+
directTemplateLoading: boolean,
65+
): ts.Decorator {
6966
if (!isComponentDecorator(node, typeChecker)) {
7067
return node;
7168
}
@@ -85,10 +82,8 @@ function visitDecorator(
8582
const styleReplacements: ts.Expression[] = [];
8683

8784
// visit all properties
88-
let properties = ts.visitNodes(
89-
objectExpression.properties,
90-
(node: ts.ObjectLiteralElementLike) =>
91-
visitComponentMetadata(node, styleReplacements, directTemplateLoading),
85+
let properties = ts.visitNodes(objectExpression.properties, (node: ts.ObjectLiteralElementLike) =>
86+
visitComponentMetadata(context, node, styleReplacements, directTemplateLoading),
9287
);
9388

9489
// replace properties with updated properties
@@ -103,16 +98,14 @@ function visitDecorator(
10398

10499
return ts.updateDecorator(
105100
node,
106-
ts.updateCall(
107-
decoratorFactory,
108-
decoratorFactory.expression,
109-
decoratorFactory.typeArguments,
110-
[ts.updateObjectLiteral(objectExpression, properties)],
111-
),
101+
ts.updateCall(decoratorFactory, decoratorFactory.expression, decoratorFactory.typeArguments, [
102+
ts.updateObjectLiteral(objectExpression, properties),
103+
]),
112104
);
113105
}
114106

115107
function visitComponentMetadata(
108+
context: ts.TransformationContext,
116109
node: ts.ObjectLiteralElementLike,
117110
styleReplacements: ts.Expression[],
118111
directTemplateLoading: boolean,
@@ -124,14 +117,17 @@ function visitComponentMetadata(
124117
const name = node.name.text;
125118
switch (name) {
126119
case 'moduleId':
127-
128120
return undefined;
129121

130122
case 'templateUrl':
131123
return ts.updatePropertyAssignment(
132124
node,
133125
ts.createIdentifier('template'),
134-
createRequireExpression(node.initializer, directTemplateLoading ? '!raw-loader!' : ''),
126+
createRequireExpression(
127+
context,
128+
node.initializer,
129+
directTemplateLoading ? '!raw-loader!' : '',
130+
),
135131
);
136132

137133
case 'styles':
@@ -141,16 +137,15 @@ function visitComponentMetadata(
141137
}
142138

143139
const isInlineStyles = name === 'styles';
144-
const styles = ts.visitNodes(
145-
node.initializer.elements,
146-
(node: ts.Expression) => {
147-
if (!ts.isStringLiteral(node) && !ts.isNoSubstitutionTemplateLiteral(node)) {
148-
return node;
149-
}
150-
151-
return isInlineStyles ? ts.createLiteral(node.text) : createRequireExpression(node);
152-
},
153-
);
140+
const styles = ts.visitNodes(node.initializer.elements, (node: ts.Expression) => {
141+
if (!ts.isStringLiteral(node) && !ts.isNoSubstitutionTemplateLiteral(node)) {
142+
return node;
143+
}
144+
145+
return isInlineStyles
146+
? ts.createLiteral(node.text)
147+
: createRequireExpression(context, node);
148+
});
154149

155150
// Styles should be placed first
156151
if (isInlineStyles) {
@@ -188,17 +183,21 @@ function isComponentDecorator(node: ts.Node, typeChecker: ts.TypeChecker): node
188183
return false;
189184
}
190185

191-
function createRequireExpression(node: ts.Expression, loader?: string): ts.Expression {
186+
function createRequireExpression(
187+
context: ts.TransformationContext,
188+
node: ts.Expression,
189+
loader?: string,
190+
): ts.Expression {
192191
const url = getResourceUrl(node, loader);
193192
if (!url) {
194193
return node;
195194
}
196195

197-
const callExpression = ts.createCall(
198-
ts.createIdentifier('require'),
199-
undefined,
200-
[ts.createLiteral(url)],
201-
);
196+
context.requestEmitHelper(importDefaultHelper);
197+
198+
const callExpression = ts.createCall(ts.createIdentifier('require'), undefined, [
199+
ts.createLiteral(url),
200+
]);
202201

203202
return ts.createPropertyAccess(
204203
ts.createCall(

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

+32-1
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ describe('@ngtools/webpack transformers', () => {
391391
`;
392392

393393
const output = tags.stripIndent`
394-
import { __decorate, __importDefault } from "tslib";
394+
import { __decorate } from "tslib";
395395
import { Component } from 'foo';
396396
397397
let AppComponent = class AppComponent {
@@ -453,5 +453,36 @@ describe('@ngtools/webpack transformers', () => {
453453
const result = transform(input, false);
454454
expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
455455
});
456+
457+
it('should not emit import default helper if no changes are made', () => {
458+
const input = tags.stripIndent`
459+
import { Component } from '@angular/core';
460+
461+
@Component({
462+
selector: 'app-root'
463+
})
464+
export class AppComponent {
465+
title = 'app';
466+
}
467+
`;
468+
const output = tags.stripIndent`
469+
import { __decorate } from "tslib";
470+
import { Component } from '@angular/core';
471+
let AppComponent = class AppComponent {
472+
constructor() {
473+
this.title = 'app';
474+
}
475+
};
476+
AppComponent = __decorate([
477+
Component({
478+
selector: 'app-root'
479+
})
480+
], AppComponent);
481+
export { AppComponent };
482+
`;
483+
484+
const result = transform(input);
485+
expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
486+
});
456487
});
457488
});

0 commit comments

Comments
 (0)
Please sign in to comment.