From 179b6caf588a7e94c1183813d1afe37c142db309 Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Mon, 7 Sep 2020 20:57:49 +0200 Subject: [PATCH] fix(@ngtools/webpack): emit `require` in replace resources when using 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: https://github.com/microsoft/TypeScript/issues/18369#issuecomment-329852888 Closes #18718 --- .../src/transformers/replace_resources.ts | 74 ++++++++++++------- .../transformers/replace_resources_spec.ts | 39 +++++++++- 2 files changed, 85 insertions(+), 28 deletions(-) diff --git a/packages/ngtools/webpack/src/transformers/replace_resources.ts b/packages/ngtools/webpack/src/transformers/replace_resources.ts index f52d85cc10d6..8ae31104160e 100644 --- a/packages/ngtools/webpack/src/transformers/replace_resources.ts +++ b/packages/ngtools/webpack/src/transformers/replace_resources.ts @@ -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, ); @@ -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; @@ -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, ); @@ -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; @@ -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; } @@ -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 @@ -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)) { @@ -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; diff --git a/packages/ngtools/webpack/src/transformers/replace_resources_spec.ts b/packages/ngtools/webpack/src/transformers/replace_resources_spec.ts index 7d2bb4d4b05d..c07660351a15 100644 --- a/packages/ngtools/webpack/src/transformers/replace_resources_spec.ts +++ b/packages/ngtools/webpack/src/transformers/replace_resources_spec.ts @@ -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'; @@ -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); @@ -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';