fix(ivy): emit fs-relative paths when rootDir(s) aren't in effect (#33828)

Previously, the compiler assumed that all TS files logically within a
project existed under one or more "root directories". If the TS compiler
option `rootDir` or `rootDirs` was set, they would dictate the root
directories in use, otherwise the current directory was used.

Unfortunately this assumption was unfounded - it's common for projects
without explicit `rootDirs` to import from files outside the current
working directory. In such cases the `LogicalProjectStrategy` would attempt
to generate imports into those files, and fail. This would lead to no
`ReferenceEmitStrategy` being able to generate an import, and end in a
compiler assertion failure.

This commit introduces a new strategy to use when there are no `rootDirs`
explicitly present, the `RelativePathStrategy`. It uses simpler, filesystem-
relative paths to generate imports, even to files above the current working
directory.

Fixes #33659
Fixes #33562

PR Close #33828

packages/compiler-cli/src/ngtsc/imports/README.md

@@ -68,7 +68,17 @@ This `ReferenceEmitStrategy` queries the `Reference` for a `ts.Identifier` that'
 
 ### `LogicalProjectStrategy`
 
-This `ReferenceEmitStrategy` is used to import referenced classes that are declared in the current project, and not in any third-party or external libraries. It constructs an import path that's valid within the logical filesystem of the project, even if the project has multiple `rootDirs`.
+This `ReferenceEmitStrategy` is used to import referenced classes that are declared in the current project, and not in any third-party or external libraries, whenever `rootDir` or `rootDirs` is set in the TS compiler options.
+
+When `rootDir`(s) are present, multiple physical directories can be mapped into the same logical namespace. So consider two files `/app/app.cmp.ts` and `/lib/lib.cmp.ts`. Ordinarily, a relative import (such as the kind generated by `RelativePathStrategy`) from the former to the latter would be `../lib/lib.cmp`. However, if both `/app` and `/lib` are project `rootDirs`, then the files within are logically in the same "directory", and the correct import is `./lib.cmp`.
+
+The `LogicalProjectStrategy` constructs `LogicalProjectPath`s between files and generates module specifiers that are relative imports within that namespace, honoring the project's `rootDirs` settings.
+
+The `LogicalProjectStrategy` will decline to generate an import into any file which falls outside the project's `rootDirs`, as such a relative specifier is not representable in the merged namespace.
+
+### `RelativePathStrategy`
+
+This `ReferenceEmitStrategy` is used to generate relative imports between two files in the project, assuming the layout of files on the disk maps directly to the module specifier namespace. This is the case if the project does not have `rootDir`/`rootDirs` configured in its TS compiler options.
 
 ### `AbsoluteModuleStrategy`
 

packages/compiler-cli/src/ngtsc/imports/index.ts

@@ -9,7 +9,7 @@
 export {AliasStrategy, AliasingHost, FileToModuleAliasingHost, PrivateExportAliasingHost} from './src/alias';
 export {ImportRewriter, NoopImportRewriter, R3SymbolsImportRewriter, validateAndRewriteCoreSymbol} from './src/core';
 export {DefaultImportRecorder, DefaultImportTracker, NOOP_DEFAULT_IMPORT_RECORDER} from './src/default';
-export {AbsoluteModuleStrategy, FileToModuleHost, FileToModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ReferenceEmitStrategy, ReferenceEmitter} from './src/emitter';
+export {AbsoluteModuleStrategy, FileToModuleHost, FileToModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ReferenceEmitStrategy, ReferenceEmitter, RelativePathStrategy} from './src/emitter';
 export {Reexport} from './src/reexport';
 export {ImportMode, OwningModule, Reference} from './src/references';
 export {ModuleResolver} from './src/resolver';

packages/compiler-cli/src/ngtsc/imports/src/emitter.ts

@@ -7,12 +7,17 @@
  */
 import {Expression, ExternalExpr, ExternalReference, WrappedNodeExpr} from '@angular/compiler';
 import * as ts from 'typescript';
-import {LogicalFileSystem, LogicalProjectPath, absoluteFrom} from '../../file_system';
+
+import {LogicalFileSystem, LogicalProjectPath, PathSegment, absoluteFrom, absoluteFromSourceFile, basename, dirname, relative, resolve} from '../../file_system';
+import {stripExtension} from '../../file_system/src/util';
 import {ReflectionHost} from '../../reflection';
 import {getSourceFile, getSourceFileOrNull, isDeclaration, nodeNameForError, resolveModuleName} from '../../util/src/typescript';
+
 import {findExportedNameOfNode} from './find_export';
 import {ImportMode, Reference} from './references';
 
+
+
 /**
  * A host which supports an operation to convert a file name into a module name.
  *
@@ -233,6 +238,28 @@ export class LogicalProjectStrategy implements ReferenceEmitStrategy {
   }
 }
 
+/**
+ * A `ReferenceEmitStrategy` which constructs relatives paths between `ts.SourceFile`s.
+ *
+ * This strategy can be used if there is no `rootDir`/`rootDirs` structure for the project which
+ * necessitates the stronger logic of `LogicalProjectStrategy`.
+ */
+export class RelativePathStrategy implements ReferenceEmitStrategy {
+  constructor(private reflector: ReflectionHost) {}
+
+  emit(ref: Reference<ts.Node>, context: ts.SourceFile): Expression|null {
+    const destSf = getSourceFile(ref.node);
+    let moduleName = stripExtension(
+        relative(dirname(absoluteFromSourceFile(context)), absoluteFromSourceFile(destSf)));
+    if (!moduleName.startsWith('../')) {
+      moduleName = ('./' + moduleName) as PathSegment;
+    }
+
+    const name = findExportedNameOfNode(ref.node, destSf, this.reflector);
+    return new ExternalExpr({moduleName, name});
+  }
+}
+
 /**
  * A `ReferenceEmitStrategy` which uses a `FileToModuleHost` to generate absolute import references.
  */

packages/compiler-cli/src/ngtsc/program.ts

@@ -18,7 +18,7 @@ import {CycleAnalyzer, ImportGraph} from './cycles';
 import {ErrorCode, ngErrorCode} from './diagnostics';
 import {FlatIndexGenerator, ReferenceGraph, checkForPrivateExports, findFlatIndexEntryPoint} from './entry_point';
 import {AbsoluteFsPath, LogicalFileSystem, absoluteFrom} from './file_system';
-import {AbsoluteModuleStrategy, AliasStrategy, AliasingHost, DefaultImportTracker, FileToModuleAliasingHost, FileToModuleHost, FileToModuleStrategy, ImportRewriter, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, NoopImportRewriter, PrivateExportAliasingHost, R3SymbolsImportRewriter, Reference, ReferenceEmitter} from './imports';
+import {AbsoluteModuleStrategy, AliasStrategy, AliasingHost, DefaultImportTracker, FileToModuleAliasingHost, FileToModuleHost, FileToModuleStrategy, ImportRewriter, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, NoopImportRewriter, PrivateExportAliasingHost, R3SymbolsImportRewriter, Reference, ReferenceEmitStrategy, ReferenceEmitter, RelativePathStrategy} from './imports';
 import {IncrementalState} from './incremental';
 import {IndexedComponent, IndexingContext} from './indexer';
 import {generateAnalysis} from './indexer/src/transform';
@@ -520,6 +520,24 @@ export class NgtscProgram implements api.Program {
 
     // Construct the ReferenceEmitter.
     if (this.fileToModuleHost === null || !this.options._useHostForImportGeneration) {
+      let localImportStrategy: ReferenceEmitStrategy;
+
+      // The strategy used for local, in-project imports depends on whether TS has been configured
+      // with rootDirs. If so, then multiple directories may be mapped in the same "module
+      // namespace" and the logic of `LogicalProjectStrategy` is required to generate correct
+      // imports which may cross these multiple directories. Otherwise, plain relative imports are
+      // sufficient.
+      if (this.options.rootDir !== undefined ||
+          (this.options.rootDirs !== undefined && this.options.rootDirs.length > 0)) {
+        // rootDirs logic is in effect - use the `LogicalProjectStrategy` for in-project relative
+        // imports.
+        localImportStrategy =
+            new LogicalProjectStrategy(this.reflector, new LogicalFileSystem(this.rootDirs));
+      } else {
+        // Plain relative imports are all that's needed.
+        localImportStrategy = new RelativePathStrategy(this.reflector);
+      }
+
       // The CompilerHost doesn't have fileNameToModuleName, so build an NPM-centric reference
       // resolution strategy.
       this.refEmitter = new ReferenceEmitter([
@@ -528,10 +546,10 @@ export class NgtscProgram implements api.Program {
         // Next, attempt to use an absolute import.
         new AbsoluteModuleStrategy(
             this.tsProgram, checker, this.options, this.host, this.reflector),
-        // Finally, check if the reference is being written into a file within the project's logical
+        // Finally, check if the reference is being written into a file within the project's .ts
-        // file system, and use a relative import if so. If this fails, ReferenceEmitter will throw
+        // sources, and use a relative import if so. If this fails, ReferenceEmitter will throw
         // an error.
-        new LogicalProjectStrategy(this.reflector, new LogicalFileSystem(this.rootDirs)),
+        localImportStrategy,
       ]);
 
       // If an entrypoint is present, then all user imports should be directed through the
@@ -569,10 +587,8 @@ export class NgtscProgram implements api.Program {
     this.metaReader = new CompoundMetadataReader([localMetaReader, dtsReader]);
 
 
-    // If a flat module entrypoint was specified, then track references via a `ReferenceGraph`
+    // If a flat module entrypoint was specified, then track references via a `ReferenceGraph` in
-    // in
+    // order to produce proper diagnostics for incorrectly exported directives/pipes/etc. If there
-    // order to produce proper diagnostics for incorrectly exported directives/pipes/etc. If
-    // there
     // is no flat module entrypoint then don't pay the cost of tracking references.
     let referencesRegistry: ReferencesRegistry;
     if (this.entryPoint !== null) {

packages/compiler-cli/test/ngtsc/monorepo_spec.ts

@@ -0,0 +1,117 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {absoluteFrom} from '../../src/ngtsc/file_system';
+import {runInEachFileSystem} from '../../src/ngtsc/file_system/testing';
+import {loadStandardTestFiles} from '../helpers/src/mock_file_loading';
+
+import {NgtscTestEnvironment} from './env';
+
+const testFiles = loadStandardTestFiles();
+
+runInEachFileSystem(() => {
+  describe('monorepos', () => {
+    let env !: NgtscTestEnvironment;
+
+    beforeEach(() => {
+      env = NgtscTestEnvironment.setup(testFiles, absoluteFrom('/app'));
+      env.tsconfig();
+
+      // env.tsconfig() will write to /app/tsconfig.json, but list it as extending
+      // ./tsconfig-base.json. So that file should exist, and redirect to ../tsconfig-base.json.
+      env.write('/app/tsconfig-base.json', JSON.stringify({'extends': '../tsconfig-base.json'}));
+    });
+
+    it('should compile a project with a reference above the current dir', () => {
+      env.write('/app/index.ts', `
+        import {Component, NgModule} from '@angular/core';
+        import {LibModule} from '../lib/module';
+
+        @Component({
+          selector: 'app-cmp',
+          template: '<lib-cmp></lib-cmp>',
+        })
+        export class AppCmp {}
+
+        @NgModule({
+          declarations: [AppCmp],
+          imports: [LibModule],
+        })
+        export class AppModule {}
+      `);
+      env.write('/lib/module.ts', `
+        import {NgModule} from '@angular/core';
+        import {LibCmp} from './cmp';
+
+        @NgModule({
+          declarations: [LibCmp],
+          exports: [LibCmp],
+        })
+        export class LibModule {}
+      `);
+      env.write('/lib/cmp.ts', `
+        import {Component} from '@angular/core';
+
+        @Component({
+          selector: 'lib-cmp',
+          template: '...',
+        })
+        export class LibCmp {}
+      `);
+
+      env.driveMain();
+
+      const jsContents = env.getContents('app/index.js');
+      expect(jsContents).toContain(`import * as i1 from "../lib/cmp";`);
+    });
+
+    it('should compile a project with a reference into the same dir', () => {
+      env.write('/app/index.ts', `
+        import {Component, NgModule} from '@angular/core';
+        import {TargetModule} from './target';
+
+        @Component({
+          selector: 'app-cmp',
+          template: '<target-cmp></target-cmp>',
+        })
+        export class AppCmp {}
+
+        @NgModule({
+          declarations: [AppCmp],
+          imports: [TargetModule],
+        })
+        export class AppModule {}
+      `);
+
+      env.write('/app/target.ts', `
+        import {Component, NgModule} from '@angular/core';
+
+        @Component({
+          selector: 'target-cmp',
+          template: '...',
+        })
+        export class TargetCmp {}
+
+        @NgModule({
+          declarations: [TargetCmp],
+          exports: [TargetCmp],
+        })
+        export class TargetModule {}
+      `);
+
+      env.driveMain();
+
+      // Look for index.js, not app/index.js, because of TypeScript's behavior of stripping off the
+      // common prefix of all input files.
+      const jsContents = env.getContents('index.js');
+
+      // The real goal of this test was to check that the relative import has the leading './'.
+      expect(jsContents).toContain(`import * as i1 from "./target";`);
+    });
+  });
+});