From fe12d0dc789e98d51676297b2be7855988c8f69d Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Wed, 6 Nov 2019 17:03:56 +0000 Subject: [PATCH] fix(ngcc): render adjacent statements after static properties (#33630) See https://github.com/angular/angular/pull/33337#issuecomment-545487737 Fixes FW-1664 PR Close #33630 --- integration/ngcc/test.sh | 4 + .../src/rendering/esm5_rendering_formatter.ts | 21 +++ .../src/rendering/esm_rendering_formatter.ts | 42 ++++++ .../ngcc/src/rendering/renderer.ts | 47 +++++-- .../ngcc/src/rendering/rendering_formatter.ts | 2 + .../commonjs_rendering_formatter_spec.ts | 49 +++++++ .../ngcc/test/rendering/dts_renderer_spec.ts | 4 + .../esm5_rendering_formatter_spec.ts | 50 +++++++ .../rendering/esm_rendering_formatter_spec.ts | 47 ++++++- .../ngcc/test/rendering/renderer_spec.ts | 130 ++++++++++++------ .../rendering/umd_rendering_formatter_spec.ts | 62 ++++++++- 11 files changed, 397 insertions(+), 61 deletions(-) diff --git a/integration/ngcc/test.sh b/integration/ngcc/test.sh index da360842fa109..25c28f9f1953b 100755 --- a/integration/ngcc/test.sh +++ b/integration/ngcc/test.sh @@ -78,6 +78,10 @@ assertSucceeded "Expected 'ngcc' to log 'Compiling'." grep "ApplicationModule.ɵmod = ɵngcc0.ɵɵdefineNgModule" node_modules/@angular/core/esm5/src/application_module.js assertSucceeded "Expected 'ngcc' to correctly compile 'ApplicationModule' in '@angular/core' (esm5)." +# Did it place the `setClassMetadata` call correctly? + cat node_modules/@angular/core/fesm2015/core.js | awk 'ORS=" "' | grep "ApplicationRef.ctorParameters.*setClassMetadata(ApplicationRef" + assertSucceeded "Expected 'ngcc' to place 'setClassMetadata' after static properties like 'ctorParameters' in '@angular/core' (fesm2015)." + # Did it transform @angular/core typing files correctly? grep "import [*] as ɵngcc0 from './src/r3_symbols';" node_modules/@angular/core/core.d.ts diff --git a/packages/compiler-cli/ngcc/src/rendering/esm5_rendering_formatter.ts b/packages/compiler-cli/ngcc/src/rendering/esm5_rendering_formatter.ts index 138398ec0021f..39c50f11a838f 100644 --- a/packages/compiler-cli/ngcc/src/rendering/esm5_rendering_formatter.ts +++ b/packages/compiler-cli/ngcc/src/rendering/esm5_rendering_formatter.ts @@ -35,4 +35,25 @@ export class Esm5RenderingFormatter extends EsmRenderingFormatter { const insertionPoint = returnStatement.getFullStart(); output.appendLeft(insertionPoint, '\n' + definitions); } + + /** + * Add the adjacent statements inside the IIFE of each decorated class + */ + addAdjacentStatements(output: MagicString, compiledClass: CompiledClass, statements: string): + void { + const iifeBody = getIifeBody(compiledClass.declaration); + if (!iifeBody) { + throw new Error( + `Compiled class declaration is not inside an IIFE: ${compiledClass.name} in ${compiledClass.declaration.getSourceFile().fileName}`); + } + + const returnStatement = iifeBody.statements.find(ts.isReturnStatement); + if (!returnStatement) { + throw new Error( + `Compiled class wrapper IIFE does not have a return statement: ${compiledClass.name} in ${compiledClass.declaration.getSourceFile().fileName}`); + } + + const insertionPoint = returnStatement.getFullStart(); + output.appendLeft(insertionPoint, '\n' + statements); + } } 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 b78e5ea14136d..a5c40cd12536f 100644 --- a/packages/compiler-cli/ngcc/src/rendering/esm_rendering_formatter.ts +++ b/packages/compiler-cli/ngcc/src/rendering/esm_rendering_formatter.ts @@ -100,6 +100,30 @@ export class EsmRenderingFormatter implements RenderingFormatter { output.appendLeft(insertionPoint, '\n' + definitions); } + /** + * Add the adjacent statements after all static properties of the class. + */ + addAdjacentStatements(output: MagicString, compiledClass: CompiledClass, statements: string): + void { + const classSymbol = this.host.getClassSymbol(compiledClass.declaration); + if (!classSymbol) { + throw new Error(`Compiled class does not have a valid symbol: ${compiledClass.name}`); + } + + let insertionPoint = classSymbol.declaration.valueDeclaration.getEnd(); + + // If there are static members on this class then insert after the last one + if (classSymbol.declaration.exports !== undefined) { + classSymbol.declaration.exports.forEach(exportSymbol => { + const exportStatement = getContainingStatement(exportSymbol); + if (exportStatement !== null) { + insertionPoint = Math.max(insertionPoint, exportStatement.getEnd()); + } + }); + } + output.appendLeft(insertionPoint, '\n' + statements); + } + /** * Remove static decorator properties from classes. */ @@ -244,3 +268,21 @@ function getNextSiblingInArray(node: T, array: ts.NodeArray index + 1 ? array[index + 1] : null; } + +/** + * Find the statement that contains the given class member + * @param symbol the symbol of a static member of a class + */ +function getContainingStatement(symbol: ts.Symbol): ts.ExpressionStatement|null { + if (symbol.valueDeclaration === undefined) { + return null; + } + let node: ts.Node|null = symbol.valueDeclaration; + while (node) { + if (ts.isExpressionStatement(node)) { + break; + } + node = node.parent; + } + return node || null; +} diff --git a/packages/compiler-cli/ngcc/src/rendering/renderer.ts b/packages/compiler-cli/ngcc/src/rendering/renderer.ts index 1560399553321..24fc2f93d10ef 100644 --- a/packages/compiler-cli/ngcc/src/rendering/renderer.ts +++ b/packages/compiler-cli/ngcc/src/rendering/renderer.ts @@ -86,6 +86,10 @@ export class Renderer { this.renderDefinitions(compiledFile.sourceFile, clazz, importManager); this.srcFormatter.addDefinitions(outputText, clazz, renderedDefinition); + const renderedStatements = + this.renderAdjacentStatements(compiledFile.sourceFile, clazz, importManager); + this.srcFormatter.addAdjacentStatements(outputText, clazz, renderedStatements); + if (!isEntryPoint && clazz.reexports.length > 0) { this.srcFormatter.addDirectExports( outputText, clazz.reexports, importManager, compiledFile.sourceFile); @@ -147,26 +151,45 @@ export class Renderer { } /** - * Render the definitions as source code for the given class. - * @param sourceFile The file containing the class to process. - * @param clazz The class whose definitions are to be rendered. - * @param compilation The results of analyzing the class - this is used to generate the rendered - * definitions. - * @param imports An object that tracks the imports that are needed by the rendered definitions. - */ + * Render the definitions as source code for the given class. + * @param sourceFile The file containing the class to process. + * @param clazz The class whose definitions are to be rendered. + * @param compilation The results of analyzing the class - this is used to generate the rendered + * definitions. + * @param imports An object that tracks the imports that are needed by the rendered definitions. + */ private renderDefinitions( sourceFile: ts.SourceFile, compiledClass: CompiledClass, imports: ImportManager): string { - const printer = createPrinter(); const name = this.host.getInternalNameOfClass(compiledClass.declaration); - const translate = (stmt: Statement) => - translateStatement(stmt, imports, NOOP_DEFAULT_IMPORT_RECORDER); - const print = (stmt: Statement) => - printer.printNode(ts.EmitHint.Unspecified, translate(stmt), sourceFile); const statements: Statement[] = compiledClass.compilation.map( c => { return createAssignmentStatement(name, c.name, c.initializer); }); + return this.renderStatements(sourceFile, statements, imports); + } + + /** + * Render the adjacent statements as source code for the given class. + * @param sourceFile The file containing the class to process. + * @param clazz The class whose statements are to be rendered. + * @param compilation The results of analyzing the class - this is used to generate the rendered + * definitions. + * @param imports An object that tracks the imports that are needed by the rendered definitions. + */ + private renderAdjacentStatements( + sourceFile: ts.SourceFile, compiledClass: CompiledClass, imports: ImportManager): string { + const statements: Statement[] = []; for (const c of compiledClass.compilation) { statements.push(...c.statements); } + return this.renderStatements(sourceFile, statements, imports); + } + + private renderStatements( + sourceFile: ts.SourceFile, statements: Statement[], imports: ImportManager): string { + const printer = createPrinter(); + const translate = (stmt: Statement) => + translateStatement(stmt, imports, NOOP_DEFAULT_IMPORT_RECORDER); + const print = (stmt: Statement) => + printer.printNode(ts.EmitHint.Unspecified, translate(stmt), sourceFile); return statements.map(print).join('\n'); } } diff --git a/packages/compiler-cli/ngcc/src/rendering/rendering_formatter.ts b/packages/compiler-cli/ngcc/src/rendering/rendering_formatter.ts index 799afcd21b7d9..db3bd2993d690 100644 --- a/packages/compiler-cli/ngcc/src/rendering/rendering_formatter.ts +++ b/packages/compiler-cli/ngcc/src/rendering/rendering_formatter.ts @@ -36,6 +36,8 @@ export interface RenderingFormatter { output: MagicString, exports: Reexport[], importManager: ImportManager, file: ts.SourceFile): void; addDefinitions(output: MagicString, compiledClass: CompiledClass, definitions: string): void; + addAdjacentStatements(output: MagicString, compiledClass: CompiledClass, statements: string): + void; removeDecorators(output: MagicString, decoratorsToRemove: RedundantDecoratorMap): void; rewriteSwitchableDeclarations( outputText: MagicString, sourceFile: ts.SourceFile, 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 14ddeeefdc328..66135482b70fe 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 @@ -345,6 +345,55 @@ SOME DEFINITION TEXT }); }); + describe('addAdjacentStatements', () => { + const contents = `var core = require('@angular/core');\n` + + `var SomeDirective = /** @class **/ (function () {\n` + + ` function SomeDirective(zone, cons) {}\n` + + ` SomeDirective.prototype.method = function() {}\n` + + ` SomeDirective.decorators = [\n` + + ` { type: core.Directive, args: [{ selector: '[a]' }] },\n` + + ` { type: OtherA }\n` + + ` ];\n` + + ` SomeDirective.ctorParameters = () => [\n` + + ` { type: core.NgZone },\n` + + ` { type: core.Console }\n` + + ` ];\n` + + ` return SomeDirective;\n` + + `}());\n` + + `export {SomeDirective};`; + + it('should insert the statements after all the static methods of the class', () => { + const program = {name: _('/node_modules/test-package/some/file.js'), contents}; + const {renderer, decorationAnalyses, sourceFile} = setup(program); + const output = new MagicString(contents); + const compiledClass = decorationAnalyses.get(sourceFile) !.compiledClasses.find( + c => c.name === 'SomeDirective') !; + renderer.addAdjacentStatements(output, compiledClass, 'SOME STATEMENTS'); + expect(output.toString()) + .toContain( + ` SomeDirective.ctorParameters = () => [\n` + + ` { type: core.NgZone },\n` + + ` { type: core.Console }\n` + + ` ];\n` + + `SOME STATEMENTS\n` + + ` return SomeDirective;\n`); + }); + + it('should insert the statements after any definitions', () => { + const program = {name: _('/node_modules/test-package/some/file.js'), contents}; + const {renderer, decorationAnalyses, sourceFile} = setup(program); + const output = new MagicString(contents); + const compiledClass = decorationAnalyses.get(sourceFile) !.compiledClasses.find( + c => c.name === 'SomeDirective') !; + renderer.addDefinitions(output, compiledClass, 'SOME DEFINITIONS'); + renderer.addAdjacentStatements(output, compiledClass, 'SOME STATEMENTS'); + const definitionsPosition = output.toString().indexOf('SOME DEFINITIONS'); + const statementsPosition = output.toString().indexOf('SOME STATEMENTS'); + expect(definitionsPosition).not.toEqual(-1, 'definitions should exist'); + expect(statementsPosition).not.toEqual(-1, 'statements should exist'); + expect(statementsPosition).toBeGreaterThan(definitionsPosition); + }); + }); describe('removeDecorators', () => { diff --git a/packages/compiler-cli/ngcc/test/rendering/dts_renderer_spec.ts b/packages/compiler-cli/ngcc/test/rendering/dts_renderer_spec.ts index fb064c80181f7..e86db2cec7f15 100644 --- a/packages/compiler-cli/ngcc/test/rendering/dts_renderer_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/dts_renderer_spec.ts @@ -39,6 +39,9 @@ class TestRenderingFormatter implements RenderingFormatter { addDefinitions(output: MagicString, compiledClass: CompiledClass, definitions: string) { output.prepend('\n// ADD DEFINITIONS\n'); } + addAdjacentStatements(output: MagicString, compiledClass: CompiledClass, statements: string) { + output.prepend('\n// ADD ADJACENT STATEMENTS\n'); + } removeDecorators(output: MagicString, decoratorsToRemove: RedundantDecoratorMap) { output.prepend('\n// REMOVE DECORATORS\n'); } @@ -79,6 +82,7 @@ function createTestRenderer( spyOn(testFormatter, 'addExports').and.callThrough(); spyOn(testFormatter, 'addImports').and.callThrough(); spyOn(testFormatter, 'addDefinitions').and.callThrough(); + spyOn(testFormatter, 'addAdjacentStatements').and.callThrough(); spyOn(testFormatter, 'addConstants').and.callThrough(); spyOn(testFormatter, 'removeDecorators').and.callThrough(); spyOn(testFormatter, 'rewriteSwitchableDeclarations').and.callThrough(); 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 1b7b64ee4d39a..ee104ac61646c 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 @@ -334,6 +334,56 @@ SOME DEFINITION TEXT }); }); + describe('addAdjacentStatements', () => { + const contents = `import {Directive, NgZone, Console} from '@angular/core';\n` + + `var SomeDirective = /** @class **/ (function () {\n` + + ` function SomeDirective(zone, cons) {}\n` + + ` SomeDirective.prototype.method = function() {}\n` + + ` SomeDirective.decorators = [\n` + + ` { type: Directive, args: [{ selector: '[a]' }] },\n` + + ` { type: OtherA }\n` + + ` ];\n` + + ` SomeDirective.ctorParameters = () => [\n` + + ` { type: NgZone },\n` + + ` { type: Console }\n` + + ` ];\n` + + ` return SomeDirective;\n` + + `}());\n` + + `export {SomeDirective};`; + + it('should insert the statements after all the static methods of the class', () => { + const program = {name: _('/node_modules/test-package/some/file.js'), contents}; + const {renderer, decorationAnalyses, sourceFile} = setup(program); + const output = new MagicString(contents); + const compiledClass = decorationAnalyses.get(sourceFile) !.compiledClasses.find( + c => c.name === 'SomeDirective') !; + renderer.addAdjacentStatements(output, compiledClass, 'SOME STATEMENTS'); + expect(output.toString()) + .toContain( + ` SomeDirective.ctorParameters = () => [\n` + + ` { type: NgZone },\n` + + ` { type: Console }\n` + + ` ];\n` + + `SOME STATEMENTS\n` + + ` return SomeDirective;\n`); + }); + + it('should insert the statements after any definitions', () => { + const program = {name: _('/node_modules/test-package/some/file.js'), contents}; + const {renderer, decorationAnalyses, sourceFile} = setup(program); + const output = new MagicString(contents); + const compiledClass = decorationAnalyses.get(sourceFile) !.compiledClasses.find( + c => c.name === 'SomeDirective') !; + renderer.addDefinitions(output, compiledClass, 'SOME DEFINITIONS'); + renderer.addAdjacentStatements(output, compiledClass, 'SOME STATEMENTS'); + const definitionsPosition = output.toString().indexOf('SOME DEFINITIONS'); + const statementsPosition = output.toString().indexOf('SOME STATEMENTS'); + expect(definitionsPosition).not.toEqual(-1, 'definitions should exist'); + expect(statementsPosition).not.toEqual(-1, 'statements should exist'); + expect(statementsPosition).toBeGreaterThan(definitionsPosition); + }); + }); + describe('removeDecorators', () => { 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 7dbb778cf05f8..49e9d6f099308 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 @@ -241,9 +241,54 @@ SOME DEFINITION TEXT A.decorators = [ `); }); - }); + describe('addAdjacentStatements', () => { + const contents = `import {Directive, NgZone, Console} from '@angular/core';\n` + + `export class SomeDirective {\n` + + ` constructor(zone, cons) {}\n` + + ` method() {}\n` + + `}\n` + + `SomeDirective.decorators = [\n` + + ` { type: Directive, args: [{ selector: '[a]' }] },\n` + + ` { type: OtherA }\n` + + `];\n` + + `SomeDirective.ctorParameters = () => [\n` + + ` { type: NgZone },\n` + + ` { type: Console }\n` + + `];`; + + it('should insert the statements after all the static methods of the class', () => { + const program = {name: _('/node_modules/test-package/some/file.js'), contents}; + const {renderer, decorationAnalyses, sourceFile} = setup([program]); + const output = new MagicString(contents); + const compiledClass = decorationAnalyses.get(sourceFile) !.compiledClasses.find( + c => c.name === 'SomeDirective') !; + renderer.addAdjacentStatements(output, compiledClass, 'SOME STATEMENTS'); + expect(output.toString()) + .toContain( + `SomeDirective.ctorParameters = () => [\n` + + ` { type: NgZone },\n` + + ` { type: Console }\n` + + `];\n` + + `SOME STATEMENTS`); + }); + + it('should insert the statements after any definitions', () => { + const program = {name: _('/node_modules/test-package/some/file.js'), contents}; + const {renderer, decorationAnalyses, sourceFile} = setup([program]); + const output = new MagicString(contents); + const compiledClass = decorationAnalyses.get(sourceFile) !.compiledClasses.find( + c => c.name === 'SomeDirective') !; + renderer.addDefinitions(output, compiledClass, 'SOME DEFINITIONS'); + renderer.addAdjacentStatements(output, compiledClass, 'SOME STATEMENTS'); + const definitionsPosition = output.toString().indexOf('SOME DEFINITIONS'); + const statementsPosition = output.toString().indexOf('SOME STATEMENTS'); + expect(definitionsPosition).not.toEqual(-1, 'definitions should exist'); + expect(statementsPosition).not.toEqual(-1, 'statements should exist'); + expect(statementsPosition).toBeGreaterThan(definitionsPosition); + }); + }); describe('removeDecorators', () => { describe('[static property declaration]', () => { diff --git a/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts b/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts index 968bf13acad95..732870f04ae0f 100644 --- a/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts @@ -42,6 +42,9 @@ class TestRenderingFormatter implements RenderingFormatter { addDefinitions(output: MagicString, compiledClass: CompiledClass, definitions: string) { output.prepend('\n// ADD DEFINITIONS\n'); } + addAdjacentStatements(output: MagicString, compiledClass: CompiledClass, statements: string) { + output.prepend('\n// ADD ADJACENT STATEMENTS\n'); + } removeDecorators(output: MagicString, decoratorsToRemove: RedundantDecoratorMap) { output.prepend('\n// REMOVE DECORATORS\n'); } @@ -84,6 +87,7 @@ function createTestRenderer( spyOn(testFormatter, 'addExports').and.callThrough(); spyOn(testFormatter, 'addImports').and.callThrough(); spyOn(testFormatter, 'addDefinitions').and.callThrough(); + spyOn(testFormatter, 'addAdjacentStatements').and.callThrough(); spyOn(testFormatter, 'addConstants').and.callThrough(); spyOn(testFormatter, 'removeDecorators').and.callThrough(); spyOn(testFormatter, 'rewriteSwitchableDeclarations').and.callThrough(); @@ -149,6 +153,8 @@ runInEachFileSystem(() => { // ADD CONSTANTS +// ADD ADJACENT STATEMENTS + // ADD DEFINITIONS // REMOVE DECORATORS @@ -160,14 +166,14 @@ runInEachFileSystem(() => { 'sources': [_('/node_modules/test-package/src/file.js')], 'sourcesContent': [INPUT_PROGRAM.contents], 'names': [], - 'mappings': ';;;;;;;;;;AAAA;;;;;;;;;' + 'mappings': ';;;;;;;;;;;;AAAA;;;;;;;;;' }); MERGED_OUTPUT_PROGRAM_MAP = fromObject({ 'version': 3, 'sources': [_('/node_modules/test-package/src/file.ts')], 'names': [], - 'mappings': ';;;;;;;;;;AAAA', + 'mappings': ';;;;;;;;;;;;AAAA', 'file': 'file.js', 'sourcesContent': [INPUT_PROGRAM.contents] }); @@ -200,8 +206,11 @@ A.ɵcmp = ɵngcc0.ɵɵdefineComponent({ type: A, selectors: [["a"]], decls: 1, v ɵngcc0.ɵɵtext(0); } if (rf & 2) { ɵngcc0.ɵɵtextInterpolate(ctx.person.name); - } }, encapsulation: 2 }); -/*@__PURE__*/ ɵngcc0.ɵsetClassMetadata(A, [{ + } }, encapsulation: 2 });`); + + const addAdjacentStatementsSpy = testFormatter.addAdjacentStatements as jasmine.Spy; + expect(addAdjacentStatementsSpy.calls.first().args[2]) + .toEqual(`/*@__PURE__*/ ɵngcc0.ɵsetClassMetadata(A, [{ type: Component, args: [{ selector: 'a', template: '{{ person!.name }}' }] }], null, null);`); @@ -226,7 +235,7 @@ A.ɵcmp = ɵngcc0.ɵɵdefineComponent({ type: A, selectors: [["a"]], decls: 1, v () => { const {renderer, decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses, testFormatter} = createTestRenderer('test-package', [INPUT_PROGRAM]); - const result = renderer.renderProgram( + renderer.renderProgram( decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses); const addDefinitionsSpy = testFormatter.addDefinitions as jasmine.Spy; expect(addDefinitionsSpy.calls.first().args[0].toString()).toEqual(RENDERED_CONTENTS); @@ -234,11 +243,25 @@ A.ɵcmp = ɵngcc0.ɵɵdefineComponent({ type: A, selectors: [["a"]], decls: 1, v name: 'A', decorators: [jasmine.objectContaining({name: 'Directive'})] })); - expect(addDefinitionsSpy.calls.first().args[2]) .toEqual(`A.ɵfac = function A_Factory(t) { return new (t || A)(); }; -A.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: A, selectors: [["", "a", ""]] }); -/*@__PURE__*/ ɵngcc0.ɵsetClassMetadata(A, [{ +A.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: A, selectors: [["", "a", ""]] });`); + }); + + it('should call addAdjacentStatements with the source code, the analyzed class and the rendered statements', + () => { + const {renderer, decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses, + testFormatter} = createTestRenderer('test-package', [INPUT_PROGRAM]); + renderer.renderProgram( + decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses); + const addAdjacentStatementsSpy = testFormatter.addAdjacentStatements as jasmine.Spy; + expect(addAdjacentStatementsSpy.calls.first().args[0].toString()) + .toEqual(RENDERED_CONTENTS); + expect(addAdjacentStatementsSpy.calls.first().args[1]) + .toEqual(jasmine.objectContaining( + {name: 'A', decorators: [jasmine.objectContaining({name: 'Directive'})]})); + expect(addAdjacentStatementsSpy.calls.first().args[2]) + .toEqual(`/*@__PURE__*/ ɵngcc0.ɵsetClassMetadata(A, [{ type: Directive, args: [{ selector: '[a]' }] }], null, { foo: [] });`); @@ -268,23 +291,25 @@ A.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: A, selectors: [["", "a", ""]] }); .toEqual(`{ type: Directive, args: [{ selector: '[a]' }] }`); }); - it('should render static fields before any additional statements', () => { + it('should render definitions as static fields', () => { const {renderer, decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses, testFormatter} = createTestRenderer('test-package', [NGMODULE_PROGRAM]); renderer.renderProgram( decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses); const addDefinitionsSpy = testFormatter.addDefinitions as jasmine.Spy; const definitions: string = addDefinitionsSpy.calls.first().args[2]; - const ngModuleDef = definitions.indexOf('ɵmod'); - expect(ngModuleDef).not.toEqual(-1, 'ɵmod should exist'); - const ngInjectorDef = definitions.indexOf('ɵinj'); - expect(ngInjectorDef).not.toEqual(-1, 'ɵinj should exist'); - const setClassMetadata = definitions.indexOf('setClassMetadata'); - expect(setClassMetadata).not.toEqual(-1, 'setClassMetadata call should exist'); - expect(setClassMetadata) - .toBeGreaterThan(ngModuleDef, 'setClassMetadata should follow ɵmod'); - expect(setClassMetadata) - .toBeGreaterThan(ngInjectorDef, 'setClassMetadata should follow ɵinj'); + expect(definitions).toContain('A.ɵmod = ɵngcc0.ɵɵdefineNgModule('); + expect(definitions).toContain('A.ɵinj = ɵngcc0.ɵɵdefineInjector('); + }); + + it('should render adjacent statements', () => { + const {renderer, decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses, + testFormatter} = createTestRenderer('test-package', [NGMODULE_PROGRAM]); + renderer.renderProgram( + decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses); + const addAdjacentStatementsSpy = testFormatter.addAdjacentStatements as jasmine.Spy; + const statements: string = addAdjacentStatementsSpy.calls.first().args[2]; + expect(statements).toContain('ɵsetClassMetadata(A'); }); it('should render directives using the inner class name if different from outer', () => { @@ -308,12 +333,15 @@ A.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: A, selectors: [["", "a", ""]] }); decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses); const addDefinitionsSpy = testFormatter.addDefinitions as jasmine.Spy; - const output = addDefinitionsSpy.calls.first().args[2]; - expect(output).toContain('InnerClass.ɵfac'); - expect(output).toContain('new (t || InnerClass)'); - expect(output).toContain('InnerClass.ɵdir'); - expect(output).toContain('type: InnerClass'); - expect(output).toContain('ɵsetClassMetadata(InnerClass'); + const definitions = addDefinitionsSpy.calls.first().args[2]; + expect(definitions).toContain('InnerClass.ɵfac'); + expect(definitions).toContain('new (t || InnerClass)'); + expect(definitions).toContain('InnerClass.ɵdir'); + expect(definitions).toContain('type: InnerClass'); + + const addAdjacentStatementsSpy = testFormatter.addAdjacentStatements as jasmine.Spy; + const statements = addAdjacentStatementsSpy.calls.first().args[2]; + expect(statements).toContain('ɵsetClassMetadata(InnerClass'); }); it('should render injectables using the inner class name if different from outer', () => { @@ -337,12 +365,15 @@ A.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: A, selectors: [["", "a", ""]] }); decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses); const addDefinitionsSpy = testFormatter.addDefinitions as jasmine.Spy; - const output = addDefinitionsSpy.calls.first().args[2]; - expect(output).toContain('InnerClass.ɵfac'); - expect(output).toContain('new (t || InnerClass)()'); - expect(output).toContain('InnerClass.ɵprov'); - expect(output).toContain('token: InnerClass'); - expect(output).toContain('ɵsetClassMetadata(InnerClass'); + const definitions = addDefinitionsSpy.calls.first().args[2]; + expect(definitions).toContain('InnerClass.ɵfac'); + expect(definitions).toContain('new (t || InnerClass)()'); + expect(definitions).toContain('InnerClass.ɵprov'); + expect(definitions).toContain('token: InnerClass'); + + const addAdjacentStatementsSpy = testFormatter.addAdjacentStatements as jasmine.Spy; + const statements = addAdjacentStatementsSpy.calls.first().args[2]; + expect(statements).toContain('ɵsetClassMetadata(InnerClass'); }); it('should render ng-modules using the inner class name if different from outer', () => { @@ -371,11 +402,15 @@ A.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: A, selectors: [["", "a", ""]] }); decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses); const addDefinitionsSpy = testFormatter.addDefinitions as jasmine.Spy; - const output = addDefinitionsSpy.calls.all()[1].args[2]; - expect(output).toContain('InnerClass.ɵmod'); - expect(output).toContain('type: InnerClass'); - expect(output).toContain('ɵɵsetNgModuleScope(InnerClass'); - expect(output).toContain('ɵsetClassMetadata(InnerClass'); + const definitions = addDefinitionsSpy.calls.all()[1].args[2]; + expect(definitions).toContain('InnerClass.ɵmod'); + expect(definitions).toContain('type: InnerClass'); + + + const addAdjacentStatementsSpy = testFormatter.addAdjacentStatements as jasmine.Spy; + const statements = addAdjacentStatementsSpy.calls.all()[1].args[2]; + expect(statements).toContain('ɵɵsetNgModuleScope(InnerClass'); + expect(statements).toContain('ɵsetClassMetadata(InnerClass'); }); it('should render pipes using the inner class name if different from outer', () => { @@ -399,11 +434,14 @@ A.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: A, selectors: [["", "a", ""]] }); decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses); const addDefinitionsSpy = testFormatter.addDefinitions as jasmine.Spy; - const output = addDefinitionsSpy.calls.first().args[2]; - expect(output).toContain('InnerClass.ɵfac'); - expect(output).toContain('new (t || InnerClass)()'); - expect(output).toContain('InnerClass.ɵpipe'); - expect(output).toContain('ɵsetClassMetadata(InnerClass'); + const definitions = addDefinitionsSpy.calls.first().args[2]; + expect(definitions).toContain('InnerClass.ɵfac'); + expect(definitions).toContain('new (t || InnerClass)()'); + expect(definitions).toContain('InnerClass.ɵpipe'); + + const addAdjacentStatementsSpy = testFormatter.addAdjacentStatements as jasmine.Spy; + const statements = addAdjacentStatementsSpy.calls.first().args[2]; + expect(statements).toContain('ɵsetClassMetadata(InnerClass'); }); it('should render classes without decorators if class fields are decorated', () => { @@ -446,12 +484,14 @@ UndecoratedBase.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: UndecoratedBase, vie testFormatter} = createTestRenderer('test-package', [INPUT_PROGRAM]); const addExportsSpy = testFormatter.addExports as jasmine.Spy; const addDefinitionsSpy = testFormatter.addDefinitions as jasmine.Spy; + const addAdjacentStatementsSpy = testFormatter.addAdjacentStatements as jasmine.Spy; const addConstantsSpy = testFormatter.addConstants as jasmine.Spy; const addImportsSpy = testFormatter.addImports as jasmine.Spy; renderer.renderProgram( decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses); expect(addExportsSpy).toHaveBeenCalledBefore(addImportsSpy); expect(addDefinitionsSpy).toHaveBeenCalledBefore(addImportsSpy); + expect(addAdjacentStatementsSpy).toHaveBeenCalledBefore(addImportsSpy); expect(addConstantsSpy).toHaveBeenCalledBefore(addImportsSpy); }); }); @@ -513,8 +553,8 @@ UndecoratedBase.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: UndecoratedBase, vie testFormatter} = createTestRenderer('@angular/core', [CORE_FILE, R3_SYMBOLS_FILE]); renderer.renderProgram( decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses); - const addDefinitionsSpy = testFormatter.addDefinitions as jasmine.Spy; - expect(addDefinitionsSpy.calls.first().args[2]) + const addAdjacentStatementsSpy = testFormatter.addAdjacentStatements as jasmine.Spy; + expect(addAdjacentStatementsSpy.calls.first().args[2]) .toContain(`/*@__PURE__*/ ɵngcc0.setClassMetadata(`); const addImportsSpy = testFormatter.addImports as jasmine.Spy; expect(addImportsSpy.calls.first().args[1]).toEqual([ @@ -533,8 +573,8 @@ UndecoratedBase.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: UndecoratedBase, vie testFormatter} = createTestRenderer('@angular/core', [CORE_FILE]); renderer.renderProgram( decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses); - const addDefinitionsSpy = testFormatter.addDefinitions as jasmine.Spy; - expect(addDefinitionsSpy.calls.first().args[2]) + const addAdjacentStatementsSpy = testFormatter.addAdjacentStatements as jasmine.Spy; + expect(addAdjacentStatementsSpy.calls.first().args[2]) .toContain(`/*@__PURE__*/ setClassMetadata(`); const addImportsSpy = testFormatter.addImports as jasmine.Spy; expect(addImportsSpy.calls.first().args[1]).toEqual([]); 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 1a5158dd05ba2..0ea177b569093 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 @@ -154,7 +154,7 @@ typeof define === 'function' && define.amd ? define('file', ['exports','/tslib', ], A); return A; }()); - export { A }; + exports.A = A; var B = /** @class */ (function () { function B() { } @@ -164,7 +164,7 @@ typeof define === 'function' && define.amd ? define('file', ['exports','/tslib', ], B); return B; }()); - export { B }; + exports.B = B; var C = /** @class */ (function () { function C() { } @@ -173,7 +173,7 @@ typeof define === 'function' && define.amd ? define('file', ['exports','/tslib', ], C); return C; }()); - export { C }; + exports.C = C; var D = /** @class */ (function () { function D() { } @@ -415,6 +415,62 @@ SOME DEFINITION TEXT }); }); + describe('addAdjacentStatements', () => { + const contents = `(function (global, factory) {\n` + + ` typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports,require('tslib'),require('@angular/core')) :\n` + + ` typeof define === 'function' && define.amd ? define('file', ['exports','/tslib','@angular/core'], factory) :\n` + + ` (factory(global.file,global.tslib,global.ng.core));\n` + + ` }(this, (function (exports,tslib,core) {'use strict';\n` + + `\n` + + ` var SomeDirective = /** @class **/ (function () {\n` + + ` function SomeDirective(zone, cons) {}\n` + + ` SomeDirective.prototype.method = function() {}\n` + + ` SomeDirective.decorators = [\n` + + ` { type: core.Directive, args: [{ selector: '[a]' }] },\n` + + ` { type: OtherA }\n` + + ` ];\n` + + ` SomeDirective.ctorParameters = () => [\n` + + ` { type: core.NgZone },\n` + + ` { type: core.Console }\n` + + ` ];\n` + + ` return SomeDirective;\n` + + ` }());\n` + + ` exports.SomeDirective = SomeDirective;\n` + + `})));`; + + it('should insert the statements after all the static methods of the class', () => { + const program = {name: _('/node_modules/test-package/some/file.js'), contents}; + const {renderer, decorationAnalyses, sourceFile} = setup(program); + const output = new MagicString(contents); + const compiledClass = decorationAnalyses.get(sourceFile) !.compiledClasses.find( + c => c.name === 'SomeDirective') !; + renderer.addAdjacentStatements(output, compiledClass, 'SOME STATEMENTS'); + expect(output.toString()) + .toContain( + ` SomeDirective.ctorParameters = () => [\n` + + ` { type: core.NgZone },\n` + + ` { type: core.Console }\n` + + ` ];\n` + + `SOME STATEMENTS\n` + + ` return SomeDirective;\n`); + }); + + it('should insert the statements after any definitions', () => { + const program = {name: _('/node_modules/test-package/some/file.js'), contents}; + const {renderer, decorationAnalyses, sourceFile} = setup(program); + const output = new MagicString(contents); + const compiledClass = decorationAnalyses.get(sourceFile) !.compiledClasses.find( + c => c.name === 'SomeDirective') !; + renderer.addDefinitions(output, compiledClass, 'SOME DEFINITIONS'); + renderer.addAdjacentStatements(output, compiledClass, 'SOME STATEMENTS'); + const definitionsPosition = output.toString().indexOf('SOME DEFINITIONS'); + const statementsPosition = output.toString().indexOf('SOME STATEMENTS'); + expect(definitionsPosition).not.toEqual(-1, 'definitions should exist'); + expect(statementsPosition).not.toEqual(-1, 'statements should exist'); + expect(statementsPosition).toBeGreaterThan(definitionsPosition); + }); + }); + describe('removeDecorators', () => { it('should delete the decorator (and following comma) that was matched in the analysis',