From e27b920ac36ae6d8dd5ec1711b42da00c1ef30c9 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Thu, 31 Dec 2020 11:05:16 +0000 Subject: [PATCH] refactor(compiler-cli): split up `NodeJSFileSystem` class (#40281) This class is refactored to extend the new `NodeJSReadonlyFileSystem` which itself extends `NodeJSPathManipulation`. These new classes allow consumers to create file-systems that provide a subset of the full file-system. PR Close #40281 --- .../file_system/src/node_js_file_system.ts | 142 ++++++++++-------- .../test/node_js_file_system_spec.ts | 136 ++++++++++------- 2 files changed, 154 insertions(+), 124 deletions(-) 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 4440928319..12c91285c8 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 @@ -9,74 +9,18 @@ import * as fs from 'fs'; import * as fsExtra from 'fs-extra'; import * as p from 'path'; -import {absoluteFrom} from './helpers'; -import {AbsoluteFsPath, FileStats, FileSystem, PathSegment, PathString} from './types'; +import {AbsoluteFsPath, FileStats, FileSystem, PathManipulation, PathSegment, PathString, ReadonlyFileSystem} from './types'; /** - * A wrapper around the Node.js file-system (i.e the `fs` package). + * A wrapper around the Node.js file-system that supports path manipulation. */ -export class NodeJSFileSystem implements FileSystem { - private _caseSensitive: boolean|undefined = undefined; - exists(path: AbsoluteFsPath): boolean { - return fs.existsSync(path); - } - readFile(path: AbsoluteFsPath): string { - return fs.readFileSync(path, 'utf8'); - } - readFileBuffer(path: AbsoluteFsPath): Uint8Array { - return fs.readFileSync(path); - } - writeFile(path: AbsoluteFsPath, data: string|Uint8Array, exclusive: boolean = false): void { - fs.writeFileSync(path, data, exclusive ? {flag: 'wx'} : undefined); - } - removeFile(path: AbsoluteFsPath): void { - fs.unlinkSync(path); - } - symlink(target: AbsoluteFsPath, path: AbsoluteFsPath): void { - fs.symlinkSync(target, path); - } - readdir(path: AbsoluteFsPath): PathSegment[] { - return fs.readdirSync(path) as PathSegment[]; - } - lstat(path: AbsoluteFsPath): FileStats { - return fs.lstatSync(path); - } - stat(path: AbsoluteFsPath): FileStats { - return fs.statSync(path); - } +export class NodeJSPathManipulation implements PathManipulation { pwd(): AbsoluteFsPath { return this.normalize(process.cwd()) as AbsoluteFsPath; } chdir(dir: AbsoluteFsPath): void { process.chdir(dir); } - copyFile(from: AbsoluteFsPath, to: AbsoluteFsPath): void { - fs.copyFileSync(from, to); - } - moveFile(from: AbsoluteFsPath, to: AbsoluteFsPath): void { - fs.renameSync(from, to); - } - ensureDir(path: AbsoluteFsPath): void { - const parents: AbsoluteFsPath[] = []; - while (!this.isRoot(path) && !this.exists(path)) { - parents.push(path); - path = this.dirname(path); - } - while (parents.length) { - this.safeMkdir(parents.pop()!); - } - } - removeDeep(path: AbsoluteFsPath): void { - fsExtra.removeSync(path); - } - isCaseSensitive(): boolean { - if (this._caseSensitive === undefined) { - // Note the use of the real file-system is intentional: - // `this.exists()` relies upon `isCaseSensitive()` so that would cause an infinite recursion. - this._caseSensitive = !fs.existsSync(togglePathCase(__filename)); - } - return this._caseSensitive; - } resolve(...paths: string[]): AbsoluteFsPath { return this.normalize(p.resolve(...paths)) as AbsoluteFsPath; } @@ -102,15 +46,82 @@ export class NodeJSFileSystem implements FileSystem { extname(path: AbsoluteFsPath|PathSegment): string { return p.extname(path); } + normalize(path: T): T { + // Convert backslashes to forward slashes + return path.replace(/\\/g, '/') as T; + } +} + +/** + * A wrapper around the Node.js file-system that supports readonly operations and path manipulation. + */ +export class NodeJSReadonlyFileSystem extends NodeJSPathManipulation implements ReadonlyFileSystem { + private _caseSensitive: boolean|undefined = undefined; + isCaseSensitive(): boolean { + if (this._caseSensitive === undefined) { + // Note the use of the real file-system is intentional: + // `this.exists()` relies upon `isCaseSensitive()` so that would cause an infinite recursion. + this._caseSensitive = !fs.existsSync(this.normalize(toggleCase(__filename))); + } + return this._caseSensitive; + } + exists(path: AbsoluteFsPath): boolean { + return fs.existsSync(path); + } + readFile(path: AbsoluteFsPath): string { + return fs.readFileSync(path, 'utf8'); + } + readFileBuffer(path: AbsoluteFsPath): Uint8Array { + return fs.readFileSync(path); + } + readdir(path: AbsoluteFsPath): PathSegment[] { + return fs.readdirSync(path) as PathSegment[]; + } + lstat(path: AbsoluteFsPath): FileStats { + return fs.lstatSync(path); + } + stat(path: AbsoluteFsPath): FileStats { + return fs.statSync(path); + } realpath(path: AbsoluteFsPath): AbsoluteFsPath { return this.resolve(fs.realpathSync(path)); } getDefaultLibLocation(): AbsoluteFsPath { return this.resolve(require.resolve('typescript'), '..'); } - normalize(path: T): T { - // Convert backslashes to forward slashes - return path.replace(/\\/g, '/') as T; +} + +/** + * A wrapper around the Node.js file-system (i.e. the `fs` package). + */ +export class NodeJSFileSystem extends NodeJSReadonlyFileSystem implements FileSystem { + writeFile(path: AbsoluteFsPath, data: string|Uint8Array, exclusive: boolean = false): void { + fs.writeFileSync(path, data, exclusive ? {flag: 'wx'} : undefined); + } + removeFile(path: AbsoluteFsPath): void { + fs.unlinkSync(path); + } + symlink(target: AbsoluteFsPath, path: AbsoluteFsPath): void { + fs.symlinkSync(target, path); + } + copyFile(from: AbsoluteFsPath, to: AbsoluteFsPath): void { + fs.copyFileSync(from, to); + } + moveFile(from: AbsoluteFsPath, to: AbsoluteFsPath): void { + fs.renameSync(from, to); + } + ensureDir(path: AbsoluteFsPath): void { + const parents: AbsoluteFsPath[] = []; + while (!this.isRoot(path) && !this.exists(path)) { + parents.push(path); + path = this.dirname(path); + } + while (parents.length) { + this.safeMkdir(parents.pop()!); + } + } + removeDeep(path: AbsoluteFsPath): void { + fsExtra.removeSync(path); } private safeMkdir(path: AbsoluteFsPath): void { @@ -127,9 +138,8 @@ export class NodeJSFileSystem implements FileSystem { } /** - * Toggle the case of each character in a file path. + * Toggle the case of each character in a string. */ -function togglePathCase(str: string): AbsoluteFsPath { - return absoluteFrom( - str.replace(/\w/g, ch => ch.toUpperCase() === ch ? ch.toLowerCase() : ch.toUpperCase())); +function toggleCase(str: string): string { + return str.replace(/\w/g, ch => ch.toUpperCase() === ch ? ch.toLowerCase() : ch.toUpperCase()); } 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 ea87f7dc62..4cd7cfb60c 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 @@ -8,22 +8,55 @@ import * as realFs from 'fs'; import * as fsExtra from 'fs-extra'; import * as os from 'os'; -import {absoluteFrom, dirname, relativeFrom, setFileSystem} from '../src/helpers'; -import {NodeJSFileSystem} from '../src/node_js_file_system'; -import {AbsoluteFsPath} from '../src/types'; +import {NodeJSFileSystem, NodeJSPathManipulation, NodeJSReadonlyFileSystem} from '../src/node_js_file_system'; +import {AbsoluteFsPath, PathSegment} from '../src/types'; -describe('NodeJSFileSystem', () => { - let fs: NodeJSFileSystem; +describe('NodeJSPathManipulation', () => { + let fs: NodeJSPathManipulation; let abcPath: AbsoluteFsPath; let xyzPath: AbsoluteFsPath; beforeEach(() => { - fs = new NodeJSFileSystem(); - // Set the file-system so that calls like `absoluteFrom()` - // and `relativeFrom()` work correctly. - setFileSystem(fs); - abcPath = absoluteFrom('/a/b/c'); - xyzPath = absoluteFrom('/x/y/z'); + fs = new NodeJSPathManipulation(); + abcPath = fs.resolve('/a/b/c'); + xyzPath = fs.resolve('/x/y/z'); + }); + + describe('pwd()', () => { + it('should delegate to process.cwd()', () => { + const spy = spyOn(process, 'cwd').and.returnValue(abcPath); + const result = fs.pwd(); + expect(result).toEqual(abcPath); + expect(spy).toHaveBeenCalledWith(); + }); + }); + + if (os.platform() === 'win32') { + // Only relevant on Windows + describe('relative', () => { + it('should handle Windows paths on different drives', () => { + expect(fs.relative('C:\\a\\b\\c', 'D:\\a\\b\\d')).toEqual(fs.resolve('D:\\a\\b\\d')); + }); + }); + } +}); + +describe('NodeJSReadonlyFileSystem', () => { + let fs: NodeJSReadonlyFileSystem; + let abcPath: AbsoluteFsPath; + let xyzPath: AbsoluteFsPath; + + beforeEach(() => { + fs = new NodeJSReadonlyFileSystem(); + abcPath = fs.resolve('/a/b/c'); + xyzPath = fs.resolve('/x/y/z'); + }); + + describe('isCaseSensitive()', () => { + it('should return true if the FS is case-sensitive', () => { + const isCaseSensitive = !realFs.existsSync(__filename.toUpperCase()); + expect(fs.isCaseSensitive()).toEqual(isCaseSensitive); + }); }); describe('exists()', () => { @@ -55,30 +88,11 @@ describe('NodeJSFileSystem', () => { }); }); - describe('writeFile()', () => { - it('should delegate to fs.writeFileSync()', () => { - const spy = spyOn(realFs, 'writeFileSync'); - fs.writeFile(abcPath, 'Some contents'); - expect(spy).toHaveBeenCalledWith(abcPath, 'Some contents', undefined); - spy.calls.reset(); - fs.writeFile(abcPath, 'Some contents', /* exclusive */ true); - expect(spy).toHaveBeenCalledWith(abcPath, 'Some contents', {flag: 'wx'}); - }); - }); - - describe('removeFile()', () => { - it('should delegate to fs.unlink()', () => { - const spy = spyOn(realFs, 'unlinkSync'); - fs.removeFile(abcPath); - expect(spy).toHaveBeenCalledWith(abcPath); - }); - }); - describe('readdir()', () => { it('should delegate to fs.readdirSync()', () => { const spy = spyOn(realFs, 'readdirSync').and.returnValue(['x', 'y/z'] as any); const result = fs.readdir(abcPath); - expect(result).toEqual([relativeFrom('x'), relativeFrom('y/z')]); + expect(result).toEqual(['x' as PathSegment, 'y/z' as PathSegment]); // TODO: @JiaLiPassion need to wait for @types/jasmine update to handle optional parameters. // https://github.com/DefinitelyTyped/DefinitelyTyped/issues/43486 expect(spy as any).toHaveBeenCalledWith(abcPath); @@ -106,13 +120,35 @@ describe('NodeJSFileSystem', () => { expect(spy as any).toHaveBeenCalledWith(abcPath); }); }); +}); - describe('pwd()', () => { - it('should delegate to process.cwd()', () => { - const spy = spyOn(process, 'cwd').and.returnValue(abcPath); - const result = fs.pwd(); - expect(result).toEqual(abcPath); - expect(spy).toHaveBeenCalledWith(); +describe('NodeJSFileSystem', () => { + let fs: NodeJSFileSystem; + let abcPath: AbsoluteFsPath; + let xyzPath: AbsoluteFsPath; + + beforeEach(() => { + fs = new NodeJSFileSystem(); + abcPath = fs.resolve('/a/b/c'); + xyzPath = fs.resolve('/x/y/z'); + }); + + describe('writeFile()', () => { + it('should delegate to fs.writeFileSync()', () => { + const spy = spyOn(realFs, 'writeFileSync'); + fs.writeFile(abcPath, 'Some contents'); + expect(spy).toHaveBeenCalledWith(abcPath, 'Some contents', undefined); + spy.calls.reset(); + fs.writeFile(abcPath, 'Some contents', /* exclusive */ true); + expect(spy).toHaveBeenCalledWith(abcPath, 'Some contents', {flag: 'wx'}); + }); + }); + + describe('removeFile()', () => { + it('should delegate to fs.unlink()', () => { + const spy = spyOn(realFs, 'unlinkSync'); + fs.removeFile(abcPath); + expect(spy).toHaveBeenCalledWith(abcPath); }); }); @@ -134,10 +170,10 @@ describe('NodeJSFileSystem', () => { describe('ensureDir()', () => { it('should call exists() and fs.mkdir()', () => { - const aPath = absoluteFrom('/a'); - const abPath = absoluteFrom('/a/b'); - const xPath = absoluteFrom('/x'); - const xyPath = absoluteFrom('/x/y'); + const aPath = fs.resolve('/a'); + const abPath = fs.resolve('/a/b'); + const xPath = fs.resolve('/x'); + const xyPath = fs.resolve('/x/y'); const mkdirCalls: string[] = []; const existsCalls: string[] = []; spyOn(realFs, 'mkdirSync').and.callFake(((path: string) => mkdirCalls.push(path)) as any); @@ -190,7 +226,7 @@ describe('NodeJSFileSystem', () => { fs.ensureDir(abcPath); expect(mkdirSyncSpy).toHaveBeenCalledTimes(3); expect(mkdirSyncSpy).toHaveBeenCalledWith(abcPath); - expect(mkdirSyncSpy).toHaveBeenCalledWith(dirname(abcPath)); + expect(mkdirSyncSpy).toHaveBeenCalledWith(fs.dirname(abcPath)); }); it('should fail if creating the directory throws and the directory does not exist', () => { @@ -239,20 +275,4 @@ describe('NodeJSFileSystem', () => { expect(spy).toHaveBeenCalledWith(abcPath); }); }); - - describe('isCaseSensitive()', () => { - it('should return true if the FS is case-sensitive', () => { - const isCaseSensitive = !realFs.existsSync(__filename.toUpperCase()); - expect(fs.isCaseSensitive()).toEqual(isCaseSensitive); - }); - }); - - if (os.platform() === 'win32') { - // Only relevant on Windows - describe('relative', () => { - it('should handle Windows paths on different drives', () => { - expect(fs.relative('C:\\a\\b\\c', 'D:\\a\\b\\d')).toEqual(absoluteFrom('D:\\a\\b\\d')); - }); - }); - } });