From da7466295030d28cb60b46e0c0a8132e4a773292 Mon Sep 17 00:00:00 2001 From: "Micael Levi L. Cavalcante" Date: Sun, 7 Jan 2024 15:58:38 -0400 Subject: [PATCH 1/3] perf: switch to sync version instead from the promise api of 'fs' module --- .../nest-configuration.loader.ts | 6 ++--- lib/readers/file-system.reader.ts | 14 +++++----- lib/readers/reader.ts | 6 ++--- .../nest-configuration.loader.spec.ts | 22 +++++++--------- test/lib/readers/file-system.reader.spec.ts | 26 +++++++++---------- 5 files changed, 34 insertions(+), 40 deletions(-) diff --git a/lib/configuration/nest-configuration.loader.ts b/lib/configuration/nest-configuration.loader.ts index 274ac13d1..3a62ba497 100644 --- a/lib/configuration/nest-configuration.loader.ts +++ b/lib/configuration/nest-configuration.loader.ts @@ -14,7 +14,7 @@ const loadedConfigsCache = new Map>(); export class NestConfigurationLoader implements ConfigurationLoader { constructor(private readonly reader: Reader) {} - public async load(name?: string): Promise> { + public load(name?: string): Required { const cacheEntryKey = `${this.reader.constructor.name}:${name}`; const cachedConfig = loadedConfigsCache.get(cacheEntryKey); if (cachedConfig) { @@ -24,8 +24,8 @@ export class NestConfigurationLoader implements ConfigurationLoader { let loadedConfig: Required | undefined; const content: string | undefined = name - ? await this.reader.read(name) - : await this.reader.readAnyOf([ + ? this.reader.read(name) + : this.reader.readAnyOf([ 'nest-cli.json', '.nestcli.json', '.nest-cli.json', diff --git a/lib/readers/file-system.reader.ts b/lib/readers/file-system.reader.ts index b6030b17b..0cbfb9a56 100644 --- a/lib/readers/file-system.reader.ts +++ b/lib/readers/file-system.reader.ts @@ -5,22 +5,22 @@ import { Reader } from './reader'; export class FileSystemReader implements Reader { constructor(private readonly directory: string) {} - public list(): Promise { - return fs.promises.readdir(this.directory); + public list(): string[] { + return fs.readdirSync(this.directory); } - public read(name: string): Promise { - return fs.promises.readFile(path.join(this.directory, name), 'utf8'); + public read(name: string): string { + return fs.readFileSync(path.join(this.directory, name), 'utf8'); } - public async readAnyOf(filenames: string[]): Promise { + public readAnyOf(filenames: string[]): string | undefined { try { for (const file of filenames) { - return await this.read(file); + return this.read(file); } } catch (err) { return filenames.length > 0 - ? await this.readAnyOf(filenames.slice(1, filenames.length)) + ? this.readAnyOf(filenames.slice(1, filenames.length)) : undefined; } } diff --git a/lib/readers/reader.ts b/lib/readers/reader.ts index 796014707..cf444fa9d 100644 --- a/lib/readers/reader.ts +++ b/lib/readers/reader.ts @@ -1,5 +1,5 @@ export interface Reader { - list(): string[] | Promise; - read(name: string): string | Promise; - readAnyOf(filenames: string[]): string | Promise; + list(): string[]; + read(name: string): string; + readAnyOf(filenames: string[]): string | undefined; } diff --git a/test/lib/configuration/nest-configuration.loader.spec.ts b/test/lib/configuration/nest-configuration.loader.spec.ts index ae0e92841..a986171ef 100644 --- a/test/lib/configuration/nest-configuration.loader.spec.ts +++ b/test/lib/configuration/nest-configuration.loader.spec.ts @@ -9,21 +9,17 @@ describe('Nest Configuration Loader', () => { mock.mockImplementation(() => { return { readAnyOf: jest.fn(() => - Promise.resolve( - JSON.stringify({ - language: 'ts', - collection: '@nestjs/schematics', - }), - ), + JSON.stringify({ + language: 'ts', + collection: '@nestjs/schematics', + }), ), read: jest.fn(() => - Promise.resolve( - JSON.stringify({ - language: 'ts', - collection: '@nestjs/schematics', - entryFile: 'secondary', - }), - ), + JSON.stringify({ + language: 'ts', + collection: '@nestjs/schematics', + entryFile: 'secondary', + }), ), }; }); diff --git a/test/lib/readers/file-system.reader.spec.ts b/test/lib/readers/file-system.reader.spec.ts index 208a8ad4f..1c62e04dd 100644 --- a/test/lib/readers/file-system.reader.spec.ts +++ b/test/lib/readers/file-system.reader.spec.ts @@ -2,10 +2,8 @@ import * as fs from 'fs'; import { FileSystemReader, Reader } from '../../../lib/readers'; jest.mock('fs', () => ({ - promises: { - readdir: jest.fn().mockResolvedValue([]), - readFile: jest.fn().mockResolvedValue('content'), - }, + readdirSync: jest.fn().mockResolvedValue([]), + readFileSync: jest.fn().mockResolvedValue('content'), })); const dir: string = process.cwd(); @@ -15,25 +13,25 @@ describe('File System Reader', () => { afterAll(() => { jest.clearAllMocks(); }); - it('should use fs.promises.readdir when list', async () => { - await reader.list(); - expect(fs.promises.readdir).toHaveBeenCalled(); + it('should use fs.readdirSync when list (for performance reasons)', async () => { + reader.list(); + expect(fs.readdirSync).toHaveBeenCalled(); }); - it('should use fs.promises.readFile when read', async () => { - await reader.read('filename'); - expect(fs.promises.readFile).toHaveBeenCalled(); + it('should use fs.readFileSync when read (for performance reasons)', async () => { + reader.read('filename'); + expect(fs.readFileSync).toHaveBeenCalled(); }); describe('readAnyOf tests', () => { - it('should call readFile when running readAnyOf fn', async () => { + it('should call readFileSync when running readAnyOf fn', async () => { const filenames: string[] = ['file1', 'file2', 'file3']; - await reader.readAnyOf(filenames); + reader.readAnyOf(filenames); - expect(fs.promises.readFile).toHaveBeenCalled(); + expect(fs.readFileSync).toHaveBeenCalled(); }); it('should return undefined when no file is passed', async () => { - const content = await reader.readAnyOf([]); + const content = reader.readAnyOf([]); expect(content).toEqual(undefined); }); }); From 5b747acd02bc961004ad8277625bcf17105b4cea Mon Sep 17 00:00:00 2001 From: "Micael Levi L. Cavalcante" Date: Sun, 14 Jan 2024 12:38:20 -0400 Subject: [PATCH 2/3] fix: crash when some file config was found without permissions --- .../nest-configuration.loader.ts | 14 +++++-- lib/readers/file-system.reader.ts | 41 +++++++++++++++---- lib/readers/reader.ts | 13 +++++- 3 files changed, 55 insertions(+), 13 deletions(-) diff --git a/lib/configuration/nest-configuration.loader.ts b/lib/configuration/nest-configuration.loader.ts index 3a62ba497..e5f6142fb 100644 --- a/lib/configuration/nest-configuration.loader.ts +++ b/lib/configuration/nest-configuration.loader.ts @@ -1,4 +1,4 @@ -import { Reader } from '../readers'; +import { Reader, ReaderFileLackPersmissionsError } from '../readers'; import { Configuration } from './configuration'; import { ConfigurationLoader } from './configuration.loader'; import { defaultConfiguration } from './defaults'; @@ -23,7 +23,7 @@ export class NestConfigurationLoader implements ConfigurationLoader { let loadedConfig: Required | undefined; - const content: string | undefined = name + const contentOrError = name ? this.reader.read(name) : this.reader.readAnyOf([ 'nest-cli.json', @@ -32,8 +32,14 @@ export class NestConfigurationLoader implements ConfigurationLoader { 'nest.json', ]); - if (content) { - const fileConfig = JSON.parse(content); + if (contentOrError) { + const isMissingPersmissionsError = contentOrError instanceof ReaderFileLackPersmissionsError; + if (isMissingPersmissionsError) { + console.error(contentOrError.message); + process.exit(1); + } + + const fileConfig = JSON.parse(contentOrError); if (fileConfig.compilerOptions) { loadedConfig = { ...defaultConfiguration, diff --git a/lib/readers/file-system.reader.ts b/lib/readers/file-system.reader.ts index 0cbfb9a56..afdd26d7a 100644 --- a/lib/readers/file-system.reader.ts +++ b/lib/readers/file-system.reader.ts @@ -1,6 +1,6 @@ import * as fs from 'fs'; import * as path from 'path'; -import { Reader } from './reader'; +import { Reader, ReaderFileLackPersmissionsError } from './reader'; export class FileSystemReader implements Reader { constructor(private readonly directory: string) {} @@ -13,15 +13,40 @@ export class FileSystemReader implements Reader { return fs.readFileSync(path.join(this.directory, name), 'utf8'); } - public readAnyOf(filenames: string[]): string | undefined { - try { - for (const file of filenames) { + public readAnyOf( + filenames: string[], + ): string | undefined | ReaderFileLackPersmissionsError { + let firstFilePathFoundButWithInsufficientPermissions: string | undefined; + + for (let idx=0; idx < filenames.length; idx++) { + const file = filenames[idx]; + + try { return this.read(file); + } catch (readErr) { + if ( + !firstFilePathFoundButWithInsufficientPermissions && + typeof readErr?.code === 'string' + ) { + const isInsufficientPermissionsError = + readErr.code === 'EACCES' || readErr.code === 'EPERM'; + if (isInsufficientPermissionsError) { + firstFilePathFoundButWithInsufficientPermissions = readErr.path; + } + } + + const isLastFileToLookFor = idx === filenames.length - 1; + if (!isLastFileToLookFor) continue; + + if (firstFilePathFoundButWithInsufficientPermissions) { + return new ReaderFileLackPersmissionsError( + firstFilePathFoundButWithInsufficientPermissions, + readErr.code, + ); + } else { + return undefined; + } } - } catch (err) { - return filenames.length > 0 - ? this.readAnyOf(filenames.slice(1, filenames.length)) - : undefined; } } } diff --git a/lib/readers/reader.ts b/lib/readers/reader.ts index cf444fa9d..e4c95becb 100644 --- a/lib/readers/reader.ts +++ b/lib/readers/reader.ts @@ -1,5 +1,16 @@ +export class ReaderFileLackPersmissionsError extends Error { + constructor( + public readonly filePath: string, + public readonly fsErrorCode: string, + ) { + super(`File ${filePath} lacks read permissions!`); + } +} + export interface Reader { list(): string[]; read(name: string): string; - readAnyOf(filenames: string[]): string | undefined; + readAnyOf( + filenames: string[], + ): string | undefined | ReaderFileLackPersmissionsError; } From b44b23a77ebe919166b9c3f478664f310adabbd4 Mon Sep 17 00:00:00 2001 From: "Micael Levi L. Cavalcante" Date: Mon, 15 Jan 2024 08:29:17 -0400 Subject: [PATCH 3/3] style: fix formatting --- lib/compiler/assets-manager.ts | 5 +++-- lib/configuration/nest-configuration.loader.ts | 3 ++- lib/readers/file-system.reader.ts | 10 ++++++---- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/compiler/assets-manager.ts b/lib/compiler/assets-manager.ts index f381f876f..d9defad48 100644 --- a/lib/compiler/assets-manager.ts +++ b/lib/compiler/assets-manager.ts @@ -101,8 +101,9 @@ export class AssetsManager { this.watchers.push(watcher); } else { - const files = sync(item.glob, { ignore: item.exclude }) - .filter((matched) => statSync(matched).isFile()); + const files = sync(item.glob, { ignore: item.exclude }).filter( + (matched) => statSync(matched).isFile(), + ); for (const path of files) { this.actionOnFile({ ...option, path, action: 'change' }); } diff --git a/lib/configuration/nest-configuration.loader.ts b/lib/configuration/nest-configuration.loader.ts index e5f6142fb..939860d61 100644 --- a/lib/configuration/nest-configuration.loader.ts +++ b/lib/configuration/nest-configuration.loader.ts @@ -33,7 +33,8 @@ export class NestConfigurationLoader implements ConfigurationLoader { ]); if (contentOrError) { - const isMissingPersmissionsError = contentOrError instanceof ReaderFileLackPersmissionsError; + const isMissingPersmissionsError = + contentOrError instanceof ReaderFileLackPersmissionsError; if (isMissingPersmissionsError) { console.error(contentOrError.message); process.exit(1); diff --git a/lib/readers/file-system.reader.ts b/lib/readers/file-system.reader.ts index afdd26d7a..9649204ef 100644 --- a/lib/readers/file-system.reader.ts +++ b/lib/readers/file-system.reader.ts @@ -18,8 +18,8 @@ export class FileSystemReader implements Reader { ): string | undefined | ReaderFileLackPersmissionsError { let firstFilePathFoundButWithInsufficientPermissions: string | undefined; - for (let idx=0; idx < filenames.length; idx++) { - const file = filenames[idx]; + for (let id = 0; id < filenames.length; id++) { + const file = filenames[id]; try { return this.read(file); @@ -35,8 +35,10 @@ export class FileSystemReader implements Reader { } } - const isLastFileToLookFor = idx === filenames.length - 1; - if (!isLastFileToLookFor) continue; + const isLastFileToLookFor = id === filenames.length - 1; + if (!isLastFileToLookFor) { + continue; + } if (firstFilePathFoundButWithInsufficientPermissions) { return new ReaderFileLackPersmissionsError(