Skip to content

Commit

Permalink
fix(ivy): move setClassMetadata calls into a pure iife (#33337)
Browse files Browse the repository at this point in the history
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
angular/angular-cli#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
  • Loading branch information
alxhub committed Nov 20, 2019
1 parent 5267c92 commit 213e3c3
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 16 deletions.
2 changes: 1 addition & 1 deletion aio/scripts/_payload-limits.json
Expand Up @@ -12,7 +12,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 2987,
"main-es2015": 461159,
"main-es2015": 506857,
"polyfills-es2015": 52503
}
}
Expand Down
2 changes: 1 addition & 1 deletion integration/_payload-limits.json
Expand Up @@ -39,7 +39,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 2289,
"main-es2015": 268404,
"main-es2015": 312772,
"polyfills-es2015": 36808,
"5-es2015": 751
}
Expand Down
4 changes: 2 additions & 2 deletions packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts
Expand Up @@ -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', () => {
Expand Down
12 changes: 6 additions & 6 deletions packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts
Expand Up @@ -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); })();`);
});


Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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'}
Expand All @@ -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([]);
});
Expand Down
10 changes: 7 additions & 3 deletions packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
}

/**
Expand Down
Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand Down
15 changes: 15 additions & 0 deletions packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
Expand Up @@ -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';
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler/src/compiler.ts
Expand Up @@ -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';
Expand Down

0 comments on commit 213e3c3

Please sign in to comment.