fix(ngcc): ensure rendering formatters work with IIFE wrapped classes (#36989)

After the refactoring of the reflection hosts to accommodate
ES2015 classes wrapped in IIFEs. The same treatment needs to
be applied to the rendering formatters.

PR Close #36989
This commit is contained in:
Pete Bacon Darwin 2020-05-12 08:20:00 +01:00 committed by Kara Erickson
parent d7440c452a
commit c8ee390d23
7 changed files with 469 additions and 409 deletions

View File

@ -2489,7 +2489,7 @@ function isSynthesizedSuperCall(expression: ts.Expression): boolean {
* Find the statement that contains the given node * Find the statement that contains the given node
* @param node a node whose containing statement we wish to find * @param node a node whose containing statement we wish to find
*/ */
function getContainingStatement(node: ts.Node): ts.Statement { export function getContainingStatement(node: ts.Node): ts.Statement {
while (node.parent) { while (node.parent) {
if (ts.isBlock(node.parent) || ts.isSourceFile(node.parent)) { if (ts.isBlock(node.parent) || ts.isSourceFile(node.parent)) {
break; break;

View File

@ -8,10 +8,12 @@
import {Statement} from '@angular/compiler'; import {Statement} from '@angular/compiler';
import MagicString from 'magic-string'; import MagicString from 'magic-string';
import * as ts from 'typescript'; import * as ts from 'typescript';
import {NOOP_DEFAULT_IMPORT_RECORDER} from '../../../src/ngtsc/imports'; import {NOOP_DEFAULT_IMPORT_RECORDER} from '../../../src/ngtsc/imports';
import {ImportManager, translateStatement} from '../../../src/ngtsc/translator'; import {ImportManager, translateStatement} from '../../../src/ngtsc/translator';
import {CompiledClass} from '../analysis/types'; import {CompiledClass} from '../analysis/types';
import {getIifeBody} from '../host/esm5_host'; import {getContainingStatement} from '../host/esm2015_host';
import {EsmRenderingFormatter} from './esm_rendering_formatter'; import {EsmRenderingFormatter} from './esm_rendering_formatter';
/** /**
@ -20,11 +22,24 @@ import {EsmRenderingFormatter} from './esm_rendering_formatter';
*/ */
export class Esm5RenderingFormatter extends EsmRenderingFormatter { export class Esm5RenderingFormatter extends EsmRenderingFormatter {
/** /**
* Add the definitions inside the IIFE of each decorated class * Add the definitions, directly before the return statement, inside the IIFE of each decorated
* class.
*/ */
addDefinitions(output: MagicString, compiledClass: CompiledClass, definitions: string): void { addDefinitions(output: MagicString, compiledClass: CompiledClass, definitions: string): void {
const iifeBody = getIifeBody(compiledClass.declaration); const classSymbol = this.host.getClassSymbol(compiledClass.declaration);
if (!iifeBody) { if (!classSymbol) {
throw new Error(
`Compiled class "${compiledClass.name}" in "${
compiledClass.declaration.getSourceFile()
.fileName}" does not have a valid syntax.\n` +
`Expected an ES5 IIFE wrapped function. But got:\n` +
compiledClass.declaration.getText());
}
const declarationStatement =
getContainingStatement(classSymbol.implementation.valueDeclaration);
const iifeBody = declarationStatement.parent;
if (!iifeBody || !ts.isBlock(iifeBody)) {
throw new Error(`Compiled class declaration is not inside an IIFE: ${compiledClass.name} in ${ throw new Error(`Compiled class declaration is not inside an IIFE: ${compiledClass.name} in ${
compiledClass.declaration.getSourceFile().fileName}`); compiledClass.declaration.getSourceFile().fileName}`);
} }

View File

@ -16,7 +16,7 @@ import {isDtsPath} from '../../../src/ngtsc/util/src/typescript';
import {ModuleWithProvidersInfo} from '../analysis/module_with_providers_analyzer'; import {ModuleWithProvidersInfo} from '../analysis/module_with_providers_analyzer';
import {ExportInfo} from '../analysis/private_declarations_analyzer'; import {ExportInfo} from '../analysis/private_declarations_analyzer';
import {CompiledClass} from '../analysis/types'; import {CompiledClass} from '../analysis/types';
import {isAssignment} from '../host/esm2015_host'; import {getContainingStatement, isAssignment} from '../host/esm2015_host';
import {NgccReflectionHost, POST_R3_MARKER, PRE_R3_MARKER, SwitchableVariableDeclaration} from '../host/ngcc_host'; import {NgccReflectionHost, POST_R3_MARKER, PRE_R3_MARKER, SwitchableVariableDeclaration} from '../host/ngcc_host';
import {RedundantDecoratorMap, RenderingFormatter} from './rendering_formatter'; import {RedundantDecoratorMap, RenderingFormatter} from './rendering_formatter';
@ -104,7 +104,8 @@ export class EsmRenderingFormatter implements RenderingFormatter {
if (!classSymbol) { if (!classSymbol) {
throw new Error(`Compiled class does not have a valid symbol: ${compiledClass.name}`); throw new Error(`Compiled class does not have a valid symbol: ${compiledClass.name}`);
} }
const declarationStatement = getDeclarationStatement(classSymbol.declaration.valueDeclaration); const declarationStatement =
getContainingStatement(classSymbol.implementation.valueDeclaration);
const insertionPoint = declarationStatement.getEnd(); const insertionPoint = declarationStatement.getEnd();
output.appendLeft(insertionPoint, '\n' + definitions); output.appendLeft(insertionPoint, '\n' + definitions);
} }
@ -277,17 +278,6 @@ export class EsmRenderingFormatter implements RenderingFormatter {
} }
} }
function getDeclarationStatement(node: ts.Node): ts.Statement {
let statement = node;
while (statement) {
if (ts.isVariableStatement(statement) || ts.isClassDeclaration(statement)) {
return statement;
}
statement = statement.parent;
}
throw new Error(`Class is not defined in a declaration statement: ${node.getText()}`);
}
function findStatement(node: ts.Node): ts.Statement|undefined { function findStatement(node: ts.Node): ts.Statement|undefined {
while (node) { while (node) {
if (ts.isExpressionStatement(node) || ts.isReturnStatement(node)) { if (ts.isExpressionStatement(node) || ts.isReturnStatement(node)) {

View File

@ -315,8 +315,11 @@ SOME DEFINITION TEXT
program, absoluteFromSourceFile(sourceFile), 'NoIife', ts.isFunctionDeclaration); program, absoluteFromSourceFile(sourceFile), 'NoIife', ts.isFunctionDeclaration);
const mockNoIifeClass: any = {declaration: noIifeDeclaration, name: 'NoIife'}; const mockNoIifeClass: any = {declaration: noIifeDeclaration, name: 'NoIife'};
expect(() => renderer.addDefinitions(output, mockNoIifeClass, 'SOME DEFINITION TEXT')) expect(() => renderer.addDefinitions(output, mockNoIifeClass, 'SOME DEFINITION TEXT'))
.toThrowError(`Compiled class declaration is not inside an IIFE: NoIife in ${ .toThrowError(
_('/node_modules/test-package/some/file.js')}`); `Compiled class "NoIife" in "${
_('/node_modules/test-package/some/file.js')}" does not have a valid syntax.\n` +
`Expected an ES5 IIFE wrapped function. But got:\n` +
`function NoIife() {}`);
const badIifeDeclaration = getDeclaration( const badIifeDeclaration = getDeclaration(
program, absoluteFromSourceFile(sourceFile), 'BadIife', ts.isVariableDeclaration); program, absoluteFromSourceFile(sourceFile), 'BadIife', ts.isVariableDeclaration);

View File

@ -321,8 +321,11 @@ SOME DEFINITION TEXT
program, absoluteFromSourceFile(sourceFile), 'NoIife', ts.isFunctionDeclaration); program, absoluteFromSourceFile(sourceFile), 'NoIife', ts.isFunctionDeclaration);
const mockNoIifeClass: any = {declaration: noIifeDeclaration, name: 'NoIife'}; const mockNoIifeClass: any = {declaration: noIifeDeclaration, name: 'NoIife'};
expect(() => renderer.addDefinitions(output, mockNoIifeClass, 'SOME DEFINITION TEXT')) expect(() => renderer.addDefinitions(output, mockNoIifeClass, 'SOME DEFINITION TEXT'))
.toThrowError(`Compiled class declaration is not inside an IIFE: NoIife in ${ .toThrowError(
_('/node_modules/test-package/some/file.js')}`); `Compiled class "NoIife" in "${
_('/node_modules/test-package/some/file.js')}" does not have a valid syntax.\n` +
`Expected an ES5 IIFE wrapped function. But got:\n` +
`function NoIife() {}`);
const badIifeDeclaration = getDeclaration( const badIifeDeclaration = getDeclaration(
program, absoluteFromSourceFile(sourceFile), 'BadIife', ts.isVariableDeclaration); program, absoluteFromSourceFile(sourceFile), 'BadIife', ts.isVariableDeclaration);

View File

@ -54,17 +54,8 @@ function setup(files: TestFile[], dtsFiles?: TestFile[]) {
}; };
} }
runInEachFileSystem(() => { const PROGRAM_CONTENT: Record<string, string> = {
describe('EsmRenderingFormatter', () => { 'top-level': `
let _: typeof absoluteFrom;
let PROGRAM: TestFile;
beforeEach(() => {
_ = absoluteFrom;
PROGRAM = {
name: _('/node_modules/test-package/some/file.js'),
contents: `
/* A copyright notice */ /* A copyright notice */
import 'some-side-effect'; import 'some-side-effect';
import {Directive} from '@angular/core'; import {Directive} from '@angular/core';
@ -93,12 +84,69 @@ function compileNgModuleFactory__PRE_R3__(injector, options, moduleType) {
return compiler.compileModuleAsync(moduleType); return compiler.compileModuleAsync(moduleType);
} }
function compileNgModuleFactory__POST_R3__(injector, options, moduleType) {
ngDevMode && assertNgModuleType(moduleType);
return Promise.resolve(new R3NgModuleFactory(moduleType));
}
// Some other content`,
'iife-wrapped': `
/* A copyright notice */
import 'some-side-effect';
import {Directive} from '@angular/core';
let A = /** @class */ (() => {
class A {}
A.decorators = [
{ type: Directive, args: [{ selector: '[a]' }] },
{ type: OtherA }
];
return A;
})();
let B = /** @class */ (() => {
class B {}
B.decorators = [
{ type: OtherB },
{ type: Directive, args: [{ selector: '[b]' }] }
];
return B;
})();
var C_1;
let C = C_1 = /** @class */ (() => {
class C {}
C.decorators = [
{ type: Directive, args: [{ selector: '[c]' }] },
];
return C;
})();
export A, B, C;
let compileNgModuleFactory = compileNgModuleFactory__PRE_R3__;
let badlyFormattedVariable = __PRE_R3__badlyFormattedVariable;
function compileNgModuleFactory__PRE_R3__(injector, options, moduleType) {
const compilerFactory = injector.get(CompilerFactory);
const compiler = compilerFactory.createCompiler([options]);
return compiler.compileModuleAsync(moduleType);
}
function compileNgModuleFactory__POST_R3__(injector, options, moduleType) { function compileNgModuleFactory__POST_R3__(injector, options, moduleType) {
ngDevMode && assertNgModuleType(moduleType); ngDevMode && assertNgModuleType(moduleType);
return Promise.resolve(new R3NgModuleFactory(moduleType)); return Promise.resolve(new R3NgModuleFactory(moduleType));
} }
// Some other content` // Some other content`
}; };
runInEachFileSystem(() => {
['top-level', 'iife-wrapped'].forEach(classLayout => {
describe(`EsmRenderingFormatter {${classLayout} classes}`, () => {
let _: typeof absoluteFrom;
let PROGRAM: TestFile;
beforeEach(() => {
_ = absoluteFrom;
PROGRAM = {
name: _('/node_modules/test-package/some/file.js'),
contents: PROGRAM_CONTENT[classLayout],
};
}); });
describe('addImports', () => { describe('addImports', () => {
@ -175,7 +223,7 @@ export {TopLevelComponent};`);
import {Directive} from '@angular/core'; import {Directive} from '@angular/core';
const x = 3; const x = 3;
export class A {}`); `);
}); });
it('should insert constants after inserted imports', () => { it('should insert constants after inserted imports', () => {
@ -189,7 +237,7 @@ import {Directive} from '@angular/core';
import * as i0 from '@angular/core'; import * as i0 from '@angular/core';
const x = 3; const x = 3;
export class A {`); `);
}); });
}); });
@ -222,8 +270,7 @@ export class A {`);
const compiledClass = const compiledClass =
decorationAnalyses.get(sourceFile)!.compiledClasses.find(c => c.name === 'A')!; decorationAnalyses.get(sourceFile)!.compiledClasses.find(c => c.name === 'A')!;
renderer.addDefinitions(output, compiledClass, 'SOME DEFINITION TEXT'); renderer.addDefinitions(output, compiledClass, 'SOME DEFINITION TEXT');
expect(output.toString()).toContain(` expect(output.toString()).toContain(`class A {}
export class A {}
SOME DEFINITION TEXT SOME DEFINITION TEXT
A.decorators = [ A.decorators = [
`); `);
@ -236,11 +283,8 @@ A.decorators = [
const compiledClass = const compiledClass =
decorationAnalyses.get(sourceFile)!.compiledClasses.find(c => c.name === 'C')!; decorationAnalyses.get(sourceFile)!.compiledClasses.find(c => c.name === 'C')!;
renderer.addDefinitions(output, compiledClass, 'SOME DEFINITION TEXT'); renderer.addDefinitions(output, compiledClass, 'SOME DEFINITION TEXT');
expect(output.toString()).toContain(` expect(output.toString())
let C = C_1 = class C {}; .toMatch(/class C \{\};?\nSOME DEFINITION TEXT\nC.decorators = \[/);
SOME DEFINITION TEXT
C.decorators = [
`);
}); });
}); });
@ -638,7 +682,9 @@ export { D };
expect(renderer.printStatement(stmt1, sourceFile, importManager)).toBe('const foo = 42;'); expect(renderer.printStatement(stmt1, sourceFile, importManager)).toBe('const foo = 42;');
expect(renderer.printStatement(stmt2, sourceFile, importManager)).toBe('var bar = true;'); expect(renderer.printStatement(stmt2, sourceFile, importManager)).toBe('var bar = true;');
expect(renderer.printStatement(stmt3, sourceFile, importManager)).toBe('var baz = "qux";'); expect(renderer.printStatement(stmt3, sourceFile, importManager))
.toBe('var baz = "qux";');
});
}); });
}); });
}); });

View File

@ -481,8 +481,11 @@ SOME DEFINITION TEXT
program, absoluteFromSourceFile(sourceFile), 'NoIife', ts.isFunctionDeclaration); program, absoluteFromSourceFile(sourceFile), 'NoIife', ts.isFunctionDeclaration);
const mockNoIifeClass: any = {declaration: noIifeDeclaration, name: 'NoIife'}; const mockNoIifeClass: any = {declaration: noIifeDeclaration, name: 'NoIife'};
expect(() => renderer.addDefinitions(output, mockNoIifeClass, 'SOME DEFINITION TEXT')) expect(() => renderer.addDefinitions(output, mockNoIifeClass, 'SOME DEFINITION TEXT'))
.toThrowError(`Compiled class declaration is not inside an IIFE: NoIife in ${ .toThrowError(
_('/node_modules/test-package/some/file.js')}`); `Compiled class "NoIife" in "${
_('/node_modules/test-package/some/file.js')}" does not have a valid syntax.\n` +
`Expected an ES5 IIFE wrapped function. But got:\n` +
`function NoIife() {}`);
const badIifeDeclaration = getDeclaration( const badIifeDeclaration = getDeclaration(
program, absoluteFromSourceFile(sourceFile), 'BadIife', ts.isVariableDeclaration); program, absoluteFromSourceFile(sourceFile), 'BadIife', ts.isVariableDeclaration);