From b6e88479674f8caea633d99edf061060e2051e9c Mon Sep 17 00:00:00 2001
From: George Kalpakas <kalpakas.g@gmail.com>
Date: Mon, 10 Feb 2020 19:25:36 +0200
Subject: [PATCH] fix(ngcc): handle imports in dts files when processing
 CommonJS (#35191)

When statically evaluating CommonJS code it is possible to find that we
are looking for the declaration of an identifier that actually came from
a typings file (rather than a CommonJS file).

Previously, the CommonJS reflection host would always try to use a
CommonJS specific algorithm for finding identifier declarations, but
when the id is actually in a typings file this resulted in the returned
declaration being the containing file of the declaration rather than the
declaration itself.

Now the CommonJS reflection host will check to see if the file
containing the identifier is a typings file and use the appropriate
stategy.

(Note: This is the equivalent of #34356 but for CommonJS.)

PR Close #35191
---
 .../ngcc/src/host/commonjs_host.ts            |  3 +-
 .../ngcc/test/host/commonjs_host_spec.ts      | 33 +++++++++++++++++++
 .../ngcc/test/host/umd_host_spec.ts           |  6 +---
 3 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/packages/compiler-cli/ngcc/src/host/commonjs_host.ts b/packages/compiler-cli/ngcc/src/host/commonjs_host.ts
index aa73065ee3..18dae5b3ee 100644
--- a/packages/compiler-cli/ngcc/src/host/commonjs_host.ts
+++ b/packages/compiler-cli/ngcc/src/host/commonjs_host.ts
@@ -47,7 +47,8 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
   }
 
   getDeclarationOfIdentifier(id: ts.Identifier): Declaration|null {
-    return this.getCommonJsImportedDeclaration(id) || super.getDeclarationOfIdentifier(id);
+    return (!id.getSourceFile().isDeclarationFile && this.getCommonJsImportedDeclaration(id)) ||
+        super.getDeclarationOfIdentifier(id);
   }
 
   getExportsOfModule(module: ts.Node): Map<string, Declaration>|null {
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 b824d3d486..ae02f2e410 100644
--- a/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts
+++ b/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts
@@ -1786,6 +1786,39 @@ exports.ExternalModule = ExternalModule;
           const importOfIdent = host.getDeclarationOfIdentifier(identifier !) !;
           expect(importOfIdent.viaModule).toBe('lib');
         });
+
+        it('should return the correct declaration of an identifier imported in a typings file',
+           () => {
+             const files = [
+               {
+                 name: _('/node_modules/test-package/index.d.ts'),
+                 contents: `
+                   import {SubModule} from 'sub_module';
+                   export const x = SubModule;
+                 `,
+               },
+               {
+                 name: _('/node_modules/sub_module/index.d.ts'),
+                 contents: 'export class SubModule {}',
+               }
+             ];
+             loadTestFiles(files);
+             const bundle = makeTestBundleProgram(files[0].name);
+             const host = new CommonJsReflectionHost(new MockLogger(), false, bundle);
+             const expectedDeclaration = getDeclaration(
+                 bundle.program, files[1].name, 'SubModule', isNamedClassDeclaration);
+             const x =
+                 getDeclaration(bundle.program, files[0].name, 'x', isNamedVariableDeclaration);
+             if (x.initializer === undefined || !ts.isIdentifier(x.initializer)) {
+               return fail('Expected constant `x` to have an identifer as an initializer.');
+             }
+             const decl = host.getDeclarationOfIdentifier(x.initializer);
+             if (decl === null) {
+               return fail('Expected to find a declaration for ' + x.initializer.getText());
+             }
+             expect(decl.viaModule).toEqual('sub_module');
+             expect(decl.node).toBe(expectedDeclaration);
+           });
       });
 
       describe('getExportsOfModule()', () => {
diff --git a/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts b/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts
index fba66dbc49..213f3a008f 100644
--- a/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts
+++ b/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts
@@ -1916,10 +1916,6 @@ runInEachFileSystem(() => {
               export const x = SubModule;
               `,
              },
-             {
-               name: _('/node_modules/packages.json'),
-               contents: '{ "typings: "index.d.ts" }',
-             },
              {
                name: _('/node_modules/sub_module/index.d.ts'),
                contents: `export class SubModule {}`,
@@ -1929,7 +1925,7 @@ runInEachFileSystem(() => {
            const bundle = makeTestBundleProgram(FILES[0].name);
            const host = new UmdReflectionHost(new MockLogger(), false, bundle);
            const expectedDeclaration =
-               getDeclaration(bundle.program, FILES[2].name, 'SubModule', isNamedClassDeclaration);
+               getDeclaration(bundle.program, FILES[1].name, 'SubModule', isNamedClassDeclaration);
            const x = getDeclaration(bundle.program, FILES[0].name, 'x', isNamedVariableDeclaration);
            if (x.initializer === undefined || !ts.isIdentifier(x.initializer)) {
              return fail('Expected constant `x` to have an identifer as an initializer.');