diff --git a/src/cdk/schematics/ng-update/devkit-file-system.ts b/src/cdk/schematics/ng-update/devkit-file-system.ts index a6646af6667c..926a006905f6 100644 --- a/src/cdk/schematics/ng-update/devkit-file-system.ts +++ b/src/cdk/schematics/ng-update/devkit-file-system.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {basename, dirname, normalize, NormalizedRoot, Path} from '@angular-devkit/core'; +import {normalize, Path} from '@angular-devkit/core'; import {Tree, UpdateRecorder} from '@angular-devkit/schematics'; import {DirectoryEntry, FileSystem} from '../update-tool/file-system'; import * as path from 'path'; @@ -42,10 +42,25 @@ export class DevkitFileSystem extends FileSystem { this._updateRecorderCache.clear(); } - exists(fileOrDirPath: Path) { - // We need to check for both file or directory existence, in order - // to comply with the expectation from the TypeScript compiler. - return this._tree.exists(fileOrDirPath) || this._isExistingDirectory(fileOrDirPath); + fileExists(filePath: Path) { + return this._tree.exists(filePath); + } + + directoryExists(dirPath: Path) { + // The devkit tree does not expose an API for checking whether a given + // directory exists. It throws a specific error though if a directory + // is being read as a file. We use that to check if a directory exists. + try { + this._tree.get(dirPath); + } catch (e) { + // Note: We do not use an `instanceof` check here. It could happen that the devkit version + // used by the CLI is different than the one we end up loading. This can happen depending + // on how Yarn/NPM hoists the NPM packages / whether there are multiple versions installed. + if (e instanceof Error && e.constructor.name === 'PathIsDirectoryException') { + return true; + } + } + return false; } overwrite(filePath: Path, content: string) { @@ -69,23 +84,4 @@ export class DevkitFileSystem extends FileSystem { const {subdirs: directories, subfiles: files} = this._tree.getDir(dirPath); return {directories, files}; } - - private _isExistingDirectory(dirPath: Path) { - if (dirPath === NormalizedRoot) { - return true; - } - - const parent = dirname(dirPath); - const dirName = basename(dirPath); - // TypeScript also checks potential entry points, so e.g. importing - // package.json will result in a lookup of /package.json/package.json - // and /package.json/index.ts. In order to avoid failure, we check if - // the parent is an existing file and return false, if that is the case. - if (this._tree.exists(parent)) { - return false; - } - - const dir = this._tree.getDir(parent); - return dir.subdirs.indexOf(dirName) !== -1; - } } diff --git a/src/cdk/schematics/ng-update/test-cases/misc/module-resolution.spec.ts b/src/cdk/schematics/ng-update/test-cases/misc/module-resolution.spec.ts new file mode 100644 index 000000000000..11f2c1f93b40 --- /dev/null +++ b/src/cdk/schematics/ng-update/test-cases/misc/module-resolution.spec.ts @@ -0,0 +1,24 @@ +import {MIGRATION_PATH} from '../../../paths'; +import {createTestCaseSetup} from '../../../testing'; + +describe('ng update typescript program module resolution', () => { + + // Regression test for: https://github.com/angular/components/issues/22919. + it('should not error if module resolution tries a non-existent path where a path segment ' + + 'matches an existing file', async () => { + const {runFixers, writeFile} = await createTestCaseSetup( + 'migration-v6', MIGRATION_PATH, []); + + writeFile('/node_modules/some-other-module/package.json', `{}`); + writeFile('/node_modules/some-other-module/styles.css', '') + + // We add an import to a non-existent sub-path of `some-other-module/styles`. The TypeScript + // module resolution logic could try various sub-paths. This previously resulted in an error + // as the devkit tree `getDir` logic accidentally walked up the path and threw an error if + // a path segment is an actual file. + writeFile('/projects/cdk-testing/src/main.ts', + `import 'some-other-module/styles.css/non/existent';`); + + await expectAsync(runFixers()).toBeResolved(); + }); +}); diff --git a/src/cdk/schematics/update-tool/component-resource-collector.ts b/src/cdk/schematics/update-tool/component-resource-collector.ts index 8509edfc0808..15ccaa7ae27b 100644 --- a/src/cdk/schematics/update-tool/component-resource-collector.ts +++ b/src/cdk/schematics/update-tool/component-resource-collector.ts @@ -148,7 +148,7 @@ export class ComponentResourceCollector { // In case the template does not exist in the file system, skip this // external template. - if (!this._fileSystem.exists(templatePath)) { + if (!this._fileSystem.fileExists(templatePath)) { return; } diff --git a/src/cdk/schematics/update-tool/file-system.ts b/src/cdk/schematics/update-tool/file-system.ts index 459487f6aca9..511ba24bf92e 100644 --- a/src/cdk/schematics/update-tool/file-system.ts +++ b/src/cdk/schematics/update-tool/file-system.ts @@ -43,8 +43,10 @@ export interface DirectoryEntry { * changes. This is necessary to support virtual file systems as used in the CLI devkit. */ export abstract class FileSystem { - /** Checks whether the given file or directory exists. */ - abstract exists(path: WorkspacePath): boolean; + /** Checks whether the given file exists. */ + abstract fileExists(path: WorkspacePath): boolean; + /** Checks whether the given directory exists. */ + abstract directoryExists(path: WorkspacePath): boolean; /** Gets the contents of the given file. */ abstract read(filePath: WorkspacePath): string|null; /** Reads the given directory to retrieve children. */ diff --git a/src/cdk/schematics/update-tool/utils/virtual-host.ts b/src/cdk/schematics/update-tool/utils/virtual-host.ts index 02598dc18114..1010057dc632 100644 --- a/src/cdk/schematics/update-tool/utils/virtual-host.ts +++ b/src/cdk/schematics/update-tool/utils/virtual-host.ts @@ -40,7 +40,7 @@ export class FileSystemHost implements ts.ParseConfigHost { constructor(private _fileSystem: FileSystem) {} fileExists(path: string): boolean { - return this._fileSystem.exists(this._fileSystem.resolve(path)); + return this._fileSystem.fileExists(this._fileSystem.resolve(path)); } readFile(path: string): string|undefined { @@ -84,7 +84,7 @@ export function createFileSystemCompilerHost( host.readFile = virtualHost.readFile.bind(virtualHost); host.readDirectory = virtualHost.readDirectory.bind(virtualHost); host.fileExists = virtualHost.fileExists.bind(virtualHost); - host.directoryExists = (dirPath) => fileSystem.exists(fileSystem.resolve(dirPath)); + host.directoryExists = (dirPath) => fileSystem.directoryExists(fileSystem.resolve(dirPath)); host.getCurrentDirectory = () => '/'; host.getCanonicalFileName = p => fileSystem.resolve(p); diff --git a/src/material/schematics/ng-update/migrations/hammer-gestures-v9/hammer-gestures-migration.ts b/src/material/schematics/ng-update/migrations/hammer-gestures-v9/hammer-gestures-migration.ts index ea11a9cf90f4..1ebbff85d435 100644 --- a/src/material/schematics/ng-update/migrations/hammer-gestures-v9/hammer-gestures-migration.ts +++ b/src/material/schematics/ng-update/migrations/hammer-gestures-v9/hammer-gestures-migration.ts @@ -483,13 +483,13 @@ export class HammerGesturesMigration extends DevkitMigration { * be stored in the specified file path. */ private _getAvailableGestureConfigFileName(sourceRoot: Path) { - if (!this.fileSystem.exists(join(sourceRoot, `${GESTURE_CONFIG_FILE_NAME}.ts`))) { + if (!this.fileSystem.fileExists(join(sourceRoot, `${GESTURE_CONFIG_FILE_NAME}.ts`))) { return `${GESTURE_CONFIG_FILE_NAME}.ts`; } let possibleName = `${GESTURE_CONFIG_FILE_NAME}-`; let index = 1; - while (this.fileSystem.exists(join(sourceRoot, `${possibleName}-${index}.ts`))) { + while (this.fileSystem.fileExists(join(sourceRoot, `${possibleName}-${index}.ts`))) { index++; } return `${possibleName + index}.ts`; @@ -630,7 +630,7 @@ export class HammerGesturesMigration extends DevkitMigration { private _removeHammerFromIndexFile() { const indexFilePaths = getProjectIndexFiles(this.context.project); indexFilePaths.forEach(filePath => { - if (!this.fileSystem.exists(filePath)) { + if (!this.fileSystem.fileExists(filePath)) { return; }