From afcff73be30411b37963b90807106c95bc92a7e7 Mon Sep 17 00:00:00 2001
From: Alex Rickabaugh <alx+alxhub@alxandria.net>
Date: Wed, 2 Oct 2019 10:30:53 +0300
Subject: [PATCH] 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
---
 .../ngcc/src/host/commonjs_host.ts            |  3 +-
 .../ngcc/test/host/commonjs_host_spec.ts      | 57 ++++++++++++++++++-
 2 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/packages/compiler-cli/ngcc/src/host/commonjs_host.ts b/packages/compiler-cli/ngcc/src/host/commonjs_host.ts
index 4df4ed664d..e63623c8bc 100644
--- a/packages/compiler-cli/ngcc/src/host/commonjs_host.ts
+++ b/packages/compiler-cli/ngcc/src/host/commonjs_host.ts
@@ -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
diff --git a/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts b/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts
index 67e6c93102..72f01d82c8 100644
--- a/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts
+++ b/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts
@@ -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'],