fix(ngcc): remove `__decorator` calls even when part of the IIFE return statement (#33777)

Previously we only removed `__decorate()` calls that looked like:

```
SomeClass = __decorate(...);
```

But in some minified scenarios this call gets wrapped up with the
return statement of the IIFE.

```
return SomeClass = __decorate(...);
```

This is now removed also, leaving just the return statement:

```
return SomeClass;
```

PR Close #33777
This commit is contained in:
Pete Bacon Darwin 2019-11-13 08:36:13 +00:00 committed by Kara Erickson
parent 9c6ee5fcd0
commit d21471e24e
2 changed files with 116 additions and 54 deletions

View File

@ -17,6 +17,7 @@ import {ExportInfo} from '../analysis/private_declarations_analyzer';
import {RenderingFormatter, RedundantDecoratorMap} from './rendering_formatter'; import {RenderingFormatter, RedundantDecoratorMap} from './rendering_formatter';
import {stripExtension} from './utils'; import {stripExtension} from './utils';
import {Reexport} from '../../../src/ngtsc/imports'; import {Reexport} from '../../../src/ngtsc/imports';
import {isAssignment} from '../host/esm2015_host';
/** /**
* A RenderingFormatter that works with ECMAScript Module import and export statements. * A RenderingFormatter that works with ECMAScript Module import and export statements.
@ -124,7 +125,19 @@ export class EsmRenderingFormatter implements RenderingFormatter {
// Remove the entire statement // Remove the entire statement
const statement = findStatement(containerNode); const statement = findStatement(containerNode);
if (statement) { if (statement) {
output.remove(statement.getFullStart(), statement.getEnd()); if (ts.isExpressionStatement(statement)) {
// The statement looks like: `SomeClass = __decorate(...);`
// Remove it completely
output.remove(statement.getFullStart(), statement.getEnd());
} else if (
ts.isReturnStatement(statement) && statement.expression &&
isAssignment(statement.expression)) {
// The statement looks like: `return SomeClass = __decorate(...);`
// We only want to end up with: `return SomeClass;`
const startOfRemoval = statement.expression.left.getEnd();
const endOfRemoval = getEndExceptSemicolon(statement);
output.remove(startOfRemoval, endOfRemoval);
}
} }
} else { } else {
nodesToRemove.forEach(node => { nodesToRemove.forEach(node => {
@ -239,7 +252,7 @@ export class EsmRenderingFormatter implements RenderingFormatter {
function findStatement(node: ts.Node) { function findStatement(node: ts.Node) {
while (node) { while (node) {
if (ts.isExpressionStatement(node)) { if (ts.isExpressionStatement(node) || ts.isReturnStatement(node)) {
return node; return node;
} }
node = node.parent; node = node.parent;
@ -257,3 +270,9 @@ function getNextSiblingInArray<T extends ts.Node>(node: T, array: ts.NodeArray<T
const index = array.indexOf(node); const index = array.indexOf(node);
return index !== -1 && array.length > index + 1 ? array[index + 1] : null; return index !== -1 && array.length > index + 1 ? array[index + 1] : null;
} }
function getEndExceptSemicolon(statement: ts.Statement): number {
const lastToken = statement.getLastToken();
return (lastToken && lastToken.kind === ts.SyntaxKind.SemicolonToken) ? statement.getEnd() - 1 :
statement.getEnd();
}

View File

@ -160,6 +160,20 @@ var D = /** @class */ (function () {
return D; return D;
}()); }());
export { D }; export { D };
var E = /** @class */ (function () {
function E() {}
return E = tslib_1.__decorate([
Directive({ selector: '[e]' })
], E);
}());
export { E };
var F = /** @class */ (function () {
function F() {}
return F = tslib_1.__decorate([
Directive({ selector: '[f]' })
], F)
}());
export { F };
// Some other content` // Some other content`
}; };
}); });
@ -445,62 +459,91 @@ SOME DEFINITION TEXT
expect(output.toString()).not.toContain(`C.decorators`); expect(output.toString()).not.toContain(`C.decorators`);
}); });
});
describe('[__decorate declarations]', () => { describe('[__decorate declarations]', () => {
it('should delete the decorator (and following comma) that was matched in the analysis', it('should delete the decorator (and following comma) that was matched in the analysis',
() => { () => {
const {renderer, decorationAnalyses, sourceFile} = setup(PROGRAM_DECORATE_HELPER); const {renderer, decorationAnalyses, sourceFile} = setup(PROGRAM_DECORATE_HELPER);
const output = new MagicString(PROGRAM_DECORATE_HELPER.contents); const output = new MagicString(PROGRAM_DECORATE_HELPER.contents);
const compiledClass = const compiledClass =
decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'A') !; decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'A') !;
const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !; const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !;
const decoratorsToRemove = new Map<ts.Node, ts.Node[]>(); const decoratorsToRemove = new Map<ts.Node, ts.Node[]>();
decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]);
renderer.removeDecorators(output, decoratorsToRemove); renderer.removeDecorators(output, decoratorsToRemove);
expect(output.toString()).not.toContain(`Directive({ selector: '[a]' }),`); expect(output.toString()).not.toContain(`Directive({ selector: '[a]' }),`);
expect(output.toString()).toContain(`OtherA()`); expect(output.toString()).toContain(`OtherA()`);
expect(output.toString()).toContain(`Directive({ selector: '[b]' })`); expect(output.toString()).toContain(`Directive({ selector: '[b]' })`);
expect(output.toString()).toContain(`OtherB()`); expect(output.toString()).toContain(`OtherB()`);
expect(output.toString()).toContain(`Directive({ selector: '[c]' })`); expect(output.toString()).toContain(`Directive({ selector: '[c]' })`);
}); });
it('should delete the decorator (but cope with no trailing comma) that was matched in the analysis', it('should delete the decorator (but cope with no trailing comma) that was matched in the analysis',
() => { () => {
const {renderer, decorationAnalyses, sourceFile} = setup(PROGRAM_DECORATE_HELPER); const {renderer, decorationAnalyses, sourceFile} = setup(PROGRAM_DECORATE_HELPER);
const output = new MagicString(PROGRAM_DECORATE_HELPER.contents); const output = new MagicString(PROGRAM_DECORATE_HELPER.contents);
const compiledClass = const compiledClass =
decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'B') !; decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'B') !;
const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !; const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !;
const decoratorsToRemove = new Map<ts.Node, ts.Node[]>(); const decoratorsToRemove = new Map<ts.Node, ts.Node[]>();
decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]);
renderer.removeDecorators(output, decoratorsToRemove); renderer.removeDecorators(output, decoratorsToRemove);
expect(output.toString()).toContain(`Directive({ selector: '[a]' }),`); expect(output.toString()).toContain(`Directive({ selector: '[a]' }),`);
expect(output.toString()).toContain(`OtherA()`); expect(output.toString()).toContain(`OtherA()`);
expect(output.toString()).not.toContain(`Directive({ selector: '[b]' })`); expect(output.toString()).not.toContain(`Directive({ selector: '[b]' })`);
expect(output.toString()).toContain(`OtherB()`); expect(output.toString()).toContain(`OtherB()`);
expect(output.toString()).toContain(`Directive({ selector: '[c]' })`); expect(output.toString()).toContain(`Directive({ selector: '[c]' })`);
}); });
it('should delete the decorator (and its container if there are not other decorators left) that was matched in the analysis',
() => {
const {renderer, decorationAnalyses, sourceFile} = setup(PROGRAM_DECORATE_HELPER);
const output = new MagicString(PROGRAM_DECORATE_HELPER.contents);
const compiledClass =
decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'C') !;
const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !;
const decoratorsToRemove = new Map<ts.Node, ts.Node[]>();
decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]);
renderer.removeDecorators(output, decoratorsToRemove);
expect(output.toString()).toContain(`Directive({ selector: '[a]' }),`);
expect(output.toString()).toContain(`OtherA()`);
expect(output.toString()).toContain(`Directive({ selector: '[b]' })`);
expect(output.toString()).toContain(`OtherB()`);
expect(output.toString()).not.toContain(`Directive({ selector: '[c]' })`);
expect(output.toString()).not.toContain(`C = tslib_1.__decorate([`);
expect(output.toString()).toContain(`function C() {\n }\n return C;`);
});
it('should delete the decorator (and its container if there are no other decorators left) that was matched in the analysis', it('should delete call to `__decorate` but keep a return statement and semi-colon if there are no other decorators left',
() => { () => {
const {renderer, decorationAnalyses, sourceFile} = setup(PROGRAM_DECORATE_HELPER); const {renderer, decorationAnalyses, sourceFile} = setup(PROGRAM_DECORATE_HELPER);
const output = new MagicString(PROGRAM_DECORATE_HELPER.contents); const output = new MagicString(PROGRAM_DECORATE_HELPER.contents);
const compiledClass = const compiledClass =
decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'C') !; decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'E') !;
const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !; const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !;
const decoratorsToRemove = new Map<ts.Node, ts.Node[]>(); const decoratorsToRemove = new Map<ts.Node, ts.Node[]>();
decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]);
renderer.removeDecorators(output, decoratorsToRemove); renderer.removeDecorators(output, decoratorsToRemove);
expect(output.toString()).toContain(`Directive({ selector: '[a]' }),`); expect(output.toString()).not.toContain(`Directive({ selector: '[e]' })`);
expect(output.toString()).toContain(`OtherA()`); expect(output.toString()).not.toContain(`E = tslib_1.__decorate([`);
expect(output.toString()).toContain(`Directive({ selector: '[b]' })`); expect(output.toString()).toContain(`function E() {}\n return E;`);
expect(output.toString()).toContain(`OtherB()`); });
expect(output.toString()).not.toContain(`Directive({ selector: '[c]' })`);
expect(output.toString()).not.toContain(`C = tslib_1.__decorate([`); it('should delete call to `__decorate` but keep a return statement and no semi-colon if there are no other decorators left',
expect(output.toString()).toContain(`function C() {\n }\n return C;`); () => {
}); const {renderer, decorationAnalyses, sourceFile} = setup(PROGRAM_DECORATE_HELPER);
const output = new MagicString(PROGRAM_DECORATE_HELPER.contents);
const compiledClass =
decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'F') !;
const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !;
const decoratorsToRemove = new Map<ts.Node, ts.Node[]>();
decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]);
renderer.removeDecorators(output, decoratorsToRemove);
expect(output.toString()).not.toContain(`Directive({ selector: '[f]' })`);
expect(output.toString()).not.toContain(`F = tslib_1.__decorate([`);
expect(output.toString()).toContain(`function F() {}\n return F\n`);
});
});
}); });
}); });
}); });