From f2d47c96c45edcb87f9fd53f06c4250e4d0479cd Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Tue, 30 Jul 2019 15:57:46 -0700 Subject: [PATCH] fix(ivy): ngcc emits static fields before extra statements (#31933) This commit changes the emit order of ngcc when a class has multiple static fields being assigned. Previously, ngcc would emit each static field followed immediately by any extra statements specified for that field. This causes issues with downstream tooling such as build optimizer, which expects all of the static fields for a class to be grouped together. ngtsc already groups static fields and additional statements. This commit changes ngcc's ordering to match. PR Close #31933 --- aio/scripts/_payload-limits.json | 2 +- .../ngcc/src/rendering/renderer.ts | 14 +++++----- .../ngcc/test/rendering/renderer_spec.ts | 26 +++++++++++++++++++ 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/aio/scripts/_payload-limits.json b/aio/scripts/_payload-limits.json index 6506e6ad47..7651cb9185 100755 --- a/aio/scripts/_payload-limits.json +++ b/aio/scripts/_payload-limits.json @@ -29,7 +29,7 @@ "runtime-es5": 2932, "runtime-es2015": 2938, "main-es5": 560811, - "main-es2015": 572938, + "main-es2015": 499846, "polyfills-es5": 129161, "polyfills-es2015": 53295 } diff --git a/packages/compiler-cli/ngcc/src/rendering/renderer.ts b/packages/compiler-cli/ngcc/src/rendering/renderer.ts index 5052a153eb..5ed3071e72 100644 --- a/packages/compiler-cli/ngcc/src/rendering/renderer.ts +++ b/packages/compiler-cli/ngcc/src/rendering/renderer.ts @@ -165,14 +165,12 @@ export function renderDefinitions( translateStatement(stmt, imports, NOOP_DEFAULT_IMPORT_RECORDER); const print = (stmt: Statement) => printer.printNode(ts.EmitHint.Unspecified, translate(stmt), sourceFile); - const definitions = compiledClass.compilation - .map( - c => [createAssignmentStatement(name, c.name, c.initializer)] - .concat(c.statements) - .map(print) - .join('\n')) - .join('\n'); - return definitions; + const statements: Statement[] = + compiledClass.compilation.map(c => createAssignmentStatement(name, c.name, c.initializer)); + for (const c of compiledClass.compilation) { + statements.push(...c.statements); + } + return statements.map(print).join('\n'); } /** diff --git a/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts b/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts index 7fb2bef33c..702053b8cc 100644 --- a/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts @@ -98,6 +98,7 @@ runInEachFileSystem(() => { let _: typeof absoluteFrom; let INPUT_PROGRAM: TestFile; let COMPONENT_PROGRAM: TestFile; + let NGMODULE_PROGRAM: TestFile; let INPUT_PROGRAM_MAP: SourceMapConverter; let RENDERED_CONTENTS: string; let OUTPUT_PROGRAM_MAP: SourceMapConverter; @@ -118,6 +119,12 @@ runInEachFileSystem(() => { `import { Component } from '@angular/core';\nexport class A {}\nA.decorators = [\n { type: Component, args: [{ selector: 'a', template: '{{ person!.name }}' }] }\n];\n` }; + NGMODULE_PROGRAM = { + name: _('/node_modules/test-package/src/ngmodule.js'), + contents: + `import { NgModule } from '@angular/core';\nexport class A {}\nA.decorators = [\n { type: NgModule, args: [{}] }\n];\n` + }; + INPUT_PROGRAM_MAP = fromObject({ 'version': 3, 'file': _('/node_modules/test-package/src/file.js'), @@ -254,6 +261,25 @@ runInEachFileSystem(() => { .toEqual(`{ type: Directive, args: [{ selector: '[a]' }] }`); }); + it('should render static fields before any additional statements', () => { + 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('ngModuleDef'); + expect(ngModuleDef).not.toEqual(-1, 'ngModuleDef should exist'); + const ngInjectorDef = definitions.indexOf('ngInjectorDef'); + expect(ngInjectorDef).not.toEqual(-1, 'ngInjectorDef should exist'); + const setClassMetadata = definitions.indexOf('setClassMetadata'); + expect(setClassMetadata).not.toEqual(-1, 'setClassMetadata call should exist'); + expect(setClassMetadata) + .toBeGreaterThan(ngModuleDef, 'setClassMetadata should follow ngModuleDef'); + expect(setClassMetadata) + .toBeGreaterThan(ngInjectorDef, 'setClassMetadata should follow ngInjectorDef'); + }); + it('should render classes without decorators if handler matches', () => { const {renderer, decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses, testFormatter} =