Skip to content
Permalink
Browse files

fix(ngcc): remove `__decorator` calls even when part of the IIFE retu…

…rn 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
  • Loading branch information
petebacondarwin authored and kara committed Nov 13, 2019
1 parent 1b48908 commit e1df98be17a45e05ba0d083f14a19bb2866d7b66
@@ -17,6 +17,7 @@ import {ExportInfo} from '../analysis/private_declarations_analyzer';
import {RenderingFormatter, RedundantDecoratorMap} from './rendering_formatter';
import {stripExtension} from './utils';
import {Reexport} from '../../../src/ngtsc/imports';
import {isAssignment} from '../host/esm2015_host';

/**
* A RenderingFormatter that works with ECMAScript Module import and export statements.
@@ -124,7 +125,19 @@ export class EsmRenderingFormatter implements RenderingFormatter {
// Remove the entire statement
const statement = findStatement(containerNode);
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 {
nodesToRemove.forEach(node => {
@@ -239,7 +252,7 @@ export class EsmRenderingFormatter implements RenderingFormatter {

function findStatement(node: ts.Node) {
while (node) {
if (ts.isExpressionStatement(node)) {
if (ts.isExpressionStatement(node) || ts.isReturnStatement(node)) {
return node;
}
node = node.parent;
@@ -257,3 +270,9 @@ function getNextSiblingInArray<T extends ts.Node>(node: T, array: ts.NodeArray<T
const index = array.indexOf(node);
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();
}
@@ -160,6 +160,20 @@ var D = /** @class */ (function () {
return 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`
};
});
@@ -445,62 +459,91 @@ SOME DEFINITION TEXT
expect(output.toString()).not.toContain(`C.decorators`);
});

});

describe('[__decorate declarations]', () => {
it('should delete the decorator (and following comma) 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 === 'A') !;
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: '[a]' }),`);
expect(output.toString()).toContain(`OtherA()`);
expect(output.toString()).toContain(`Directive({ selector: '[b]' })`);
expect(output.toString()).toContain(`OtherB()`);
expect(output.toString()).toContain(`Directive({ selector: '[c]' })`);
});

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 output = new MagicString(PROGRAM_DECORATE_HELPER.contents);
const compiledClass =
decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'B') !;
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()).not.toContain(`Directive({ selector: '[b]' })`);
expect(output.toString()).toContain(`OtherB()`);
expect(output.toString()).toContain(`Directive({ selector: '[c]' })`);
});


it('should delete the decorator (and its container if there are no 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;`);
});
describe('[__decorate declarations]', () => {
it('should delete the decorator (and following comma) 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 === 'A') !;
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: '[a]' }),`);
expect(output.toString()).toContain(`OtherA()`);
expect(output.toString()).toContain(`Directive({ selector: '[b]' })`);
expect(output.toString()).toContain(`OtherB()`);
expect(output.toString()).toContain(`Directive({ selector: '[c]' })`);
});

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 output = new MagicString(PROGRAM_DECORATE_HELPER.contents);
const compiledClass =
decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'B') !;
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()).not.toContain(`Directive({ selector: '[b]' })`);
expect(output.toString()).toContain(`OtherB()`);
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 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 output = new MagicString(PROGRAM_DECORATE_HELPER.contents);
const compiledClass =
decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'E') !;
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: '[e]' })`);
expect(output.toString()).not.toContain(`E = tslib_1.__decorate([`);
expect(output.toString()).toContain(`function E() {}\n return E;`);
});

it('should delete call to `__decorate` but keep a return statement and no semi-colon if there are no other decorators left',
() => {
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`);
});
});
});
});
});

0 comments on commit e1df98b

Please sign in to comment.
You can’t perform that action at this time.