From 2e52fcf1ebbab9ec80978841b3fea6eb7c7e422f Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Fri, 31 Jan 2020 21:07:59 +0000 Subject: [PATCH] refactor(compiler-cli): add `removeDir()` to `FileSystem` (#35079) PR Close #35079 --- packages/compiler-cli/BUILD.bazel | 1 + .../integrationtest/test_helpers.js | 1 + .../src/ngtsc/file_system/BUILD.bazel | 2 ++ .../file_system/src/cached_file_system.ts | 11 +++++++++++ .../file_system/src/invalid_file_system.ts | 1 + .../file_system/src/node_js_file_system.ts | 2 ++ .../src/ngtsc/file_system/src/types.ts | 1 + .../src/ngtsc/file_system/test/BUILD.bazel | 1 + .../test/cached_file_system_spec.ts | 19 +++++++++++++++++++ .../test/node_js_file_system_spec.ts | 9 +++++++++ .../testing/src/mock_file_system.ts | 11 +++++++++++ 11 files changed, 59 insertions(+) diff --git a/packages/compiler-cli/BUILD.bazel b/packages/compiler-cli/BUILD.bazel index 88801e8093..3ad34b06c0 100644 --- a/packages/compiler-cli/BUILD.bazel +++ b/packages/compiler-cli/BUILD.bazel @@ -32,6 +32,7 @@ ts_library( "@npm//@bazel/typescript", "@npm//@types/chokidar", "@npm//@types/node", + "@npm//fs-extra", "@npm//minimist", "@npm//reflect-metadata", "@npm//tsickle", diff --git a/packages/compiler-cli/integrationtest/test_helpers.js b/packages/compiler-cli/integrationtest/test_helpers.js index a161c72957..2ab74b070a 100644 --- a/packages/compiler-cli/integrationtest/test_helpers.js +++ b/packages/compiler-cli/integrationtest/test_helpers.js @@ -47,6 +47,7 @@ const requiredNodeModules = { 'tslib': resolveNpmTreeArtifact('npm/node_modules/tslib'), 'domino': resolveNpmTreeArtifact('npm/node_modules/domino'), 'xhr2': resolveNpmTreeArtifact('npm/node_modules/xhr2'), + 'fs-extra': resolveNpmTreeArtifact('npm/node_modules/fs-extra'), // Fine grained dependencies which are used by the integration test Angular modules, and // need to be symlinked so that they can be resolved by NodeJS or NGC. diff --git a/packages/compiler-cli/src/ngtsc/file_system/BUILD.bazel b/packages/compiler-cli/src/ngtsc/file_system/BUILD.bazel index 206189e03c..00aad64ac8 100644 --- a/packages/compiler-cli/src/ngtsc/file_system/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/file_system/BUILD.bazel @@ -9,7 +9,9 @@ ts_library( ]), deps = [ "//packages:types", + "@npm//@types/fs-extra", "@npm//@types/node", + "@npm//fs-extra", "@npm//typescript", ], ) diff --git a/packages/compiler-cli/src/ngtsc/file_system/src/cached_file_system.ts b/packages/compiler-cli/src/ngtsc/file_system/src/cached_file_system.ts index ce9900194f..e37eea75ec 100644 --- a/packages/compiler-cli/src/ngtsc/file_system/src/cached_file_system.ts +++ b/packages/compiler-cli/src/ngtsc/file_system/src/cached_file_system.ts @@ -88,6 +88,17 @@ export class CachedFileSystem implements FileSystem { } } + removeDeep(path: AbsoluteFsPath): void { + this.delegate.removeDeep(path); + // Clear out all children of this directory from the exists cache. + for (const p of this.existsCache.keys()) { + if (p.startsWith(path)) { + this.existsCache.set(path, false); + } + } + } + + lstat(path: AbsoluteFsPath): FileStats { const stat = this.delegate.lstat(path); // if the `path` does not exist then `lstat` will thrown an error. diff --git a/packages/compiler-cli/src/ngtsc/file_system/src/invalid_file_system.ts b/packages/compiler-cli/src/ngtsc/file_system/src/invalid_file_system.ts index 693f813af4..2fcf235afe 100644 --- a/packages/compiler-cli/src/ngtsc/file_system/src/invalid_file_system.ts +++ b/packages/compiler-cli/src/ngtsc/file_system/src/invalid_file_system.ts @@ -30,6 +30,7 @@ export class InvalidFileSystem implements FileSystem { copyFile(from: AbsoluteFsPath, to: AbsoluteFsPath): void { throw makeError(); } moveFile(from: AbsoluteFsPath, to: AbsoluteFsPath): void { throw makeError(); } ensureDir(path: AbsoluteFsPath): void { throw makeError(); } + removeDeep(path: AbsoluteFsPath): void { throw makeError(); } isCaseSensitive(): boolean { throw makeError(); } resolve(...paths: string[]): AbsoluteFsPath { throw makeError(); } dirname(file: T): T { throw makeError(); } diff --git a/packages/compiler-cli/src/ngtsc/file_system/src/node_js_file_system.ts b/packages/compiler-cli/src/ngtsc/file_system/src/node_js_file_system.ts index 042c827dfd..b373e8e567 100644 --- a/packages/compiler-cli/src/ngtsc/file_system/src/node_js_file_system.ts +++ b/packages/compiler-cli/src/ngtsc/file_system/src/node_js_file_system.ts @@ -7,6 +7,7 @@ */ /// import * as fs from 'fs'; +import * as fsExtra from 'fs-extra'; import * as p from 'path'; import {absoluteFrom, relativeFrom} from './helpers'; import {AbsoluteFsPath, FileStats, FileSystem, PathSegment, PathString} from './types'; @@ -40,6 +41,7 @@ export class NodeJSFileSystem implements FileSystem { this.safeMkdir(parents.pop() !); } } + removeDeep(path: AbsoluteFsPath): void { fsExtra.removeSync(path); } isCaseSensitive(): boolean { if (this._caseSensitive === undefined) { this._caseSensitive = this.exists(togglePathCase(__filename)); diff --git a/packages/compiler-cli/src/ngtsc/file_system/src/types.ts b/packages/compiler-cli/src/ngtsc/file_system/src/types.ts index 28f5defa84..520b41bead 100644 --- a/packages/compiler-cli/src/ngtsc/file_system/src/types.ts +++ b/packages/compiler-cli/src/ngtsc/file_system/src/types.ts @@ -49,6 +49,7 @@ export interface FileSystem { copyFile(from: AbsoluteFsPath, to: AbsoluteFsPath): void; moveFile(from: AbsoluteFsPath, to: AbsoluteFsPath): void; ensureDir(path: AbsoluteFsPath): void; + removeDeep(path: AbsoluteFsPath): void; isCaseSensitive(): boolean; isRoot(path: AbsoluteFsPath): boolean; isRooted(path: string): boolean; diff --git a/packages/compiler-cli/src/ngtsc/file_system/test/BUILD.bazel b/packages/compiler-cli/src/ngtsc/file_system/test/BUILD.bazel index 7ce1121b43..36575562ed 100644 --- a/packages/compiler-cli/src/ngtsc/file_system/test/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/file_system/test/BUILD.bazel @@ -12,6 +12,7 @@ ts_library( "//packages:types", "//packages/compiler-cli/src/ngtsc/file_system", "//packages/compiler-cli/src/ngtsc/file_system/testing", + "@npm//@types/fs-extra", "@npm//typescript", ], ) diff --git a/packages/compiler-cli/src/ngtsc/file_system/test/cached_file_system_spec.ts b/packages/compiler-cli/src/ngtsc/file_system/test/cached_file_system_spec.ts index 354373940d..02dfdf01d3 100644 --- a/packages/compiler-cli/src/ngtsc/file_system/test/cached_file_system_spec.ts +++ b/packages/compiler-cli/src/ngtsc/file_system/test/cached_file_system_spec.ts @@ -272,4 +272,23 @@ describe('CachedFileSystem', () => { expect(existsSpy).not.toHaveBeenCalled(); }); }); + + describe('removeDeep()', () => { + it('should call delegate', () => { + const spy = spyOn(delegate, 'removeDeep'); + fs.removeDeep(abcPath); + expect(spy).toHaveBeenCalledWith(abcPath); + }); + + it('should update the exists cache', () => { + spyOn(delegate, 'removeDeep'); + const existsSpy = spyOn(delegate, 'exists').and.returnValue(true); + expect(fs.exists(abcPath)).toBe(true); + existsSpy.calls.reset(); + + fs.removeDeep(abcPath); + expect(fs.exists(abcPath)).toBeFalsy(); + expect(existsSpy).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/compiler-cli/src/ngtsc/file_system/test/node_js_file_system_spec.ts b/packages/compiler-cli/src/ngtsc/file_system/test/node_js_file_system_spec.ts index d17af51223..a2c0adc236 100644 --- a/packages/compiler-cli/src/ngtsc/file_system/test/node_js_file_system_spec.ts +++ b/packages/compiler-cli/src/ngtsc/file_system/test/node_js_file_system_spec.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ import * as realFs from 'fs'; +import * as fsExtra from 'fs-extra'; import {absoluteFrom, dirname, relativeFrom, setFileSystem} from '../src/helpers'; import {NodeJSFileSystem} from '../src/node_js_file_system'; import {AbsoluteFsPath} from '../src/types'; @@ -148,6 +149,14 @@ describe('NodeJSFileSystem', () => { expect(mkdirCalls).toEqual([xPath, xyPath, xyzPath]); }); + describe('removeDeep()', () => { + it('should delegate to fsExtra.remove()', () => { + const spy = spyOn(fsExtra, 'removeSync'); + fs.removeDeep(abcPath); + expect(spy).toHaveBeenCalledWith(abcPath); + }); + }); + it('should not fail if a directory (that did not exist before) does exist when trying to create it', () => { let abcPathExists = false; diff --git a/packages/compiler-cli/src/ngtsc/file_system/testing/src/mock_file_system.ts b/packages/compiler-cli/src/ngtsc/file_system/testing/src/mock_file_system.ts index 7dcd81249d..cf3c625878 100644 --- a/packages/compiler-cli/src/ngtsc/file_system/testing/src/mock_file_system.ts +++ b/packages/compiler-cli/src/ngtsc/file_system/testing/src/mock_file_system.ts @@ -131,6 +131,17 @@ export abstract class MockFileSystem implements FileSystem { } } + removeDeep(path: AbsoluteFsPath): void { + const [folderPath, basename] = this.splitIntoFolderAndFile(path); + const {entity} = this.findFromPath(folderPath); + if (entity === null || !isFolder(entity)) { + throw new MockFileSystemError( + 'ENOENT', path, + `Unable to remove folder "${path}". The containing folder does not exist.`); + } + delete entity[basename]; + } + isRoot(path: AbsoluteFsPath): boolean { return this.dirname(path) === path; } extname(path: AbsoluteFsPath|PathSegment): string {