From 985cadb73da1b83a27c8e669a439aa0f696160b6 Mon Sep 17 00:00:00 2001
From: JoostK <joost.koehoorn@gmail.com>
Date: Sat, 16 Nov 2019 20:59:48 +0100
Subject: [PATCH] fix(ngcc): correctly include internal .d.ts files (#33875)

Some declaration files may not be referenced from an entry-point's
main typings file, as it may declare types that are only used internally.
ngcc has logic to include declaration files based on all source files,
to ensure internal declaration files are available.

For packages following APF layout, however, this logic was insufficient.
Consider an entry-point with base path of `/esm2015/testing` and typings
residing in `/testing`, the file
`/esm2015/testing/src/nested/internal.js` has its typings file at
`/testing/src/nested/internal.d.ts`. Previously, the declaration was
assumed to be located at `/esm2015/testing/testing/internal.d.ts` (by
means of `/esm2015/testing/src/nested/../../testing/internal.d.ts`)
which is not where the declaration file can be found. This commit
resolves the issue by looking in the correct directory.

PR Close #33875
---
 .../ngcc/src/packages/entry_point_bundle.ts   | 14 +++---
 .../test/packages/entry_point_bundle_spec.ts  | 43 +++++++++++++++++++
 2 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/packages/compiler-cli/ngcc/src/packages/entry_point_bundle.ts b/packages/compiler-cli/ngcc/src/packages/entry_point_bundle.ts
index dec8293a4e..72768725a5 100644
--- a/packages/compiler-cli/ngcc/src/packages/entry_point_bundle.ts
+++ b/packages/compiler-cli/ngcc/src/packages/entry_point_bundle.ts
@@ -73,16 +73,20 @@ export function makeEntryPointBundle(
 function computePotentialDtsFilesFromJsFiles(
     fs: FileSystem, srcProgram: ts.Program, formatPath: AbsoluteFsPath,
     typingsPath: AbsoluteFsPath) {
-  const relativePath = fs.relative(fs.dirname(formatPath), fs.dirname(typingsPath));
+  const formatRoot = fs.dirname(formatPath);
+  const typingsRoot = fs.dirname(typingsPath);
   const additionalFiles: AbsoluteFsPath[] = [];
   for (const sf of srcProgram.getSourceFiles()) {
     if (!sf.fileName.endsWith('.js')) {
       continue;
     }
-    const dtsPath = fs.resolve(
-        fs.dirname(sf.fileName), relativePath, fs.basename(sf.fileName, '.js') + '.d.ts');
-    if (fs.exists(dtsPath)) {
-      additionalFiles.push(dtsPath);
+
+    // Given a source file at e.g. `esm2015/src/some/nested/index.js`, try to resolve the
+    // declaration file under the typings root in `src/some/nested/index.d.ts`.
+    const mirroredDtsPath =
+        fs.resolve(typingsRoot, fs.relative(formatRoot, sf.fileName.replace(/\.js$/, '.d.ts')));
+    if (fs.exists(mirroredDtsPath)) {
+      additionalFiles.push(mirroredDtsPath);
     }
   }
   return additionalFiles;
diff --git a/packages/compiler-cli/ngcc/test/packages/entry_point_bundle_spec.ts b/packages/compiler-cli/ngcc/test/packages/entry_point_bundle_spec.ts
index 5d43ef0031..285a219e6f 100644
--- a/packages/compiler-cli/ngcc/test/packages/entry_point_bundle_spec.ts
+++ b/packages/compiler-cli/ngcc/test/packages/entry_point_bundle_spec.ts
@@ -128,6 +128,26 @@ runInEachFileSystem(() => {
           name: _('/node_modules/other/es2015/public_api.js'),
           contents: 'export class OtherClass {};'
         },
+
+        // Mimic an AFP package with declaration files in a different tree than the sources
+        {name: _('/node_modules/internal/index.d.ts'), contents: 'export * from "./src/index";'},
+        {name: _('/node_modules/internal/src/index.d.ts'), contents: ''},
+        {
+          name: _('/node_modules/internal/src/internal.d.ts'),
+          contents: 'export declare function internal();'
+        },
+        {
+          name: _('/node_modules/internal/esm2015/index.js'),
+          contents: 'export * from "./src/index";'
+        },
+        {
+          name: _('/node_modules/internal/esm2015/src/index.js'),
+          contents: 'import {Internal} from "./internal";'
+        },
+        {
+          name: _('/node_modules/internal/esm2015/src/internal.js'),
+          contents: 'export function internal();'
+        },
       ]);
     }
 
@@ -203,6 +223,29 @@ runInEachFileSystem(() => {
              .toContain(absoluteFrom('/node_modules/test/internal.d.ts'));
        });
 
+    it('should include equivalently named, internally imported, src files in the typings program, if `mirrorDtsFromSrc` is true',
+       () => {
+         setupMockFileSystem();
+         const fs = getFileSystem();
+         const entryPoint: EntryPoint = {
+           name: 'internal',
+           packageJson: {name: 'internal'},
+           package: absoluteFrom('/node_modules/internal'),
+           path: absoluteFrom('/node_modules/internal'),
+           typings: absoluteFrom('/node_modules/internal/index.d.ts'),
+           compiledByAngular: true,
+           ignoreMissingDependencies: false,
+           generateDeepReexports: false,
+         };
+         const esm5bundle = makeEntryPointBundle(
+             fs, entryPoint, './esm2015/index.js', false, 'esm2015', /* transformDts */ true,
+             /* pathMappings */ undefined, /* mirrorDtsFromSrc */ true);
+         expect(esm5bundle.src.program.getSourceFiles().map(sf => sf.fileName))
+             .toContain(absoluteFrom('/node_modules/internal/esm2015/src/internal.js'));
+         expect(esm5bundle.dts !.program.getSourceFiles().map(sf => sf.fileName))
+             .toContain(absoluteFrom('/node_modules/internal/src/internal.d.ts'));
+       });
+
     it('should ignore, internally imported, src files in the typings program, if `mirrorDtsFromSrc` is false',
        () => {
          setupMockFileSystem();