From 213e3c3e494c93735634d4aca1833877dc29d3a5 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Wed, 31 Jul 2019 15:20:56 -0700 Subject: [PATCH] fix(ivy): move setClassMetadata calls into a pure iife (#33337) This commit transforms the setClassMetadata calls generated by ngtsc from: ```typescript /*@__PURE__*/ setClassMetadata(...); ``` to: ```typescript /*@__PURE__*/ (function() { setClassMetadata(...); })(); ``` Without the IIFE, terser won't remove these function calls because the function calls have arguments that themselves are function calls or other impure expressions. In order to make the whole block be DCE-ed by terser, we wrap it into IIFE and mark the IIFE as pure. It should be noted that this change doesn't have any impact on CLI* with build-optimizer, which removes the whole setClassMetadata block within the webpack loader, so terser or webpack itself don't get to see it at all. This is done to prevent cross-chunk retention issues caused by webpack's internal module registry. * actually we do expect a short-term size regression while https://github.com/angular/angular-cli/pull/16228 is merged and released in the next rc of the CLI. But long term this change does nothing to CLI + build-optimizer configuration and is done primarly to correct the seemingly correct but non-function PURE annotation that builds not using build-optimizer could rely on. PR Close #33337 --- aio/scripts/_payload-limits.json | 2 +- integration/_payload-limits.json | 2 +- .../ngcc/test/integration/ngcc_spec.ts | 4 ++-- .../ngcc/test/rendering/renderer_spec.ts | 12 ++++++------ .../src/ngtsc/annotations/src/metadata.ts | 10 +++++++--- .../src/ngtsc/annotations/test/metadata_spec.ts | 4 ++-- packages/compiler-cli/test/ngtsc/ngtsc_spec.ts | 15 +++++++++++++++ packages/compiler/src/compiler.ts | 2 +- 8 files changed, 35 insertions(+), 16 deletions(-) diff --git a/aio/scripts/_payload-limits.json b/aio/scripts/_payload-limits.json index aaedf6ef13ceb..d93e78687a8ac 100755 --- a/aio/scripts/_payload-limits.json +++ b/aio/scripts/_payload-limits.json @@ -12,7 +12,7 @@ "master": { "uncompressed": { "runtime-es2015": 2987, - "main-es2015": 461159, + "main-es2015": 506857, "polyfills-es2015": 52503 } } diff --git a/integration/_payload-limits.json b/integration/_payload-limits.json index 1bc93012dc888..eb4ac9073a1f4 100644 --- a/integration/_payload-limits.json +++ b/integration/_payload-limits.json @@ -39,7 +39,7 @@ "master": { "uncompressed": { "runtime-es2015": 2289, - "main-es2015": 268404, + "main-es2015": 312772, "polyfills-es2015": 36808, "5-es2015": 751 } diff --git a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts index a3fdc4de455fd..bb4bdf2e30866 100644 --- a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts +++ b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts @@ -139,10 +139,10 @@ runInEachFileSystem(() => { const jsContents = fs.readFile(_(`/node_modules/test-package/index.js`)).replace(/\s+/g, ' '); expect(jsContents) .toContain( - '/*@__PURE__*/ ɵngcc0.ɵsetClassMetadata(FooDirective, ' + + '/*@__PURE__*/ (function () { ɵngcc0.ɵsetClassMetadata(FooDirective, ' + '[{ type: Directive, args: [{ selector: \'[foo]\' }] }], ' + 'function () { return []; }, ' + - '{ bar: [{ type: Input }] });'); + '{ bar: [{ type: Input }] }); })();'); }); it('should not add `const` in ES5 generated code', () => { diff --git a/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts b/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts index e7e0b958bb846..149f748a574ba 100644 --- a/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts @@ -222,10 +222,10 @@ A.ɵcmp = ɵngcc0.ɵɵdefineComponent({ type: A, selectors: [["a"]], decls: 1, v const addAdjacentStatementsSpy = testFormatter.addAdjacentStatements as jasmine.Spy; expect(addAdjacentStatementsSpy.calls.first().args[2]).toEqual(`// TRANSPILED -/*@__PURE__*/ ɵngcc0.ɵsetClassMetadata(A, [{ +/*@__PURE__*/ (function () { ɵngcc0.ɵsetClassMetadata(A, [{ type: Component, args: [{ selector: 'a', template: '{{ person!.name }}' }] - }], null, null);`); + }], null, null); })();`); }); @@ -274,10 +274,10 @@ A.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: A, selectors: [["", "a", ""]] });` .toEqual(jasmine.objectContaining( {name: 'A', decorators: [jasmine.objectContaining({name: 'Directive'})]})); expect(addAdjacentStatementsSpy.calls.first().args[2]).toEqual(`// TRANSPILED -/*@__PURE__*/ ɵngcc0.ɵsetClassMetadata(A, [{ +/*@__PURE__*/ (function () { ɵngcc0.ɵsetClassMetadata(A, [{ type: Directive, args: [{ selector: '[a]' }] - }], null, null);`); + }], null, null); })();`); }); it('should call removeDecorators with the source code, a map of class decorators that have been analyzed', @@ -568,7 +568,7 @@ UndecoratedBase.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: UndecoratedBase, vie decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses); const addAdjacentStatementsSpy = testFormatter.addAdjacentStatements as jasmine.Spy; expect(addAdjacentStatementsSpy.calls.first().args[2]) - .toContain(`/*@__PURE__*/ ɵngcc0.setClassMetadata(`); + .toContain(`/*@__PURE__*/ (function () { ɵngcc0.setClassMetadata(`); const addImportsSpy = testFormatter.addImports as jasmine.Spy; expect(addImportsSpy.calls.first().args[1]).toEqual([ {specifier: './r3_symbols', qualifier: 'ɵngcc0'} @@ -588,7 +588,7 @@ UndecoratedBase.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: UndecoratedBase, vie decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses); const addAdjacentStatementsSpy = testFormatter.addAdjacentStatements as jasmine.Spy; expect(addAdjacentStatementsSpy.calls.first().args[2]) - .toContain(`/*@__PURE__*/ setClassMetadata(`); + .toContain(`/*@__PURE__*/ (function () { setClassMetadata(`); const addImportsSpy = testFormatter.addImports as jasmine.Spy; expect(addImportsSpy.calls.first().args[1]).toEqual([]); }); diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts b/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts index 9338c056de623..589561230e0ea 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Expression, ExternalExpr, FunctionExpr, Identifiers, InvokeFunctionExpr, LiteralArrayExpr, LiteralExpr, ReturnStatement, Statement, WrappedNodeExpr, literalMap} from '@angular/compiler'; +import {Expression, ExternalExpr, FunctionExpr, Identifiers, InvokeFunctionExpr, LiteralArrayExpr, LiteralExpr, NONE_TYPE, ReturnStatement, Statement, WrappedNodeExpr, literalMap} from '@angular/compiler'; import * as ts from 'typescript'; import {DefaultImportRecorder} from '../../imports'; @@ -84,11 +84,15 @@ export function generateSetClassMetadataCall( new WrappedNodeExpr(metaDecorators), metaCtorParameters, new WrappedNodeExpr(metaPropDecorators), - ], + ]); + const iifeFn = new FunctionExpr([], [fnCall.toStmt()], NONE_TYPE); + const iife = new InvokeFunctionExpr( + /* fn */ iifeFn, + /* args */[], /* type */ undefined, /* sourceSpan */ undefined, /* pure */ true); - return fnCall.toStmt(); + return iife.toStmt(); } /** diff --git a/packages/compiler-cli/src/ngtsc/annotations/test/metadata_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/test/metadata_spec.ts index 8eff50e326b1e..536bb02c4df8b 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/test/metadata_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/test/metadata_spec.ts @@ -23,7 +23,7 @@ runInEachFileSystem(() => { @Component('metadata') class Target {} `); expect(res).toEqual( - `/*@__PURE__*/ i0.ɵsetClassMetadata(Target, [{ type: Component, args: ['metadata'] }], null, null);`); + `/*@__PURE__*/ (function () { i0.ɵsetClassMetadata(Target, [{ type: Component, args: ['metadata'] }], null, null); })();`); }); it('should convert namespaced decorated class metadata', () => { @@ -33,7 +33,7 @@ runInEachFileSystem(() => { @core.Component('metadata') class Target {} `); expect(res).toEqual( - `/*@__PURE__*/ i0.ɵsetClassMetadata(Target, [{ type: core.Component, args: ['metadata'] }], null, null);`); + `/*@__PURE__*/ (function () { i0.ɵsetClassMetadata(Target, [{ type: core.Component, args: ['metadata'] }], null, null); })();`); }); it('should convert decorated class constructor parameter metadata', () => { diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index cdc08d05e0938..749ca50934ccc 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -2853,6 +2853,21 @@ runInEachFileSystem(os => { expect(jsContents).toContain('directives: function () { return [CmpB]; }'); }); + it('should wrap setClassMetadata in an iife', () => { + env.write('test.ts', ` + import {Injectable} from '@angular/core'; + + @Injectable({providedIn: 'root'}) + export class Service {} + `); + + env.driveMain(); + const jsContents = env.getContents('test.js').replace(/\s+/g, ' '); + expect(jsContents) + .toContain( + `/*@__PURE__*/ (function () { i0.ɵsetClassMetadata(Service, [{ type: Injectable, args: [{ providedIn: 'root' }] }], null, null); })();`); + }); + it('should emit setClassMetadata calls for all types', () => { env.write('test.ts', ` import {Component, Directive, Injectable, NgModule, Pipe} from '@angular/core'; diff --git a/packages/compiler/src/compiler.ts b/packages/compiler/src/compiler.ts index e286091e74399..d898f7a5e3cb0 100644 --- a/packages/compiler/src/compiler.ts +++ b/packages/compiler/src/compiler.ts @@ -78,7 +78,7 @@ export * from './ml_parser/tags'; export {LexerRange} from './ml_parser/lexer'; export * from './ml_parser/xml_parser'; export {NgModuleCompiler} from './ng_module_compiler'; -export {ArrayType, AssertNotNull, DYNAMIC_TYPE, BinaryOperator, BinaryOperatorExpr, BuiltinMethod, BuiltinType, BuiltinTypeName, BuiltinVar, CastExpr, ClassField, ClassMethod, ClassStmt, CommaExpr, CommentStmt, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, Expression, ExpressionStatement, ExpressionType, ExpressionVisitor, ExternalExpr, ExternalReference, literalMap, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, JSDocCommentStmt, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, MapType, NotExpr, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, StatementVisitor, ThrowStmt, TryCatchStmt, Type, TypeVisitor, WrappedNodeExpr, WriteKeyExpr, WritePropExpr, WriteVarExpr, StmtModifier, Statement, STRING_TYPE, TypeofExpr, collectExternalReferences} from './output/output_ast'; +export {ArrayType, AssertNotNull, DYNAMIC_TYPE, BinaryOperator, BinaryOperatorExpr, BuiltinMethod, BuiltinType, BuiltinTypeName, BuiltinVar, CastExpr, ClassField, ClassMethod, ClassStmt, CommaExpr, CommentStmt, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, Expression, ExpressionStatement, ExpressionType, ExpressionVisitor, ExternalExpr, ExternalReference, literalMap, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, JSDocCommentStmt, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, MapType, NotExpr, NONE_TYPE, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, StatementVisitor, ThrowStmt, TryCatchStmt, Type, TypeVisitor, WrappedNodeExpr, WriteKeyExpr, WritePropExpr, WriteVarExpr, StmtModifier, Statement, STRING_TYPE, TypeofExpr, collectExternalReferences} from './output/output_ast'; export {EmitterVisitorContext} from './output/abstract_emitter'; export {JitEvaluator} from './output/output_jit'; export * from './output/ts_emitter';