honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(ngcc): report the correct viaModule when reflecting over commonjs (#33192) 

In the ReflectionHost API, a 'viaModule' indicates that a particular value
originated in another absolute module. It should always be 'null' for values
originating in relatively-imported modules.

This commit fixes a bug in the CommonJsReflectionHost where viaModule would
be reported even for relatively-imported values, which causes invalid import
statements to be generated during compilation.

A test is added to verify the correct behavior.

FW-1628 #resolve

PR Close #33192
This commit is contained in:
Alex Rickabaugh 2019-10-02 10:30:53 +03:00 committed by Matias Niemelä
parent 2196114501
commit afcff73be3
2 changed files with 57 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
packages/compiler-cli/ngcc/src/host/commonjs_host.ts
View File

@ -174,7 +174,8 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
return null;
}
return {node: importedFile, viaModule: importInfo.from};
const viaModule = !importInfo.from.startsWith('.') ? importInfo.from : null;
return {node: importedFile, viaModule};
}
private resolveModuleName(moduleName: string, containingFile: ts.SourceFile): ts.SourceFile

57
packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts
View File

@ -1653,6 +1653,59 @@ exports.ExternalModule = ExternalModule;
expect(actualDeclaration !.node).toBe(expectedDeclarationNode);
expect(actualDeclaration !.viaModule).toBe('@angular/core');
});
it('should return viaModule: null for relative imports', () => {
loadTestFiles([
{
name: _('/index.js'),
contents: `
const a = require('./a');
var b = a.b;
`,
},
{
name: _('/a.js'),
contents: `
exports.b = 1;
`
}
]);
const {program, host: compilerHost} = makeTestBundleProgram(_('/index.js'));
const host = new CommonJsReflectionHost(new MockLogger(), false, program, compilerHost);
const variableNode =
getDeclaration(program, _('/index.js'), 'b', isNamedVariableDeclaration);
const identifier = variableNode.name as ts.Identifier;
const importOfIdent = host.getDeclarationOfIdentifier(identifier !) !;
expect(importOfIdent.node).not.toBeNull();
expect(importOfIdent.viaModule).toBeNull();
});
it('should return a viaModule for absolute imports', () => {
loadTestFiles([
{
name: _('/index.js'),
contents: `
var a = require('lib');
var b = a.x;
`,
},
{
name: _('/node_modules/lib/index.d.ts'),
contents: 'export declare const x: number;',
},
]);
const {program, host: compilerHost} = makeTestBundleProgram(_('/index.js'));
const host = new CommonJsReflectionHost(new MockLogger(), false, program, compilerHost);
const variableNode =
getDeclaration(program, _('/index.js'), 'b', isNamedVariableDeclaration);
const identifier = (variableNode.initializer !as ts.PropertyAccessExpression).name;
const importOfIdent = host.getDeclarationOfIdentifier(identifier !) !;
expect(importOfIdent.viaModule).toBe('lib');
});
});
describe('getExportsOfModule()', () => {
@ -1668,9 +1721,9 @@ exports.ExternalModule = ExternalModule;
.map(entry => [entry[0], entry[1].node !.getText(), entry[1].viaModule]))
.toEqual([
['Directive', `Directive: FnWithArg<(clazz: any) => any>`, '@angular/core'],
['a', `a = 'a'`, './a_module'],
['a', `a = 'a'`, null],
['b', `b = a_module.a`, null],
['c', `a = 'a'`, './a_module'],
['c', `a = 'a'`, null],
['d', `b = a_module.a`, null],
['e', `e = 'e'`, null],
['DirectiveX', `Directive: FnWithArg<(clazz: any) => any>`, '@angular/core'],