Skip to content

Commit

Permalink
fix(compiler-cli): ensure LogicalFileSystem maintains case in paths
Browse files Browse the repository at this point in the history
The work to support case-sensitivity in the `FileSystem` went too far
with the `LogicalFileSystem`, which is used to compute import paths
that will be added to files processed by ngtsc and ngcc.

Previously all logical paths were canonicalised, which meant that on
case-insensitive file-systems, the paths were all set to lower case.
This resulted in incorrect imports being added to files. For example:

```
import { Apollo } from './Apollo';
import { SelectPipe } from './SelectPipe';
import * as ɵngcc0 from '@angular/core';
import * as ɵngcc1 from './selectpipe';
```

The import from `./SelectPipe` is from the original file, while the
import from `./selectpipe` is added by ngcc. This causes the
TypeScript compiler to complain, or worse for paths not to be
matched correctly.

Now, when computing logical paths, the original absolute paths
are matched against rootDirs in a canonical manner, but the actual
logical path that is returned maintains it original casing.

Fixes angular#36992, angular#36993, angular#37000
  • Loading branch information
petebacondarwin committed May 15, 2020
1 parent 42a9e5a commit 3a49f9c
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 7 deletions.
23 changes: 16 additions & 7 deletions packages/compiler-cli/src/ngtsc/file_system/src/logical.ts
Expand Up @@ -47,6 +47,12 @@ export class LogicalFileSystem {
*/
private rootDirs: AbsoluteFsPath[];

/**
* The same root directories as `rootDirs` but with each one converted to its
* canonical form for matching in case-insensitive file-systems.
*/
private canonicalRootDirs: AbsoluteFsPath[];

/**
* A cache of file paths to project paths, because computation of these paths is slightly
* expensive.
Expand All @@ -56,10 +62,9 @@ export class LogicalFileSystem {
constructor(rootDirs: AbsoluteFsPath[], private compilerHost: ts.CompilerHost) {
// Make a copy and sort it by length in reverse order (longest first). This speeds up lookups,
// since there's no need to keep going through the array once a match is found.
this.rootDirs =
rootDirs.map(dir => this.compilerHost.getCanonicalFileName(dir) as AbsoluteFsPath)
.concat([])
.sort((a, b) => b.length - a.length);
this.rootDirs = rootDirs.concat([]).sort((a, b) => b.length - a.length);
this.canonicalRootDirs =
this.rootDirs.map(dir => this.compilerHost.getCanonicalFileName(dir) as AbsoluteFsPath);
}

/**
Expand All @@ -83,9 +88,13 @@ export class LogicalFileSystem {
this.compilerHost.getCanonicalFileName(physicalFile) as AbsoluteFsPath;
if (!this.cache.has(canonicalFilePath)) {
let logicalFile: LogicalProjectPath|null = null;
for (const rootDir of this.rootDirs) {
if (isWithinBasePath(rootDir, canonicalFilePath)) {
logicalFile = this.createLogicalProjectPath(canonicalFilePath, rootDir);
for (let i = 0; i < this.rootDirs.length; i++) {
const rootDir = this.rootDirs[i];
const canonicalRootDir = this.canonicalRootDirs[i];
if (isWithinBasePath(canonicalRootDir, canonicalFilePath)) {
// Note that we match against canonical paths but then create the logical path from
// original paths.
logicalFile = this.createLogicalProjectPath(physicalFile, rootDir);
// The logical project does not include any special "node_modules" nested directories.
if (logicalFile.indexOf('/node_modules/') !== -1) {
logicalFile = null;
Expand Down
24 changes: 24 additions & 0 deletions packages/compiler-cli/src/ngtsc/file_system/test/logical_spec.ts
Expand Up @@ -52,6 +52,24 @@ runInEachFileSystem(() => {
expect(nonRootFs.logicalPathOfFile(_('/test/foo/foo.ts')))
.toEqual('/foo/foo' as LogicalProjectPath);
});

it('should maintain casing of logical paths', () => {
const fs = new LogicalFileSystem([_('/Test')], host);
expect(fs.logicalPathOfFile(_('/Test/foo/Foo.ts')))
.toEqual('/foo/Foo' as LogicalProjectPath);
expect(fs.logicalPathOfFile(_('/Test/bar/bAR.ts')))
.toEqual('/bar/bAR' as LogicalProjectPath);
});

it('should use case-sensitivity when matching rootDirs', () => {
const fs = new LogicalFileSystem([_('/Test')], host);
if (host.useCaseSensitiveFileNames()) {
expect(fs.logicalPathOfFile(_('/test/car/CAR.ts'))).toBe(null);
} else {
expect(fs.logicalPathOfFile(_('/test/car/CAR.ts')))
.toEqual('/car/CAR' as LogicalProjectPath);
}
});
});

describe('utilities', () => {
Expand All @@ -66,6 +84,12 @@ runInEachFileSystem(() => {
'/foo/index' as LogicalProjectPath, '/bar/index' as LogicalProjectPath);
expect(res).toEqual('../bar/index');
});

it('should maintain casing in relative path between logical files', () => {
const res = LogicalProjectPath.relativePathBetween(
'/fOO' as LogicalProjectPath, '/bAR' as LogicalProjectPath);
expect(res).toEqual('./bAR');
});
});
});
});

0 comments on commit 3a49f9c

Please sign in to comment.