diff --git a/packages/compiler-cli/ngcc/src/rendering/esm_renderer.ts b/packages/compiler-cli/ngcc/src/rendering/esm_renderer.ts index 8de2a47f46..8e6b4f2f10 100644 --- a/packages/compiler-cli/ngcc/src/rendering/esm_renderer.ts +++ b/packages/compiler-cli/ngcc/src/rendering/esm_renderer.ts @@ -26,10 +26,13 @@ export class EsmRenderer extends Renderer { /** * Add the imports at the top of the file */ - addImports(output: MagicString, imports: {specifier: string; qualifier: string;}[]): void { - // The imports get inserted at the very top of the file. - imports.forEach( - i => { output.appendLeft(0, `import * as ${i.qualifier} from '${i.specifier}';\n`); }); + addImports( + output: MagicString, imports: {specifier: string; qualifier: string;}[], + sf: ts.SourceFile): void { + const insertionPoint = findEndOfImports(sf); + const renderedImports = + imports.map(i => `import * as ${i.qualifier} from '${i.specifier}';\n`).join(''); + output.appendLeft(insertionPoint, renderedImports); } addExports(output: MagicString, entryPointBasePath: string, exports: ExportInfo[]): void { @@ -55,14 +58,11 @@ export class EsmRenderer extends Renderer { if (constants === '') { return; } - const insertionPoint = file.statements.reduce((prev, stmt) => { - if (ts.isImportDeclaration(stmt) || ts.isImportEqualsDeclaration(stmt) || - ts.isNamespaceImport(stmt)) { - return stmt.getEnd(); - } - return prev; - }, 0); - output.appendLeft(insertionPoint, '\n' + constants + '\n'); + const insertionPoint = findEndOfImports(file); + + // Append the constants to the right of the insertion point, to ensure they get ordered after + // added imports (those are appended left to the insertion point). + output.appendRight(insertionPoint, '\n' + constants + '\n'); } /** @@ -115,6 +115,17 @@ export class EsmRenderer extends Renderer { } } +function findEndOfImports(sf: ts.SourceFile): number { + for (const stmt of sf.statements) { + if (!ts.isImportDeclaration(stmt) && !ts.isImportEqualsDeclaration(stmt) && + !ts.isNamespaceImport(stmt)) { + return stmt.getStart(); + } + } + + return 0; +} + function findStatement(node: ts.Node) { while (node) { if (ts.isExpressionStatement(node)) { diff --git a/packages/compiler-cli/ngcc/src/rendering/renderer.ts b/packages/compiler-cli/ngcc/src/rendering/renderer.ts index 172a2abe55..39d0a9caa1 100644 --- a/packages/compiler-cli/ngcc/src/rendering/renderer.ts +++ b/packages/compiler-cli/ngcc/src/rendering/renderer.ts @@ -155,7 +155,9 @@ export abstract class Renderer { renderConstantPool(compiledFile.sourceFile, compiledFile.constantPool, importManager), compiledFile.sourceFile); - this.addImports(outputText, importManager.getAllImports(compiledFile.sourceFile.fileName)); + this.addImports( + outputText, importManager.getAllImports(compiledFile.sourceFile.fileName), + compiledFile.sourceFile); } // Add exports to the entry-point file @@ -185,7 +187,7 @@ export abstract class Renderer { }); this.addModuleWithProvidersParams(outputText, renderInfo.moduleWithProviders, importManager); - this.addImports(outputText, importManager.getAllImports(dtsFile.fileName)); + this.addImports(outputText, importManager.getAllImports(dtsFile.fileName), dtsFile); this.addExports(outputText, dtsFile.fileName, renderInfo.privateExports); @@ -246,8 +248,9 @@ export abstract class Renderer { protected abstract addConstants(output: MagicString, constants: string, file: ts.SourceFile): void; - protected abstract addImports(output: MagicString, imports: {specifier: string, - qualifier: string}[]): void; + protected abstract addImports( + output: MagicString, imports: {specifier: string, qualifier: string}[], + sf: ts.SourceFile): void; protected abstract addExports( output: MagicString, entryPointBasePath: string, exports: ExportInfo[]): void; protected abstract addDefinitions( diff --git a/packages/compiler-cli/ngcc/test/rendering/esm2015_renderer_spec.ts b/packages/compiler-cli/ngcc/test/rendering/esm2015_renderer_spec.ts index 894d0c6540..d36d5838f6 100644 --- a/packages/compiler-cli/ngcc/test/rendering/esm2015_renderer_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/esm2015_renderer_spec.ts @@ -42,6 +42,7 @@ const PROGRAM = { name: '/some/file.js', contents: ` /* A copyright notice */ +import 'some-side-effect'; import {Directive} from '@angular/core'; export class A {} A.decorators = [ @@ -114,17 +115,21 @@ export { D }; describe('Esm2015Renderer', () => { describe('addImports', () => { - it('should insert the given imports at the start of the source file', () => { - const {renderer} = setup(PROGRAM); + it('should insert the given imports after existing imports of the source file', () => { + const {renderer, sourceFile} = setup(PROGRAM); const output = new MagicString(PROGRAM.contents); - renderer.addImports(output, [ - {specifier: '@angular/core', qualifier: 'i0'}, - {specifier: '@angular/common', qualifier: 'i1'} - ]); - expect(output.toString()).toContain(`import * as i0 from '@angular/core'; -import * as i1 from '@angular/common'; - -/* A copyright notice */`); + renderer.addImports( + output, + [ + {specifier: '@angular/core', qualifier: 'i0'}, + {specifier: '@angular/common', qualifier: 'i1'} + ], + sourceFile); + expect(output.toString()).toContain(`/* A copyright notice */ +import 'some-side-effect'; +import {Directive} from '@angular/core'; +import * as i0 from '@angular/core'; +import * as i1 from '@angular/common';`); }); }); @@ -173,10 +178,27 @@ export {TopLevelComponent};`); renderer.addConstants(output, 'const x = 3;', file); expect(output.toString()).toContain(` import {Directive} from '@angular/core'; -const x = 3; +const x = 3; export class A {}`); }); + + it('should insert constants after inserted imports', () => { + const {renderer, program} = setup(PROGRAM); + const file = program.getSourceFile('some/file.js'); + if (file === undefined) { + throw new Error(`Could not find source file`); + } + const output = new MagicString(PROGRAM.contents); + renderer.addConstants(output, 'const x = 3;', file); + renderer.addImports(output, [{specifier: '@angular/core', qualifier: 'i0'}], file); + expect(output.toString()).toContain(` +import {Directive} from '@angular/core'; +import * as i0 from '@angular/core'; + +const x = 3; +export class A {`); + }); }); describe('rewriteSwitchableDeclarations', () => { diff --git a/packages/compiler-cli/ngcc/test/rendering/esm5_renderer_spec.ts b/packages/compiler-cli/ngcc/test/rendering/esm5_renderer_spec.ts index 03d987862f..a7d7c17614 100644 --- a/packages/compiler-cli/ngcc/test/rendering/esm5_renderer_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/esm5_renderer_spec.ts @@ -42,6 +42,7 @@ const PROGRAM = { name: '/some/file.js', contents: ` /* A copyright notice */ +import 'some-side-effect'; import {Directive} from '@angular/core'; var A = (function() { function A() {} @@ -151,17 +152,21 @@ export { D }; describe('Esm5Renderer', () => { describe('addImports', () => { - it('should insert the given imports at the start of the source file', () => { - const {renderer} = setup(PROGRAM); + it('should insert the given imports after existing imports of the source file', () => { + const {renderer, sourceFile} = setup(PROGRAM); const output = new MagicString(PROGRAM.contents); - renderer.addImports(output, [ - {specifier: '@angular/core', qualifier: 'i0'}, - {specifier: '@angular/common', qualifier: 'i1'} - ]); - expect(output.toString()).toContain(`import * as i0 from '@angular/core'; -import * as i1 from '@angular/common'; - -/* A copyright notice */`); + renderer.addImports( + output, + [ + {specifier: '@angular/core', qualifier: 'i0'}, + {specifier: '@angular/common', qualifier: 'i1'} + ], + sourceFile); + expect(output.toString()).toContain(`/* A copyright notice */ +import 'some-side-effect'; +import {Directive} from '@angular/core'; +import * as i0 from '@angular/core'; +import * as i1 from '@angular/common';`); }); }); @@ -210,8 +215,25 @@ export {TopLevelComponent};`); renderer.addConstants(output, 'const x = 3;', file); expect(output.toString()).toContain(` import {Directive} from '@angular/core'; -const x = 3; +const x = 3; +var A = (function() {`); + }); + + it('should insert constants after inserted imports', () => { + const {renderer, program} = setup(PROGRAM); + const file = program.getSourceFile('some/file.js'); + if (file === undefined) { + throw new Error(`Could not find source file`); + } + const output = new MagicString(PROGRAM.contents); + renderer.addConstants(output, 'const x = 3;', file); + renderer.addImports(output, [{specifier: '@angular/core', qualifier: 'i0'}], file); + expect(output.toString()).toContain(` +import {Directive} from '@angular/core'; +import * as i0 from '@angular/core'; + +const x = 3; var A = (function() {`); }); }); diff --git a/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts b/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts index 177b342c86..cea122f6af 100644 --- a/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts @@ -26,7 +26,8 @@ class TestRenderer extends Renderer { logger: Logger, host: Esm2015ReflectionHost, isCore: boolean, bundle: EntryPointBundle) { super(logger, host, isCore, bundle, '/src'); } - addImports(output: MagicString, imports: {specifier: string, qualifier: string}[]) { + addImports( + output: MagicString, imports: {specifier: string, qualifier: string}[], sf: ts.SourceFile) { output.prepend('\n// ADD IMPORTS\n'); } addExports(output: MagicString, baseEntryPointPath: string, exports: { @@ -510,11 +511,15 @@ describe('Renderer', () => { export declare function withProviders7(): ({ngModule: SomeModule, providers: any[]})&{ngModule:SomeModule}; export declare function withProviders8(): (MyModuleWithProviders)&{ngModule:SomeModule};`); - expect(renderer.addImports).toHaveBeenCalledWith(jasmine.any(MagicString), [ - {specifier: './module', qualifier: 'ɵngcc0'}, - {specifier: '@angular/core', qualifier: 'ɵngcc1'}, - {specifier: 'some-library', qualifier: 'ɵngcc2'}, - ]); + expect(renderer.addImports) + .toHaveBeenCalledWith( + jasmine.any(MagicString), + [ + {specifier: './module', qualifier: 'ɵngcc0'}, + {specifier: '@angular/core', qualifier: 'ɵngcc1'}, + {specifier: 'some-library', qualifier: 'ɵngcc2'}, + ], + jasmine.anything()); // The following expectation checks that we do not mistake `ModuleWithProviders` types