Skip to content

Commit

Permalink
fix(@ngtools/webpack): emit require in replace resources when using…
Browse files Browse the repository at this point in the history
… CommonJS as module

When using CommonJs as module format TypeScript will generate unreferenced `require` when using `ts.createImportDeclaration`.

```js
const external_component_html_1 = require("!raw-loader!./external.component.html");
const core_1 = require("@angular/core");
let ExampleComponent = class ExampleComponent {
};
ExampleComponent = __decorate([
    core_1.Component({
        selector: 'example-compoent',
        template: __NG_CLI_RESOURCE__0,
    })
], ExampleComponent);
```

More context: microsoft/TypeScript#18369 (comment)

Closes #18718

(cherry picked from commit 179b6ca)
  • Loading branch information
alan-agius4 committed Sep 8, 2020
1 parent 430c099 commit 75ca7f9
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 28 deletions.
74 changes: 47 additions & 27 deletions packages/ngtools/webpack/src/transformers/replace_resources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ export function replaceResources(
return (context: ts.TransformationContext) => {
const typeChecker = getTypeChecker();
const resourceImportDeclarations: ts.ImportDeclaration[] = [];

const moduleKind = context.getCompilerOptions().module;
const visitNode: ts.Visitor = (node: ts.Node) => {
if (ts.isClassDeclaration(node)) {
const decorators = ts.visitNodes(node.decorators, node =>
ts.isDecorator(node)
? visitDecorator(node, typeChecker, directTemplateLoading, resourceImportDeclarations)
? visitDecorator(node, typeChecker, directTemplateLoading, resourceImportDeclarations, moduleKind)
: node,
);

Expand Down Expand Up @@ -68,6 +68,7 @@ function visitDecorator(
typeChecker: ts.TypeChecker,
directTemplateLoading: boolean,
resourceImportDeclarations: ts.ImportDeclaration[],
moduleKind?: ts.ModuleKind,
): ts.Decorator {
if (!isComponentDecorator(node, typeChecker)) {
return node;
Expand All @@ -90,7 +91,7 @@ function visitDecorator(
// visit all properties
let properties = ts.visitNodes(objectExpression.properties, node =>
ts.isObjectLiteralElementLike(node)
? visitComponentMetadata(node, styleReplacements, directTemplateLoading, resourceImportDeclarations)
? visitComponentMetadata(node, styleReplacements, directTemplateLoading, resourceImportDeclarations, moduleKind)
: node,
);

Expand All @@ -117,6 +118,7 @@ function visitComponentMetadata(
styleReplacements: ts.Expression[],
directTemplateLoading: boolean,
resourceImportDeclarations: ts.ImportDeclaration[],
moduleKind?: ts.ModuleKind,
): ts.ObjectLiteralElementLike | undefined {
if (!ts.isPropertyAssignment(node) || ts.isComputedPropertyName(node.name)) {
return node;
Expand All @@ -128,7 +130,12 @@ function visitComponentMetadata(
return undefined;

case 'templateUrl':
const importName = createResourceImport(node.initializer, directTemplateLoading ? '!raw-loader!' : '', resourceImportDeclarations);
const importName = createResourceImport(
node.initializer,
directTemplateLoading ? '!raw-loader!' : '',
resourceImportDeclarations,
moduleKind,
);
if (!importName) {
return node;
}
Expand All @@ -154,7 +161,7 @@ function visitComponentMetadata(
return ts.createLiteral(node.text);
}

return createResourceImport(node, undefined, resourceImportDeclarations) || node;
return createResourceImport(node, undefined, resourceImportDeclarations, moduleKind) || node;
});

// Styles should be placed first
Expand All @@ -170,28 +177,6 @@ function visitComponentMetadata(
}
}

export function createResourceImport(
node: ts.Node,
loader: string | undefined,
resourceImportDeclarations: ts.ImportDeclaration[],
): ts.Identifier | null {
const url = getResourceUrl(node, loader);
if (!url) {
return null;
}

const importName = ts.createIdentifier(`__NG_CLI_RESOURCE__${resourceImportDeclarations.length}`);

resourceImportDeclarations.push(ts.createImportDeclaration(
undefined,
undefined,
ts.createImportClause(importName, undefined),
ts.createLiteral(url),
));

return importName;
}

export function getResourceUrl(node: ts.Node, loader = ''): string | null {
// only analyze strings
if (!ts.isStringLiteral(node) && !ts.isNoSubstitutionTemplateLiteral(node)) {
Expand All @@ -214,6 +199,41 @@ function isComponentDecorator(node: ts.Node, typeChecker: ts.TypeChecker): node
return false;
}

function createResourceImport(
node: ts.Node,
loader: string | undefined,
resourceImportDeclarations: ts.ImportDeclaration[],
moduleKind = ts.ModuleKind.ES2015,
): ts.Identifier | ts.Expression | null {
const url = getResourceUrl(node, loader);
if (!url) {
return null;
}

const urlLiteral = ts.createLiteral(url);

if (moduleKind < ts.ModuleKind.ES2015) {
return ts.createPropertyAccess(
ts.createCall(
ts.createIdentifier('require'),
[],
[urlLiteral],
),
'default',
);
} else {
const importName = ts.createIdentifier(`__NG_CLI_RESOURCE__${resourceImportDeclarations.length}`);
resourceImportDeclarations.push(ts.createImportDeclaration(
undefined,
undefined,
ts.createImportClause(importName, undefined),
urlLiteral,
));

return importName;
}
}

interface DecoratorOrigin {
name: string;
module: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import { tags } from '@angular-devkit/core'; // tslint:disable-line:no-implicit-dependencies
import * as ts from 'typescript';
import { replaceResources } from './replace_resources';
import { createTypescriptContext, transformTypescript } from './spec_helpers';

Expand All @@ -14,8 +15,9 @@ function transform(
shouldTransform = true,
directTemplateLoading = true,
importHelpers = true,
module: ts.ModuleKind = ts.ModuleKind.ESNext,
) {
const { program, compilerHost } = createTypescriptContext(input, undefined, undefined, { importHelpers });
const { program, compilerHost } = createTypescriptContext(input, undefined, undefined, { importHelpers, module });
const getTypeChecker = () => program.getTypeChecker();
const transformer = replaceResources(
() => shouldTransform, getTypeChecker, directTemplateLoading);
Expand Down Expand Up @@ -67,6 +69,41 @@ describe('@ngtools/webpack transformers', () => {
expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
});

it('should replace resources with `require()` when module is CommonJs', () => {
const input = tags.stripIndent`
import { Component } from '@angular/core';
@Component({
selector: 'app-root',
templateUrl: './app.component.html',
styleUrls: ['./app.component.css', './app.component.2.css']
})
export class AppComponent {
title = 'app';
}
`;
const output = tags.stripIndent`
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.AppComponent = void 0;
const tslib_1 = require("tslib");
const core_1 = require("@angular/core");
let AppComponent = class AppComponent {
constructor() { this.title = 'app'; }
};
AppComponent = tslib_1.__decorate([
core_1.Component({
selector: 'app-root',
template: require("!raw-loader!./app.component.html").default,
styles: [require("./app.component.css").default, require("./app.component.2.css").default] }) ], AppComponent);
exports.AppComponent = AppComponent;
`;

const result = transform(input, true, true, true, ts.ModuleKind.CommonJS);
expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
});

it('should not replace resources when directTemplateLoading is false', () => {
const input = tags.stripIndent`
import { Component } from '@angular/core';
Expand Down

0 comments on commit 75ca7f9

Please sign in to comment.