From 3dfc7703c2c98145c72ce5ffb8314f3f04d544c6 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Fri, 8 May 2020 15:18:19 +0100 Subject: [PATCH] fix(compiler-cli): ensure LogicalFileSystem maintains case in paths (#37008) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #36992, #36993, #37000 PR Close #37008 --- .../src/ngtsc/file_system/src/logical.ts | 23 ++++++++++++------ .../ngtsc/file_system/test/logical_spec.ts | 24 +++++++++++++++++++ 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/file_system/src/logical.ts b/packages/compiler-cli/src/ngtsc/file_system/src/logical.ts index 2c1be6ba8b..ebaaec59a5 100644 --- a/packages/compiler-cli/src/ngtsc/file_system/src/logical.ts +++ b/packages/compiler-cli/src/ngtsc/file_system/src/logical.ts @@ -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. @@ -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); } /** @@ -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; diff --git a/packages/compiler-cli/src/ngtsc/file_system/test/logical_spec.ts b/packages/compiler-cli/src/ngtsc/file_system/test/logical_spec.ts index 1bf4fc26b2..20fce32a16 100644 --- a/packages/compiler-cli/src/ngtsc/file_system/test/logical_spec.ts +++ b/packages/compiler-cli/src/ngtsc/file_system/test/logical_spec.ts @@ -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', () => { @@ -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'); + }); }); }); });