fix(ngcc): correctly detect outer aliased class identifiers in ES5 (#35527)

In ES5 and ES2015, class identifiers may have aliases. Previously, the
`NgccReflectionHost`s recognized the following formats:
- ES5:
    ```js
    var MyClass = (function () {
      function InnerClass() {}
      InnerClass_1 = InnerClass;
      ...
    }());
    ```
- ES2015:
    ```js
    let MyClass = MyClass_1 = class MyClass { ... };
    ```

In addition to the above, this commit adds support for recognizing an
alias outside the IIFE in ES5 classes (which was previously not
supported):
```js
var MyClass = MyClass_1 = (function () { ... }());
```

Jira issue: [FW-1869](https://angular-team.atlassian.net/browse/FW-1869)

Partially addresses #35399.

PR Close #35527
This commit is contained in:
George Kalpakas 2020-02-18 13:17:59 +02:00 committed by Miško Hevery
parent 2baf90209b
commit fde89156fa
5 changed files with 229 additions and 46 deletions

View File

@ -634,7 +634,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
if (!this.preprocessedSourceFiles.has(sourceFile)) { if (!this.preprocessedSourceFiles.has(sourceFile)) {
this.preprocessedSourceFiles.add(sourceFile); this.preprocessedSourceFiles.add(sourceFile);
for (const statement of sourceFile.statements) { for (const statement of this.getModuleStatements(sourceFile)) {
this.preprocessStatement(statement); this.preprocessStatement(statement);
} }
} }
@ -660,7 +660,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
const declaration = declarations[0]; const declaration = declarations[0];
const initializer = declaration.initializer; const initializer = declaration.initializer;
if (!ts.isIdentifier(declaration.name) || !initializer || !isAssignment(initializer) || if (!ts.isIdentifier(declaration.name) || !initializer || !isAssignment(initializer) ||
!ts.isIdentifier(initializer.left) || !ts.isClassExpression(initializer.right)) { !ts.isIdentifier(initializer.left) || !this.isClass(declaration)) {
return; return;
} }

View File

@ -295,8 +295,8 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost {
* @param checker the TS program TypeChecker * @param checker the TS program TypeChecker
* @returns the inner function declaration or `undefined` if it is not a "class". * @returns the inner function declaration or `undefined` if it is not a "class".
*/ */
protected getInnerFunctionDeclarationFromClassDeclaration(decl: ts.Declaration): ts.FunctionDeclaration protected getInnerFunctionDeclarationFromClassDeclaration(decl: ts.Declaration):
|undefined { ts.FunctionDeclaration|undefined {
// Extract the IIFE body (if any). // Extract the IIFE body (if any).
const iifeBody = getIifeBody(decl); const iifeBody = getIifeBody(decl);
if (!iifeBody) return undefined; if (!iifeBody) return undefined;
@ -605,7 +605,15 @@ export function getIifeBody(declaration: ts.Declaration): ts.Block|undefined {
return undefined; return undefined;
} }
const call = stripParentheses(declaration.initializer); // Recognize a variable declaration of one of the forms:
// - `var MyClass = (function () { ... }());`
// - `var MyClass = MyClass_1 = (function () { ... }());`
let parenthesizedCall = declaration.initializer;
while (isAssignment(parenthesizedCall)) {
parenthesizedCall = parenthesizedCall.right;
}
const call = stripParentheses(parenthesizedCall);
if (!ts.isCallExpression(call)) { if (!ts.isCallExpression(call)) {
return undefined; return undefined;
} }

View File

@ -795,7 +795,8 @@ exports.MissingClass2 = MissingClass2;
contents: ` contents: `
var functions = require('./functions'); var functions = require('./functions');
var methods = require('./methods'); var methods = require('./methods');
var aliased_class = require('./aliased_class'); var outer_aliased_class = require('./outer_aliased_class');
var inner_aliased_class = require('./inner_aliased_class');
` `
}, },
{ {
@ -877,7 +878,19 @@ exports.InternalModule = InternalModule;
` `
}, },
{ {
name: _('/src/aliased_class.js'), name: _('/src/outer_aliased_class.js'),
contents: `
var AliasedModule = AliasedModule_1 = (function() {
function AliasedModule() {}
return AliasedModule;
}());
AliasedModule.forRoot = function() { return { ngModule: AliasedModule_1 }; };
exports.AliasedModule = AliasedModule;
var AliasedModule_1;
`
},
{
name: _('/src/inner_aliased_class.js'),
contents: ` contents: `
var AliasedModule = (function() { var AliasedModule = (function() {
function AliasedModule() {} function AliasedModule() {}
@ -1670,6 +1683,35 @@ exports.ExternalModule = ExternalModule;
expect(actualDeclaration !.viaModule).toBe(null); expect(actualDeclaration !.viaModule).toBe(null);
}); });
it('should return the correct declaration for an outer alias identifier', () => {
const PROGRAM_FILE: TestFile = {
name: _('/test.js'),
contents: `
var AliasedClass = AliasedClass_1 = (function () {
function InnerClass() {
}
return InnerClass;
}());
var AliasedClass_1;
`,
};
loadTestFiles([PROGRAM_FILE]);
const bundle = makeTestBundleProgram(PROGRAM_FILE.name);
const host = new CommonJsReflectionHost(new MockLogger(), false, bundle);
const expectedDeclaration = getDeclaration(
bundle.program, PROGRAM_FILE.name, 'AliasedClass', isNamedVariableDeclaration);
// Grab the `AliasedClass_1` identifier (which is an alias for `AliasedClass`).
const aliasIdentifier =
(expectedDeclaration.initializer as ts.BinaryExpression).left as ts.Identifier;
const actualDeclaration = host.getDeclarationOfIdentifier(aliasIdentifier);
expect(aliasIdentifier.getText()).toBe('AliasedClass_1');
expect(actualDeclaration).not.toBe(null);
expect(actualDeclaration !.node).toBe(expectedDeclaration);
});
it('should return the source-file of an import namespace', () => { it('should return the source-file of an import namespace', () => {
loadFakeCore(getFileSystem()); loadFakeCore(getFileSystem());
loadTestFiles([SOME_DIRECTIVE_FILE]); loadTestFiles([SOME_DIRECTIVE_FILE]);
@ -2403,17 +2445,30 @@ exports.ExternalModule = ExternalModule;
]); ]);
}); });
it('should resolve aliased module references to their original declaration (outer alias)',
() => {
loadTestFiles(MODULE_WITH_PROVIDERS_PROGRAM);
const bundle = makeTestBundleProgram(_('/src/index.js'));
const host = new CommonJsReflectionHost(new MockLogger(), false, bundle);
const file = getSourceFileOrError(bundle.program, _('/src/outer_aliased_class.js'));
const fn = host.getModuleWithProvidersFunctions(file);
expect(fn.map(fn => [fn.declaration.getText(), fn.ngModule.node.name.text])).toEqual([
['function() { return { ngModule: AliasedModule_1 }; }', 'AliasedModule'],
]);
});
// https://github.com/angular/angular/issues/29078 // https://github.com/angular/angular/issues/29078
it('should resolve aliased module references to their original declaration', () => { it('should resolve aliased module references to their original declaration (inner alias)',
loadTestFiles(MODULE_WITH_PROVIDERS_PROGRAM); () => {
const bundle = makeTestBundleProgram(_('/src/index.js')); loadTestFiles(MODULE_WITH_PROVIDERS_PROGRAM);
const host = new CommonJsReflectionHost(new MockLogger(), false, bundle); const bundle = makeTestBundleProgram(_('/src/index.js'));
const file = getSourceFileOrError(bundle.program, _('/src/aliased_class.js')); const host = new CommonJsReflectionHost(new MockLogger(), false, bundle);
const fn = host.getModuleWithProvidersFunctions(file); const file = getSourceFileOrError(bundle.program, _('/src/inner_aliased_class.js'));
expect(fn.map(fn => [fn.declaration.getText(), fn.ngModule.node.name.text])).toEqual([ const fn = host.getModuleWithProvidersFunctions(file);
['function() { return { ngModule: AliasedModule_1 }; }', 'AliasedModule'], expect(fn.map(fn => [fn.declaration.getText(), fn.ngModule.node.name.text])).toEqual([
]); ['function() { return { ngModule: AliasedModule_1 }; }', 'AliasedModule'],
}); ]);
});
}); });
}); });
}); });

View File

@ -772,7 +772,8 @@ runInEachFileSystem(() => {
contents: ` contents: `
import * as functions from './functions'; import * as functions from './functions';
import * as methods from './methods'; import * as methods from './methods';
import * as aliased_class from './aliased_class'; import * as outer_aliased_class from './outer_aliased_class';
import * as inner_aliased_class from './inner_aliased_class';
` `
}, },
{ {
@ -842,7 +843,19 @@ runInEachFileSystem(() => {
` `
}, },
{ {
name: _('/src/aliased_class.js'), name: _('/src/outer_aliased_class.js'),
contents: `
var AliasedModule = AliasedModule_1 = (function() {
function AliasedModule() {}
return AliasedModule;
}());
AliasedModule.forRoot = function() { return { ngModule: AliasedModule_1 }; };
export { AliasedModule };
var AliasedModule_1;
`
},
{
name: _('/src/inner_aliased_class.js'),
contents: ` contents: `
var AliasedModule = (function() { var AliasedModule = (function() {
function AliasedModule() {} function AliasedModule() {}
@ -2028,6 +2041,34 @@ runInEachFileSystem(() => {
expect(superGetDeclarationOfIdentifierSpy).toHaveBeenCalledTimes(2); expect(superGetDeclarationOfIdentifierSpy).toHaveBeenCalledTimes(2);
}); });
it('should return the correct declaration for an outer alias identifier', () => {
const PROGRAM_FILE: TestFile = {
name: _('/test.js'),
contents: `
var AliasedClass = AliasedClass_1 = (function () {
function InnerClass() {
}
return InnerClass;
}());
var AliasedClass_1;
`,
};
loadTestFiles([PROGRAM_FILE]);
const bundle = makeTestBundleProgram(PROGRAM_FILE.name);
const host = new Esm5ReflectionHost(new MockLogger(), false, bundle);
const expectedDeclaration = getDeclaration(
bundle.program, PROGRAM_FILE.name, 'AliasedClass', isNamedVariableDeclaration);
// Grab the `AliasedClass_1` identifier (which is an alias for `AliasedClass`).
const aliasIdentifier =
(expectedDeclaration.initializer as ts.BinaryExpression).left as ts.Identifier;
const actualDeclaration = host.getDeclarationOfIdentifier(aliasIdentifier) !;
expect(aliasIdentifier.getText()).toBe('AliasedClass_1');
expect(actualDeclaration.node !.getText()).toBe(expectedDeclaration.getText());
});
it('should return the correct outer declaration for an aliased inner class declaration inside an ES5 IIFE', it('should return the correct outer declaration for an aliased inner class declaration inside an ES5 IIFE',
() => { () => {
// Note that the inner class declaration `function FroalaEditorModule() {}` is aliased // Note that the inner class declaration `function FroalaEditorModule() {}` is aliased
@ -2672,17 +2713,30 @@ runInEachFileSystem(() => {
]); ]);
}); });
it('should resolve aliased module references to their original declaration (outer alias)',
() => {
loadTestFiles(MODULE_WITH_PROVIDERS_PROGRAM);
const bundle = makeTestBundleProgram(_('/src/index.js'));
const host = new Esm5ReflectionHost(new MockLogger(), false, bundle);
const file = getSourceFileOrError(bundle.program, _('/src/outer_aliased_class.js'));
const fn = host.getModuleWithProvidersFunctions(file);
expect(fn.map(fn => [fn.declaration.getText(), fn.ngModule.node.name.text])).toEqual([
['function() { return { ngModule: AliasedModule_1 }; }', 'AliasedModule'],
]);
});
// https://github.com/angular/angular/issues/29078 // https://github.com/angular/angular/issues/29078
it('should resolve aliased module references to their original declaration', () => { it('should resolve aliased module references to their original declaration (inner alias)',
loadTestFiles(MODULE_WITH_PROVIDERS_PROGRAM); () => {
const bundle = makeTestBundleProgram(_('/src/index.js')); loadTestFiles(MODULE_WITH_PROVIDERS_PROGRAM);
const host = new Esm5ReflectionHost(new MockLogger(), false, bundle); const bundle = makeTestBundleProgram(_('/src/index.js'));
const file = getSourceFileOrError(bundle.program, _('/src/aliased_class.js')); const host = new Esm5ReflectionHost(new MockLogger(), false, bundle);
const fn = host.getModuleWithProvidersFunctions(file); const file = getSourceFileOrError(bundle.program, _('/src/inner_aliased_class.js'));
expect(fn.map(fn => [fn.declaration.getText(), fn.ngModule.node.name.text])).toEqual([ const fn = host.getModuleWithProvidersFunctions(file);
['function() { return { ngModule: AliasedModule_1 }; }', 'AliasedModule'], expect(fn.map(fn => [fn.declaration.getText(), fn.ngModule.node.name.text])).toEqual([
]); ['function() { return { ngModule: AliasedModule_1 }; }', 'AliasedModule'],
}); ]);
});
}); });
describe('getEndOfClass()', () => { describe('getEndOfClass()', () => {

View File

@ -929,10 +929,10 @@ runInEachFileSystem(() => {
name: _('/src/index.js'), name: _('/src/index.js'),
contents: ` contents: `
(function (global, factory) { (function (global, factory) {
typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('./functions'), require('./methods'), require('./aliased_class')) : typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('./functions'), require('./methods'), require('./outer_aliased_class'), require('./inner_aliased_class')) :
typeof define === 'function' && define.amd ? define('index', ['exports', './functions', './methods', './aliased_class'], factory) : typeof define === 'function' && define.amd ? define('index', ['exports', './functions', './methods', './outer_aliased_class', './inner_aliased_class'], factory) :
(factory(global.index,global.functions,global.methods,global.aliased_class)); (factory(global.index,global.functions,global.methods,global.outer_aliased_class,global.inner_aliased_class));
}(this, (function (exports,functions,methods,aliased_class) { 'use strict'; }(this, (function (exports,functions,methods,outer_aliased_class,inner_aliased_class) { 'use strict';
})))); }))));
`, `,
}, },
@ -1025,12 +1025,30 @@ runInEachFileSystem(() => {
` `
}, },
{ {
name: _('/src/aliased_class.js'), name: _('/src/outer_aliased_class.js'),
contents: ` contents: `
(function (global, factory) { (function (global, factory) {
typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports) : typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports) :
typeof define === 'function' && define.amd ? define('aliased_class', ['exports'], factory) : typeof define === 'function' && define.amd ? define('outer_aliased_class', ['exports'], factory) :
(factory(global.aliased_class)); (factory(global.outer_aliased_class));
}(this, (function (exports,module) { 'use strict';
var AliasedModule = AliasedModule_1 = (function() {
function AliasedModule() {}
return AliasedModule;
}());
AliasedModule.forRoot = function() { return { ngModule: AliasedModule_1 }; };
exports.AliasedModule = AliasedModule;
var AliasedModule_1;
})));
`
},
{
name: _('/src/inner_aliased_class.js'),
contents: `
(function (global, factory) {
typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports) :
typeof define === 'function' && define.amd ? define('inner_aliased_class', ['exports'], factory) :
(factory(global.inner_aliased_class));
}(this, (function (exports,module) { 'use strict'; }(this, (function (exports,module) { 'use strict';
var AliasedModule = (function() { var AliasedModule = (function() {
function AliasedModule() {} function AliasedModule() {}
@ -1831,6 +1849,41 @@ runInEachFileSystem(() => {
expect(actualDeclaration !.viaModule).toBe(null); expect(actualDeclaration !.viaModule).toBe(null);
}); });
it('should return the correct declaration for an outer alias identifier', () => {
const PROGRAM_FILE: TestFile = {
name: _('/test.js'),
contents: `
(function (global, factory) {
typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports) :
typeof define === 'function' && define.amd ? define('test', ['exports'], factory) :
(factory(global.test));
}(this, (function (exports,module) { 'use strict';
var AliasedClass = AliasedClass_1 = (function () {
function InnerClass() {
}
return InnerClass;
}());
var AliasedClass_1;
})));
`,
};
loadTestFiles([PROGRAM_FILE]);
const bundle = makeTestBundleProgram(PROGRAM_FILE.name);
const host = new UmdReflectionHost(new MockLogger(), false, bundle);
const expectedDeclaration = getDeclaration(
bundle.program, PROGRAM_FILE.name, 'AliasedClass', isNamedVariableDeclaration);
// Grab the `AliasedClass_1` identifier (which is an alias for `AliasedClass`).
const aliasIdentifier =
(expectedDeclaration.initializer as ts.BinaryExpression).left as ts.Identifier;
const actualDeclaration = host.getDeclarationOfIdentifier(aliasIdentifier);
expect(aliasIdentifier.getText()).toBe('AliasedClass_1');
expect(actualDeclaration).not.toBe(null);
expect(actualDeclaration !.node).toBe(expectedDeclaration);
});
it('should return the source-file of an import namespace', () => { it('should return the source-file of an import namespace', () => {
loadFakeCore(getFileSystem()); loadFakeCore(getFileSystem());
loadTestFiles([SOME_DIRECTIVE_FILE]); loadTestFiles([SOME_DIRECTIVE_FILE]);
@ -2642,17 +2695,30 @@ runInEachFileSystem(() => {
]); ]);
}); });
it('should resolve aliased module references to their original declaration (outer alias)',
() => {
loadTestFiles(MODULE_WITH_PROVIDERS_PROGRAM);
const bundle = makeTestBundleProgram(getRootFiles(MODULE_WITH_PROVIDERS_PROGRAM)[0]);
const host = new UmdReflectionHost(new MockLogger(), false, bundle);
const file = getSourceFileOrError(bundle.program, _('/src/outer_aliased_class.js'));
const fn = host.getModuleWithProvidersFunctions(file);
expect(fn.map(fn => [fn.declaration.getText(), fn.ngModule.node.name.text])).toEqual([
['function() { return { ngModule: AliasedModule_1 }; }', 'AliasedModule'],
]);
});
// https://github.com/angular/angular/issues/29078 // https://github.com/angular/angular/issues/29078
it('should resolve aliased module references to their original declaration', () => { it('should resolve aliased module references to their original declaration (inner alias)',
loadTestFiles(MODULE_WITH_PROVIDERS_PROGRAM); () => {
const bundle = makeTestBundleProgram(getRootFiles(MODULE_WITH_PROVIDERS_PROGRAM)[0]); loadTestFiles(MODULE_WITH_PROVIDERS_PROGRAM);
const host = new UmdReflectionHost(new MockLogger(), false, bundle); const bundle = makeTestBundleProgram(getRootFiles(MODULE_WITH_PROVIDERS_PROGRAM)[0]);
const file = getSourceFileOrError(bundle.program, _('/src/aliased_class.js')); const host = new UmdReflectionHost(new MockLogger(), false, bundle);
const fn = host.getModuleWithProvidersFunctions(file); const file = getSourceFileOrError(bundle.program, _('/src/inner_aliased_class.js'));
expect(fn.map(fn => [fn.declaration.getText(), fn.ngModule.node.name.text])).toEqual([ const fn = host.getModuleWithProvidersFunctions(file);
['function() { return { ngModule: AliasedModule_1 }; }', 'AliasedModule'], expect(fn.map(fn => [fn.declaration.getText(), fn.ngModule.node.name.text])).toEqual([
]); ['function() { return { ngModule: AliasedModule_1 }; }', 'AliasedModule'],
}); ]);
});
}); });
}); });
}); });