fix(ivy): ngcc: remove redundant `__decorate()` calls (#26236)

Previously we only removed assignments to `Class.decorators = [];`
if the array was not empty.

Now we also remove calls to `__decorate([])`, similarly.

PR Close #26236
This commit is contained in:
Pete Bacon Darwin 2018-09-24 20:50:25 +01:00 committed by Jason Aden
parent 44c05c05af
commit 50d1cba174
3 changed files with 257 additions and 40 deletions

View File

@ -54,11 +54,11 @@ export class Esm2015Renderer extends Renderer {
if (ts.isArrayLiteralExpression(containerNode)) {
const items = containerNode.elements;
if (items.length === nodesToRemove.length) {
// remove any trailing semi-colon
const end = (output.slice(containerNode.getEnd(), containerNode.getEnd() + 1) === ';') ?
containerNode.getEnd() + 1 :
containerNode.getEnd();
output.remove(containerNode.parent !.getFullStart(), end);
// Remove the entire statement
const statement = findStatement(containerNode);
if (statement) {
output.remove(statement.getFullStart(), statement.getEnd());
}
} else {
nodesToRemove.forEach(node => {
// remove any trailing comma
@ -82,3 +82,13 @@ export class Esm2015Renderer extends Renderer {
});
}
}
function findStatement(node: ts.Node) {
while (node) {
if (ts.isExpressionStatement(node)) {
return node;
}
node = node.parent;
}
return undefined;
}

View File

@ -62,6 +62,44 @@ function compileNgModuleFactory__POST_NGCC__(injector, options, moduleType) {
// Some other content`
};
const PROGRAM_DECORATE_HELPER = {
name: 'some/file.js',
contents: `
import * as tslib_1 from "tslib";
var D_1;
/* A copyright notice */
import { Directive } from '@angular/core';
const OtherA = () => (node) => { };
const OtherB = () => (node) => { };
let A = class A {
};
A = tslib_1.__decorate([
Directive({ selector: '[a]' }),
OtherA()
], A);
export { A };
let B = class B {
};
B = tslib_1.__decorate([
OtherB(),
Directive({ selector: '[b]' })
], B);
export { B };
let C = class C {
};
C = tslib_1.__decorate([
Directive({ selector: '[c]' })
], C);
export { C };
let D = D_1 = class D {
};
D = D_1 = tslib_1.__decorate([
Directive({ selector: '[d]', providers: [D_1] })
], D);
export { D };
// Some other content`
};
describe('Esm2015Renderer', () => {
describe('addImports', () => {
@ -136,8 +174,9 @@ A.decorators = [
describe('removeDecorators', () => {
it('should delete the decorator (and following comma) that was matched in the analysis', () => {
describe('[static property declaration]', () => {
it('should delete the decorator (and following comma) that was matched in the analysis',
() => {
const {analyzer, parser, program, renderer} = setup(PROGRAM);
const analyzedFile = analyze(parser, analyzer, program.getSourceFile(PROGRAM.name) !);
const output = new MagicString(PROGRAM.contents);
@ -146,7 +185,8 @@ A.decorators = [
const decoratorsToRemove = new Map<ts.Node, ts.Node[]>();
decoratorsToRemove.set(decorator.node.parent !, [decorator.node]);
renderer.removeDecorators(output, decoratorsToRemove);
expect(output.toString()).not.toContain(`{ type: Directive, args: [{ selector: '[a]' }] },`);
expect(output.toString())
.not.toContain(`{ type: Directive, args: [{ selector: '[a]' }] },`);
expect(output.toString()).toContain(`{ type: OtherA }`);
expect(output.toString()).toContain(`{ type: Directive, args: [{ selector: '[b]' }] }`);
expect(output.toString()).toContain(`{ type: OtherB }`);
@ -173,7 +213,7 @@ A.decorators = [
});
it('should delete the decorator (and its container if there are not other decorators left) that was matched in the analysis',
it('should delete the decorator (and its container if there are no other decorators left) that was matched in the analysis',
() => {
const {analyzer, parser, program, renderer} = setup(PROGRAM);
const analyzedFile = analyze(parser, analyzer, program.getSourceFile(PROGRAM.name) !);
@ -187,10 +227,68 @@ A.decorators = [
expect(output.toString()).toContain(`{ type: OtherA }`);
expect(output.toString()).toContain(`{ type: Directive, args: [{ selector: '[b]' }] }`);
expect(output.toString()).toContain(`{ type: OtherB }`);
expect(output.toString()).not.toContain(`C.decorators = [
{ type: Directive, args: [{ selector: '[c]' }] },
];`);
expect(output.toString())
.not.toContain(`{ type: Directive, args: [{ selector: '[c]' }] }`);
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 {analyzer, parser, program, renderer} = setup(PROGRAM_DECORATE_HELPER);
const analyzedFile =
analyze(parser, analyzer, program.getSourceFile(PROGRAM_DECORATE_HELPER.name) !);
const output = new MagicString(PROGRAM_DECORATE_HELPER.contents);
const analyzedClass = analyzedFile.analyzedClasses.find(c => c.name === 'A') !;
const decorator = analyzedClass.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 {analyzer, parser, program, renderer} = setup(PROGRAM_DECORATE_HELPER);
const analyzedFile =
analyze(parser, analyzer, program.getSourceFile(PROGRAM_DECORATE_HELPER.name) !);
const output = new MagicString(PROGRAM_DECORATE_HELPER.contents);
const analyzedClass = analyzedFile.analyzedClasses.find(c => c.name === 'B') !;
const decorator = analyzedClass.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 {analyzer, parser, program, renderer} = setup(PROGRAM_DECORATE_HELPER);
const analyzedFile =
analyze(parser, analyzer, program.getSourceFile(PROGRAM_DECORATE_HELPER.name) !);
const output = new MagicString(PROGRAM_DECORATE_HELPER.contents);
const analyzedClass = analyzedFile.analyzedClasses.find(c => c.name === 'C') !;
const decorator = analyzedClass.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(`let C = class C {\n};\nexport { C };`);
});
});
});

View File

@ -74,6 +74,57 @@ function compileNgModuleFactory__POST_NGCC__(injector, options, moduleType) {
export {A, B, C};`
};
const PROGRAM_DECORATE_HELPER = {
name: 'some/file.js',
contents: `
import * as tslib_1 from "tslib";
/* A copyright notice */
import { Directive } from '@angular/core';
var OtherA = function () { return function (node) { }; };
var OtherB = function () { return function (node) { }; };
var A = /** @class */ (function () {
function A() {
}
A = tslib_1.__decorate([
Directive({ selector: '[a]' }),
OtherA()
], A);
return A;
}());
export { A };
var B = /** @class */ (function () {
function B() {
}
B = tslib_1.__decorate([
OtherB(),
Directive({ selector: '[b]' })
], B);
return B;
}());
export { B };
var C = /** @class */ (function () {
function C() {
}
C = tslib_1.__decorate([
Directive({ selector: '[c]' })
], C);
return C;
}());
export { C };
var D = /** @class */ (function () {
function D() {
}
D_1 = D;
var D_1;
D = D_1 = tslib_1.__decorate([
Directive({ selector: '[d]', providers: [D_1] })
], D);
return D;
}());
export { D };
// Some other content`
};
describe('Esm5Renderer', () => {
describe('addImports', () => {
@ -205,4 +256,62 @@ SOME DEFINITION TEXT
});
});
describe('[__decorate declarations]', () => {
it('should delete the decorator (and following comma) that was matched in the analysis', () => {
const {analyzer, parser, program, renderer} = setup(PROGRAM_DECORATE_HELPER);
const analyzedFile =
analyze(parser, analyzer, program.getSourceFile(PROGRAM_DECORATE_HELPER.name) !);
const output = new MagicString(PROGRAM_DECORATE_HELPER.contents);
const analyzedClass = analyzedFile.analyzedClasses.find(c => c.name === 'A') !;
const decorator = analyzedClass.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 {analyzer, parser, program, renderer} = setup(PROGRAM_DECORATE_HELPER);
const analyzedFile =
analyze(parser, analyzer, program.getSourceFile(PROGRAM_DECORATE_HELPER.name) !);
const output = new MagicString(PROGRAM_DECORATE_HELPER.contents);
const analyzedClass = analyzedFile.analyzedClasses.find(c => c.name === 'B') !;
const decorator = analyzedClass.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 {analyzer, parser, program, renderer} = setup(PROGRAM_DECORATE_HELPER);
const analyzedFile =
analyze(parser, analyzer, program.getSourceFile(PROGRAM_DECORATE_HELPER.name) !);
const output = new MagicString(PROGRAM_DECORATE_HELPER.contents);
const analyzedClass = analyzedFile.analyzedClasses.find(c => c.name === 'C') !;
const decorator = analyzedClass.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;`);
});
});
});