diff --git a/integration/ngcc/test.sh b/integration/ngcc/test.sh index b545def610..e17972a122 100755 --- a/integration/ngcc/test.sh +++ b/integration/ngcc/test.sh @@ -125,6 +125,21 @@ assertSucceeded "Expected 'ngcc' to log 'Compiling'." assertSucceeded "Expected '@angular/cdk/a11y' (umd) to actually have decorators via static properties." +# Did it transform imports in UMD correctly? +# (E.g. no trailing commas, so that it remains compatible with legacy browsers, such as IE11.) + grep "factory(exports, require('rxjs'), require('rxjs/operators'))" node_modules/@angular/core/bundles/core.umd.js + assertSucceeded "Expected 'ngcc' to not add trailing commas to CommonJS block in UMD." + + grep "define('@angular/core', \['exports', 'rxjs', 'rxjs/operators'], factory)" node_modules/@angular/core/bundles/core.umd.js + assertSucceeded "Expected 'ngcc' to not add trailing commas to AMD block in UMD." + + grep "factory((global.ng = global.ng || {}, global.ng.core = {}), global.rxjs, global.rxjs.operators)" node_modules/@angular/core/bundles/core.umd.js + assertSucceeded "Expected 'ngcc' to not add trailing commas to globals block in UMD." + + grep "(this, (function (exports, rxjs, operators) {" node_modules/@angular/core/bundles/core.umd.js + assertSucceeded "Expected 'ngcc' to not add trailing commas to factory function parameters in UMD." + + # Can it be safely run again (as a noop)? # And check that it logged skipping compilation as expected ngcc -l debug | grep 'Skipping' diff --git a/packages/compiler-cli/ngcc/src/rendering/commonjs_rendering_formatter.ts b/packages/compiler-cli/ngcc/src/rendering/commonjs_rendering_formatter.ts index 65200cb991..30938fc5ac 100644 --- a/packages/compiler-cli/ngcc/src/rendering/commonjs_rendering_formatter.ts +++ b/packages/compiler-cli/ngcc/src/rendering/commonjs_rendering_formatter.ts @@ -30,6 +30,11 @@ export class CommonJsRenderingFormatter extends Esm5RenderingFormatter { * Add the imports below any in situ imports as `require` calls. */ addImports(output: MagicString, imports: Import[], file: ts.SourceFile): void { + // Avoid unnecessary work if there are no imports to add. + if (imports.length === 0) { + return; + } + const insertionPoint = this.findEndOfImports(file); const renderedImports = imports.map(i => `var ${i.qualifier} = require('${i.specifier}');\n`).join(''); diff --git a/packages/compiler-cli/ngcc/src/rendering/esm_rendering_formatter.ts b/packages/compiler-cli/ngcc/src/rendering/esm_rendering_formatter.ts index 54e46f63fb..98a35134e2 100644 --- a/packages/compiler-cli/ngcc/src/rendering/esm_rendering_formatter.ts +++ b/packages/compiler-cli/ngcc/src/rendering/esm_rendering_formatter.ts @@ -32,6 +32,10 @@ export class EsmRenderingFormatter implements RenderingFormatter { * Add the imports at the top of the file, after any imports that are already there. */ addImports(output: MagicString, imports: Import[], sf: ts.SourceFile): void { + if (imports.length === 0) { + return; + } + const insertionPoint = this.findEndOfImports(sf); const renderedImports = imports.map(i => `import * as ${i.qualifier} from '${i.specifier}';\n`).join(''); @@ -294,4 +298,4 @@ function getEndExceptSemicolon(statement: ts.Statement): number { const lastToken = statement.getLastToken(); return (lastToken && lastToken.kind === ts.SyntaxKind.SemicolonToken) ? statement.getEnd() - 1 : statement.getEnd(); -} \ No newline at end of file +} diff --git a/packages/compiler-cli/ngcc/src/rendering/umd_rendering_formatter.ts b/packages/compiler-cli/ngcc/src/rendering/umd_rendering_formatter.ts index 0b3dc215c8..2319171bce 100644 --- a/packages/compiler-cli/ngcc/src/rendering/umd_rendering_formatter.ts +++ b/packages/compiler-cli/ngcc/src/rendering/umd_rendering_formatter.ts @@ -30,6 +30,10 @@ export class UmdRenderingFormatter extends Esm5RenderingFormatter { * Add the imports to the UMD module IIFE. */ addImports(output: MagicString, imports: Import[], file: ts.SourceFile): void { + if (imports.length === 0) { + return; + } + // Assume there is only one UMD module in the file const umdModule = this.umdHost.getUmdModule(file); if (!umdModule) { diff --git a/packages/compiler-cli/ngcc/test/rendering/commonjs_rendering_formatter_spec.ts b/packages/compiler-cli/ngcc/test/rendering/commonjs_rendering_formatter_spec.ts index 30a1390a6c..ad96ba569b 100644 --- a/packages/compiler-cli/ngcc/test/rendering/commonjs_rendering_formatter_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/commonjs_rendering_formatter_spec.ts @@ -187,6 +187,17 @@ var core = require('@angular/core'); var i0 = require('@angular/core'); var i1 = require('@angular/common');`); }); + + it('should leave the file unchanged if there are no imports to add', () => { + const {renderer, sourceFile} = setup(PROGRAM); + const output = new MagicString(PROGRAM.contents); + const contentsBefore = output.toString(); + + renderer.addImports(output, [], sourceFile); + const contentsAfter = output.toString(); + + expect(contentsAfter).toBe(contentsBefore); + }); }); describe('addExports', () => { diff --git a/packages/compiler-cli/ngcc/test/rendering/esm5_rendering_formatter_spec.ts b/packages/compiler-cli/ngcc/test/rendering/esm5_rendering_formatter_spec.ts index 79701f3378..d98f4d73cc 100644 --- a/packages/compiler-cli/ngcc/test/rendering/esm5_rendering_formatter_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/esm5_rendering_formatter_spec.ts @@ -195,6 +195,17 @@ import {Directive} from '@angular/core'; import * as i0 from '@angular/core'; import * as i1 from '@angular/common';`); }); + + it('should leave the file unchanged if there are no imports to add', () => { + const {renderer, sourceFile} = setup(PROGRAM); + const output = new MagicString(PROGRAM.contents); + const contentsBefore = output.toString(); + + renderer.addImports(output, [], sourceFile); + const contentsAfter = output.toString(); + + expect(contentsAfter).toBe(contentsBefore); + }); }); describe('addExports', () => { diff --git a/packages/compiler-cli/ngcc/test/rendering/esm_rendering_formatter_spec.ts b/packages/compiler-cli/ngcc/test/rendering/esm_rendering_formatter_spec.ts index 2336cd742e..85d43e230f 100644 --- a/packages/compiler-cli/ngcc/test/rendering/esm_rendering_formatter_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/esm_rendering_formatter_spec.ts @@ -112,6 +112,17 @@ import {Directive} from '@angular/core'; import * as i0 from '@angular/core'; import * as i1 from '@angular/common';`); }); + + it('should leave the file unchanged if there are no imports to add', () => { + const {renderer, sourceFile} = setup([PROGRAM]); + const output = new MagicString(PROGRAM.contents); + const contentsBefore = output.toString(); + + renderer.addImports(output, [], sourceFile); + const contentsAfter = output.toString(); + + expect(contentsAfter).toBe(contentsBefore); + }); }); describe('addExports', () => { diff --git a/packages/compiler-cli/ngcc/test/rendering/umd_rendering_formatter_spec.ts b/packages/compiler-cli/ngcc/test/rendering/umd_rendering_formatter_spec.ts index 0a0a8bc665..07f3e11420 100644 --- a/packages/compiler-cli/ngcc/test/rendering/umd_rendering_formatter_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/umd_rendering_formatter_spec.ts @@ -325,6 +325,18 @@ typeof define === 'function' && define.amd ? define('file', ['exports','/tslib', expect(outputSrc).toContain(`(factory(global.ng.core,global.ng.common));`); expect(outputSrc).toContain(`(function (i0,i1) {'use strict';`); }); + + it('should leave the file unchanged if there are no imports to add', () => { + const {renderer, program} = setup(PROGRAM); + const file = getSourceFileOrError(program, _('/node_modules/test-package/some/file.js')); + const output = new MagicString(PROGRAM.contents); + const contentsBefore = output.toString(); + + renderer.addImports(output, [], file); + const contentsAfter = output.toString(); + + expect(contentsAfter).toBe(contentsBefore); + }); }); describe('addExports', () => {