fix(compiler-cli): Use typescript to resolve modules for metadata (#22856)

The current module resolution simply attaches .ts to the import/export path, which does
not work if the path is using Node / CommonJS behavior to resolve to an index.ts file.
This patch uses typescript's module resolution logic, and will attempt to load the original
typescript file if this resolution returns a .js or .d.ts file

PR Close #22856

packages/compiler-cli/src/metadata/bundle_index_host.ts

@@ -103,7 +103,7 @@ export function createBundleIndexHost<H extends ts.CompilerHost>(
   // etc.
   const getMetadataBundle = (cache: MetadataCache | null) => {
     const bundler = new MetadataBundler(
-        indexModule, ngOptions.flatModuleId, new CompilerHostAdapter(host, cache),
+        indexModule, ngOptions.flatModuleId, new CompilerHostAdapter(host, cache, ngOptions),
         ngOptions.flatModulePrivateSymbolPrefix);
     return bundler.getMetadataBundle();
   };

packages/compiler-cli/src/metadata/bundler.ts

@@ -72,7 +72,7 @@ export interface BundledModule {
 }
 
 export interface MetadataBundlerHost {
-  getMetadataFor(moduleName: string): ModuleMetadata|undefined;
+  getMetadataFor(moduleName: string, containingFile: string): ModuleMetadata|undefined;
 }
 
 type StaticsMetadata = {
@@ -136,7 +136,7 @@ export class MetadataBundler {
     if (!result) {
       if (moduleName.startsWith('.')) {
         const fullModuleName = resolveModule(moduleName, this.root);
-        result = this.host.getMetadataFor(fullModuleName);
+        result = this.host.getMetadataFor(fullModuleName, this.root);
       }
       this.metadataCache.set(moduleName, result);
     }
@@ -598,11 +598,27 @@ export class MetadataBundler {
 export class CompilerHostAdapter implements MetadataBundlerHost {
   private collector = new MetadataCollector();
 
-  constructor(private host: ts.CompilerHost, private cache: MetadataCache|null) {}
+  constructor(
+      private host: ts.CompilerHost, private cache: MetadataCache|null,
+      private options: ts.CompilerOptions) {}
+
+  getMetadataFor(fileName: string, containingFile: string): ModuleMetadata|undefined {
+    const {resolvedModule} =
+        ts.resolveModuleName(fileName, containingFile, this.options, this.host);
+
+    let sourceFile: ts.SourceFile|undefined;
+    if (resolvedModule) {
+      let {resolvedFileName} = resolvedModule;
+      if (resolvedModule.extension !== '.ts') {
+        resolvedFileName = resolvedFileName.replace(/(\.d\.ts|\.js)$/, '.ts');
+      }
+      sourceFile = this.host.getSourceFile(resolvedFileName, ts.ScriptTarget.Latest);
+    } else {
+      // If typescript is unable to resolve the file, fallback on old behavior
+      if (!this.host.fileExists(fileName + '.ts')) return undefined;
+      sourceFile = this.host.getSourceFile(fileName + '.ts', ts.ScriptTarget.Latest);
+    }
 
-  getMetadataFor(fileName: string): ModuleMetadata|undefined {
-    if (!this.host.fileExists(fileName + '.ts')) return undefined;
-    const sourceFile = this.host.getSourceFile(fileName + '.ts', ts.ScriptTarget.Latest);
     // If there is a metadata cache, use it to get the metadata for this source file. Otherwise,
     // fall back on the locally created MetadataCollector.
     if (!sourceFile) {

packages/compiler-cli/test/metadata/BUILD.bazel

@@ -9,6 +9,7 @@ ts_library(
         "//packages:types",
         "//packages/compiler",
         "//packages/compiler-cli",
+        "//packages/compiler-cli/test:test_utils",
         "//packages/core",
     ],
 )

packages/compiler-cli/test/metadata/bundler_spec.ts

@@ -9,11 +9,187 @@
 import * as path from 'path';
 import * as ts from 'typescript';
 
-import {MetadataBundler, MetadataBundlerHost} from '../../src/metadata/bundler';
+import {CompilerHostAdapter, MetadataBundler, MetadataBundlerHost} from '../../src/metadata/bundler';
 import {MetadataCollector} from '../../src/metadata/collector';
 import {ClassMetadata, MetadataGlobalReferenceExpression, ModuleMetadata} from '../../src/metadata/schema';
+import {Directory, MockAotContext, MockCompilerHost} from '../mocks';
 
-import {Directory, open} from './typescript.mocks';
+describe('compiler host adapter', () => {
+
+  it('should retrieve metadata for an explicit index relative path reference', () => {
+    const context = new MockAotContext('.', SIMPLE_LIBRARY);
+    const host = new MockCompilerHost(context);
+    const options: ts.CompilerOptions = {
+      moduleResolution: ts.ModuleResolutionKind.NodeJs,
+      module: ts.ModuleKind.CommonJS,
+      target: ts.ScriptTarget.ES5,
+    };
+    const adapter = new CompilerHostAdapter(host, null, options);
+    const metadata = adapter.getMetadataFor('./lib/src/two/index', '.');
+
+    expect(metadata).toBeDefined();
+    expect(Object.keys(metadata !.metadata).sort()).toEqual([
+      'PrivateTwo',
+      'TWO_CLASSES',
+      'Two',
+      'TwoMore',
+    ]);
+  });
+
+  it('should retrieve metadata for an implied index relative path reference', () => {
+    const context = new MockAotContext('.', SIMPLE_LIBRARY_WITH_IMPLIED_INDEX);
+    const host = new MockCompilerHost(context);
+    const options: ts.CompilerOptions = {
+      moduleResolution: ts.ModuleResolutionKind.NodeJs,
+      module: ts.ModuleKind.CommonJS,
+      target: ts.ScriptTarget.ES5,
+    };
+    const adapter = new CompilerHostAdapter(host, null, options);
+    const metadata = adapter.getMetadataFor('./lib/src/two', '.');
+
+    expect(metadata).toBeDefined();
+    expect(Object.keys(metadata !.metadata).sort()).toEqual([
+      'PrivateTwo',
+      'TWO_CLASSES',
+      'Two',
+      'TwoMore',
+    ]);
+  });
+
+  it('should fail to retrieve metadata for an implied index with classic module resolution', () => {
+    const context = new MockAotContext('.', SIMPLE_LIBRARY_WITH_IMPLIED_INDEX);
+    const host = new MockCompilerHost(context);
+    const options: ts.CompilerOptions = {
+      moduleResolution: ts.ModuleResolutionKind.Classic,
+      module: ts.ModuleKind.CommonJS,
+      target: ts.ScriptTarget.ES5,
+    };
+    const adapter = new CompilerHostAdapter(host, null, options);
+    const metadata = adapter.getMetadataFor('./lib/src/two', '.');
+
+    expect(metadata).toBeUndefined();
+  });
+
+  it('should retrieve exports for an explicit index relative path reference', () => {
+    const context = new MockAotContext('.', SIMPLE_LIBRARY);
+    const host = new MockCompilerHost(context);
+    const options: ts.CompilerOptions = {
+      moduleResolution: ts.ModuleResolutionKind.NodeJs,
+      module: ts.ModuleKind.CommonJS,
+      target: ts.ScriptTarget.ES5,
+    };
+    const adapter = new CompilerHostAdapter(host, null, options);
+    const metadata = adapter.getMetadataFor('./lib/src/index', '.');
+
+    expect(metadata).toBeDefined();
+    expect(metadata !.exports !.map(e => e.export !)
+               .reduce((prev, next) => prev.concat(next), [])
+               .sort())
+        .toEqual([
+          'ONE_CLASSES',
+          'One',
+          'OneMore',
+          'TWO_CLASSES',
+          'Two',
+          'TwoMore',
+        ]);
+  });
+
+  it('should look for .ts file when resolving metadata via a package.json "main" entry', () => {
+    const files = {
+      'lib': {
+        'one.ts': `
+          class One {}
+          class OneMore extends One {}
+          class PrivateOne {}
+          const ONE_CLASSES = [One, OneMore, PrivateOne];
+          export {One, OneMore, PrivateOne, ONE_CLASSES};
+        `,
+        'one.js': `
+          // This will throw an error if the metadata collector tries to load one.js
+        `,
+        'package.json': `
+        {
+          "main": "one"
+        }
+        `
+      }
+    };
+
+    const context = new MockAotContext('.', files);
+    const host = new MockCompilerHost(context);
+    const options: ts.CompilerOptions = {
+      moduleResolution: ts.ModuleResolutionKind.NodeJs,
+      module: ts.ModuleKind.CommonJS,
+      target: ts.ScriptTarget.ES5,
+    };
+    const adapter = new CompilerHostAdapter(host, null, options);
+    const metadata = adapter.getMetadataFor('./lib', '.');
+
+    expect(metadata).toBeDefined();
+    expect(Object.keys(metadata !.metadata).sort()).toEqual([
+      'ONE_CLASSES',
+      'One',
+      'OneMore',
+      'PrivateOne',
+    ]);
+    expect(Array.isArray(metadata !.metadata !['ONE_CLASSES'])).toBeTruthy();
+  });
+
+  it('should look for non-declaration file when resolving metadata via a package.json "types" entry',
+     () => {
+       const files = {
+         'lib': {
+           'one.ts': `
+          class One {}
+          class OneMore extends One {}
+          class PrivateOne {}
+          const ONE_CLASSES = [One, OneMore, PrivateOne];
+          export {One, OneMore, PrivateOne, ONE_CLASSES};
+        `,
+           'one.d.ts': `
+          declare class One {
+          }
+          declare class OneMore extends One {
+          }
+          declare class PrivateOne {
+          }
+          declare const ONE_CLASSES: (typeof One)[];
+          export { One, OneMore, PrivateOne, ONE_CLASSES };
+        `,
+           'one.js': `
+          // This will throw an error if the metadata collector tries to load one.js
+        `,
+           'package.json': `
+        {
+          "main": "one",
+          "types": "one.d.ts"
+        }
+        `
+         }
+       };
+
+       const context = new MockAotContext('.', files);
+       const host = new MockCompilerHost(context);
+       const options: ts.CompilerOptions = {
+         moduleResolution: ts.ModuleResolutionKind.NodeJs,
+         module: ts.ModuleKind.CommonJS,
+         target: ts.ScriptTarget.ES5,
+       };
+       const adapter = new CompilerHostAdapter(host, null, options);
+       const metadata = adapter.getMetadataFor('./lib', '.');
+
+       expect(metadata).toBeDefined();
+       expect(Object.keys(metadata !.metadata).sort()).toEqual([
+         'ONE_CLASSES',
+         'One',
+         'OneMore',
+         'PrivateOne',
+       ]);
+       expect(Array.isArray(metadata !.metadata !['ONE_CLASSES'])).toBeTruthy();
+
+     });
+});
 
 describe('metadata bundler', () => {
 
@@ -231,26 +407,24 @@ describe('metadata bundler', () => {
 
 export class MockStringBundlerHost implements MetadataBundlerHost {
   collector = new MetadataCollector();
+  adapter: CompilerHostAdapter;
 
-  constructor(private dirName: string, private directory: Directory) {}
+  constructor(private dirName: string, directory: Directory) {
+    const context = new MockAotContext(dirName, directory);
+    const host = new MockCompilerHost(context);
+    const options = {
+      moduleResolution: ts.ModuleResolutionKind.NodeJs,
+      module: ts.ModuleKind.CommonJS,
+      target: ts.ScriptTarget.ES5,
+    };
+    this.adapter = new CompilerHostAdapter(host, null, options);
+  }
 
   getMetadataFor(moduleName: string): ModuleMetadata|undefined {
-    const fileName = path.join(this.dirName, moduleName) + '.ts';
-    const text = open(this.directory, fileName);
-    if (typeof text == 'string') {
-      const sourceFile = ts.createSourceFile(
-          fileName, text, ts.ScriptTarget.Latest, /* setParent */ true, ts.ScriptKind.TS);
-      const diagnostics: ts.Diagnostic[] = (sourceFile as any).parseDiagnostics;
-      if (diagnostics && diagnostics.length) {
-        throw Error('Unexpected syntax error in test');
-      }
-      const result = this.collector.getMetadata(sourceFile);
-      return result;
-    }
+    return this.adapter.getMetadataFor(moduleName, this.dirName);
   }
 }
 
-
 export const SIMPLE_LIBRARY = {
   'lib': {
     'index.ts': `
@@ -278,3 +452,31 @@ export const SIMPLE_LIBRARY = {
     }
   }
 };
+
+export const SIMPLE_LIBRARY_WITH_IMPLIED_INDEX = {
+  'lib': {
+    'index.ts': `
+      export * from './src';
+    `,
+    'src': {
+      'index.ts': `
+        export {One, OneMore, ONE_CLASSES} from './one';
+        export {Two, TwoMore, TWO_CLASSES} from './two';
+      `,
+      'one.ts': `
+        export class One {}
+        export class OneMore extends One {}
+        export class PrivateOne {}
+        export const ONE_CLASSES = [One, OneMore, PrivateOne];
+      `,
+      'two': {
+        'index.ts': `
+          export class Two {}
+          export class TwoMore extends Two {}
+          export class PrivateTwo {}
+          export const TWO_CLASSES = [Two, TwoMore, PrivateTwo];
+        `
+      }
+    }
+  }
+};

packages/compiler-cli/test/mocks.ts

@@ -19,7 +19,9 @@ export class MockAotContext {
 
   fileExists(fileName: string): boolean { return typeof this.getEntry(fileName) === 'string'; }
 
-  directoryExists(path: string): boolean { return typeof this.getEntry(path) === 'object'; }
+  directoryExists(path: string): boolean {
+    return path === this.currentDirectory || typeof this.getEntry(path) === 'object';
+  }
 
   readFile(fileName: string): string {
     const data = this.getEntry(fileName);