From 19606fdea2a0c53e1dc5d80ae89fe63e7195d440 Mon Sep 17 00:00:00 2001 From: "Micael Levi L. Cavalcante" Date: Sat, 22 Apr 2023 18:56:41 -0400 Subject: [PATCH 1/5] refactor(lib): use fs promises node api instead of promisify it --- actions/new.action.ts | 3 +-- lib/readers/file-system.reader.ts | 30 +++------------------ test/lib/readers/file-system.reader.spec.ts | 26 +++++++++--------- 3 files changed, 19 insertions(+), 40 deletions(-) diff --git a/actions/new.action.ts b/actions/new.action.ts index 231733ebd..99c3fef50 100644 --- a/actions/new.action.ts +++ b/actions/new.action.ts @@ -4,7 +4,6 @@ import * as fs from 'fs'; import * as inquirer from 'inquirer'; import { Answers, Question } from 'inquirer'; import { join } from 'path'; -import { promisify } from 'util'; import { Input } from '../commands'; import { defaultGitIgnore } from '../lib/configuration/defaults'; import { @@ -199,7 +198,7 @@ const createGitIgnoreFile = (dir: string, content?: string) => { if (fileExists(filePath)) { return; } - return promisify(fs.writeFile)(filePath, fileContent); + return fs.promises.writeFile(filePath, fileContent); }; const printCollective = () => { diff --git a/lib/readers/file-system.reader.ts b/lib/readers/file-system.reader.ts index ce9c65852..9d3da0bc0 100644 --- a/lib/readers/file-system.reader.ts +++ b/lib/readers/file-system.reader.ts @@ -4,34 +4,12 @@ import { Reader } from './reader'; export class FileSystemReader implements Reader { constructor(private readonly directory: string) {} - public async list(): Promise { - return new Promise((resolve, reject) => { - fs.readdir( - this.directory, - (error: NodeJS.ErrnoException | null, filenames: string[]) => { - if (error) { - reject(error); - } else { - resolve(filenames); - } - }, - ); - }); + public list(): Promise { + return fs.promises.readdir(this.directory); } - public async read(name: string): Promise { - return new Promise((resolve, reject) => { - fs.readFile( - `${this.directory}/${name}`, - (error: NodeJS.ErrnoException | null, data: Buffer) => { - if (error) { - reject(error); - } else { - resolve(data.toString()); - } - }, - ); - }); + public read(name: string): Promise { + return fs.promises.readFile(`${this.directory}/${name}`, 'utf8'); } public async readAnyOf(filenames: string[]): Promise { diff --git a/test/lib/readers/file-system.reader.spec.ts b/test/lib/readers/file-system.reader.spec.ts index 06abee2fb..a98b4c8eb 100644 --- a/test/lib/readers/file-system.reader.spec.ts +++ b/test/lib/readers/file-system.reader.spec.ts @@ -1,12 +1,14 @@ import * as fs from 'fs'; import { FileSystemReader, Reader } from '../../../lib/readers'; -jest.mock('fs', () => { - return { - readdir: jest.fn((dir, callback) => callback(null, [])), - readFile: jest.fn((filename, callback) => callback(null, 'content')), - }; -}); +jest.mock('fs', () => + ({ + promises: { + readdir: jest.fn().mockResolvedValue([]), + readFile: jest.fn().mockResolvedValue('content'), + }, + }) +); const dir: string = process.cwd(); const reader: Reader = new FileSystemReader(dir); @@ -15,13 +17,13 @@ describe('File System Reader', () => { afterAll(() => { jest.clearAllMocks(); }); - it('should use fs.readdir when list', async () => { + it('should use fs.promises.readdir when list', async () => { await reader.list(); - expect(fs.readdir).toHaveBeenCalled(); + expect(fs.promises.readdir).toHaveBeenCalled(); }); - it('should use fs.readFile when read', async () => { + it('should use fs.promises.readFile when read', async () => { await reader.read('filename'); - expect(fs.readFile).toHaveBeenCalled(); + expect(fs.promises.readFile).toHaveBeenCalled(); }); describe('readAnyOf tests', () => { @@ -29,10 +31,10 @@ describe('File System Reader', () => { const filenames: string[] = ['file1', 'file2', 'file3']; await reader.readAnyOf(filenames); - expect(fs.readFile).toHaveBeenCalled(); + expect(fs.promises.readFile).toHaveBeenCalled(); }); - it('should return null when no file is passed', async () => { + it('should return undefined when no file is passed', async () => { const content = await reader.readAnyOf([]); expect(content).toEqual(undefined); }); From dd5962e635aedb040a6eb3ec0578d135cb5ce421 Mon Sep 17 00:00:00 2001 From: "Micael Levi L. Cavalcante" Date: Sat, 22 Apr 2023 18:57:24 -0400 Subject: [PATCH 2/5] refactor(lib): use canonical join instead of literal slash symbol --- lib/readers/file-system.reader.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/readers/file-system.reader.ts b/lib/readers/file-system.reader.ts index 9d3da0bc0..b6030b17b 100644 --- a/lib/readers/file-system.reader.ts +++ b/lib/readers/file-system.reader.ts @@ -1,4 +1,5 @@ import * as fs from 'fs'; +import * as path from 'path'; import { Reader } from './reader'; export class FileSystemReader implements Reader { @@ -9,7 +10,7 @@ export class FileSystemReader implements Reader { } public read(name: string): Promise { - return fs.promises.readFile(`${this.directory}/${name}`, 'utf8'); + return fs.promises.readFile(path.join(this.directory, name), 'utf8'); } public async readAnyOf(filenames: string[]): Promise { From ea9cdf5d8fe3660e398787a620e6d071082c6828 Mon Sep 17 00:00:00 2001 From: "Micael Levi L. Cavalcante" Date: Sun, 23 Apr 2023 18:29:43 -0400 Subject: [PATCH 3/5] refactor(lib): use promise version of `fs.readdir` --- .../package-manager.factory.ts | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/lib/package-managers/package-manager.factory.ts b/lib/package-managers/package-manager.factory.ts index 510a15ad8..3e431d2c5 100644 --- a/lib/package-managers/package-manager.factory.ts +++ b/lib/package-managers/package-manager.factory.ts @@ -1,4 +1,4 @@ -import { readdir } from 'fs'; +import * as fs from 'fs'; import { AbstractPackageManager } from './abstract.package-manager'; import { NpmPackageManager } from './npm.package-manager'; import { PackageManager } from './package-manager'; @@ -20,22 +20,20 @@ export class PackageManagerFactory { } public static async find(): Promise { - return new Promise((resolve) => { - readdir(process.cwd(), (error, files) => { - if (error) { - resolve(this.create(PackageManager.NPM)); - } else { - if (files.findIndex((filename) => filename === 'yarn.lock') > -1) { - resolve(this.create(PackageManager.YARN)); - } else if ( - files.findIndex((filename) => filename === 'pnpm-lock.yaml') > -1 - ) { - resolve(this.create(PackageManager.PNPM)); - } else { - resolve(this.create(PackageManager.NPM)); - } - } - }); - }); + const DEFAULT_PACKAGE_MANAGER = PackageManager.NPM; + + try { + const files = await fs.promises.readdir(process.cwd()); + + const hasYarnLockFile = files.includes('yarn.lock'); + if (hasYarnLockFile) return this.create(PackageManager.YARN); + + const hasPnpmLockFile = files.includes('pnpm-lock.yaml'); + if (hasPnpmLockFile) return this.create(PackageManager.PNPM); + + return this.create(DEFAULT_PACKAGE_MANAGER); + } catch (error) { + return this.create(DEFAULT_PACKAGE_MANAGER); + } } } From f37bde8938783558a503dc1facbe8664b200eea8 Mon Sep 17 00:00:00 2001 From: "Micael Levi L. Cavalcante" Date: Sun, 23 Apr 2023 18:41:08 -0400 Subject: [PATCH 4/5] test(lib): add unit tests for `PackageManagerFactory.prototype.find` --- .../package-manager.factory.spec.ts | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 test/lib/package-managers/package-manager.factory.spec.ts diff --git a/test/lib/package-managers/package-manager.factory.spec.ts b/test/lib/package-managers/package-manager.factory.spec.ts new file mode 100644 index 000000000..01748ddb3 --- /dev/null +++ b/test/lib/package-managers/package-manager.factory.spec.ts @@ -0,0 +1,64 @@ +import * as fs from 'fs'; +import { + NpmPackageManager, + PackageManagerFactory, + PnpmPackageManager, + YarnPackageManager, +} from '../../../lib/package-managers'; + +jest.mock('fs', () => ({ + promises: { + readdir: jest.fn(), + }, +})); + +describe('PackageManagerFactory', () => { + afterAll(() => { + jest.clearAllMocks(); + }); + + describe('.prototype.find()', () => { + it('should return NpmPackageManager when no lock file is found', async () => { + (fs.promises.readdir as jest.Mock).mockResolvedValue([]); + + const whenPackageManager = PackageManagerFactory.find(); + await expect(whenPackageManager).resolves.toBeInstanceOf( + NpmPackageManager, + ); + }); + + it('should return YarnPackageManager when "yarn.lock" file is found', async () => { + (fs.promises.readdir as jest.Mock).mockResolvedValue(['yarn.lock']); + + const whenPackageManager = PackageManagerFactory.find(); + await expect(whenPackageManager).resolves.toBeInstanceOf( + YarnPackageManager, + ); + }); + + it('should return PnpmPackageManager when "pnpm-lock.yaml" file is found', async () => { + (fs.promises.readdir as jest.Mock).mockResolvedValue(['pnpm-lock.yaml']); + + const whenPackageManager = PackageManagerFactory.find(); + await expect(whenPackageManager).resolves.toBeInstanceOf( + PnpmPackageManager, + ); + }); + + describe('when there are all supported lock files', () => { + it('should prioritize "yarn.lock" file over all the others lock files', async () => { + (fs.promises.readdir as jest.Mock).mockResolvedValue([ + 'pnpm-lock.yaml', + 'package-lock.json', + // This is intentionally the last element in this array + 'yarn.lock', + ]); + + const whenPackageManager = PackageManagerFactory.find(); + await expect(whenPackageManager).resolves.toBeInstanceOf( + YarnPackageManager, + ); + }); + }); + }); +}); From c14015942f573c99a523dc70836c5b3dc04f7417 Mon Sep 17 00:00:00 2001 From: "Micael Levi L. Cavalcante" Date: Sun, 23 Apr 2023 18:42:48 -0400 Subject: [PATCH 5/5] style: format everything --- actions/add.action.ts | 16 +++---- actions/build.action.ts | 2 +- commands/generate.command.ts | 9 ++-- commands/new.command.ts | 2 +- lib/compiler/helpers/get-value-or-default.ts | 8 +++- .../package-manager.factory.ts | 8 +++- lib/schematics/collection.factory.ts | 4 +- lib/utils/project-utils.ts | 42 ++++++++++--------- .../pnpm.package-manager.spec.ts | 6 ++- test/lib/readers/file-system.reader.spec.ts | 14 +++---- 10 files changed, 63 insertions(+), 48 deletions(-) diff --git a/actions/add.action.ts b/actions/add.action.ts index df906a32c..5c5d1d09e 100644 --- a/actions/add.action.ts +++ b/actions/add.action.ts @@ -3,12 +3,12 @@ import { Input } from '../commands'; import { getValueOrDefault } from '../lib/compiler/helpers/get-value-or-default'; import { AbstractPackageManager, - PackageManagerFactory + PackageManagerFactory, } from '../lib/package-managers'; import { AbstractCollection, CollectionFactory, - SchematicOption + SchematicOption, } from '../lib/schematics'; import { MESSAGES } from '../lib/ui'; import { loadConfiguration } from '../lib/utils/load-configuration'; @@ -16,7 +16,7 @@ import { askForProjectName, hasValidOptionFlag, moveDefaultProjectToStart, - shouldAskForProject + shouldAskForProject, } from '../lib/utils/project-utils'; import { AbstractAction } from './abstract.action'; @@ -29,10 +29,8 @@ export class AddAction extends AbstractAction { const collectionName = this.getCollectionName(libraryName, packageName); const tagName = this.getTagName(packageName); const skipInstall = hasValidOptionFlag('skip-install', options); - const packageInstallSuccess = skipInstall || await this.installPackage( - collectionName, - tagName, - ); + const packageInstallSuccess = + skipInstall || (await this.installPackage(collectionName, tagName)); if (packageInstallSuccess) { const sourceRootOption: Input = await this.getSourceRoot( inputs.concat(options), @@ -46,7 +44,9 @@ export class AddAction extends AbstractAction { MESSAGES.LIBRARY_INSTALLATION_FAILED_BAD_PACKAGE(libraryName), ), ); - throw new Error(MESSAGES.LIBRARY_INSTALLATION_FAILED_BAD_PACKAGE(libraryName)); + throw new Error( + MESSAGES.LIBRARY_INSTALLATION_FAILED_BAD_PACKAGE(libraryName), + ); } } diff --git a/actions/build.action.ts b/actions/build.action.ts index 7553afc0e..98d7a961e 100644 --- a/actions/build.action.ts +++ b/actions/build.action.ts @@ -13,7 +13,7 @@ import { WebpackCompiler } from '../lib/compiler/webpack-compiler'; import { WorkspaceUtils } from '../lib/compiler/workspace-utils'; import { ConfigurationLoader, - NestConfigurationLoader + NestConfigurationLoader, } from '../lib/configuration'; import { defaultOutDir } from '../lib/configuration/defaults'; import { FileSystemReader } from '../lib/readers'; diff --git a/commands/generate.command.ts b/commands/generate.command.ts index c66544717..d6613afe5 100644 --- a/commands/generate.command.ts +++ b/commands/generate.command.ts @@ -107,9 +107,7 @@ export class GenerateCommand extends AbstractCommand { const collection = await this.getCollection(); return ( 'Generate a Nest element.\n' + - ` Schematics available on ${chalk.bold( - collection, - )} collection:\n` + + ` Schematics available on ${chalk.bold(collection)} collection:\n` + this.buildSchematicsListAsTable(await this.getSchematics(collection)) ); } @@ -145,9 +143,8 @@ export class GenerateCommand extends AbstractCommand { } private async getSchematics(collection: string): Promise { - const abstractCollection: AbstractCollection = CollectionFactory.create( - collection, - ); + const abstractCollection: AbstractCollection = + CollectionFactory.create(collection); return abstractCollection.getSchematics(); } } diff --git a/commands/new.command.ts b/commands/new.command.ts index 7cbfb2ea7..28b595e1e 100644 --- a/commands/new.command.ts +++ b/commands/new.command.ts @@ -13,7 +13,7 @@ export class NewCommand extends AbstractCommand { .option( '-d, --dry-run', 'Report actions that would be performed without writing out results.', - false + false, ) .option('-g, --skip-git', 'Skip git repository initialization.', false) .option('-s, --skip-install', 'Skip package installation.', false) diff --git a/lib/compiler/helpers/get-value-or-default.ts b/lib/compiler/helpers/get-value-or-default.ts index a2fe47b33..9ec857d6e 100644 --- a/lib/compiler/helpers/get-value-or-default.ts +++ b/lib/compiler/helpers/get-value-or-default.ts @@ -5,7 +5,13 @@ export function getValueOrDefault( configuration: Required, propertyPath: string, appName: string, - key?: 'path' | 'webpack' | 'webpackPath' | 'entryFile' | 'sourceRoot' | 'exec', + key?: + | 'path' + | 'webpack' + | 'webpackPath' + | 'entryFile' + | 'sourceRoot' + | 'exec', options: Input[] = [], defaultValue?: T, ): T { diff --git a/lib/package-managers/package-manager.factory.ts b/lib/package-managers/package-manager.factory.ts index 3e431d2c5..ea3dd7561 100644 --- a/lib/package-managers/package-manager.factory.ts +++ b/lib/package-managers/package-manager.factory.ts @@ -26,10 +26,14 @@ export class PackageManagerFactory { const files = await fs.promises.readdir(process.cwd()); const hasYarnLockFile = files.includes('yarn.lock'); - if (hasYarnLockFile) return this.create(PackageManager.YARN); + if (hasYarnLockFile) { + return this.create(PackageManager.YARN); + } const hasPnpmLockFile = files.includes('pnpm-lock.yaml'); - if (hasPnpmLockFile) return this.create(PackageManager.PNPM); + if (hasPnpmLockFile) { + return this.create(PackageManager.PNPM); + } return this.create(DEFAULT_PACKAGE_MANAGER); } catch (error) { diff --git a/lib/schematics/collection.factory.ts b/lib/schematics/collection.factory.ts index c203da440..b942f947a 100644 --- a/lib/schematics/collection.factory.ts +++ b/lib/schematics/collection.factory.ts @@ -7,7 +7,9 @@ import { NestCollection } from './nest.collection'; export class CollectionFactory { public static create(collection: Collection | string): AbstractCollection { - const schematicRunner = RunnerFactory.create(Runner.SCHEMATIC) as SchematicRunner; + const schematicRunner = RunnerFactory.create( + Runner.SCHEMATIC, + ) as SchematicRunner; if (collection === Collection.NESTJS) { return new NestCollection(schematicRunner); diff --git a/lib/utils/project-utils.ts b/lib/utils/project-utils.ts index c7b50fe0c..5c4c6fbf9 100644 --- a/lib/utils/project-utils.ts +++ b/lib/utils/project-utils.ts @@ -69,9 +69,9 @@ export function shouldGenerateSpec( } export function shouldGenerateFlat( - configuration: Required, - appName: string, - flatValue: boolean, + configuration: Required, + appName: string, + flatValue: boolean, ): boolean { // CLI parameters have the highest priority if (flatValue === true) { @@ -79,9 +79,9 @@ export function shouldGenerateFlat( } const flatConfiguration = getValueOrDefault( - configuration, - 'generateOptions.flat', - appName || '', + configuration, + 'generateOptions.flat', + appName || '', ); if (typeof flatConfiguration === 'boolean') { return flatConfiguration; @@ -90,9 +90,9 @@ export function shouldGenerateFlat( } export function getSpecFileSuffix( - configuration: Required, - appName: string, - specFileSuffixValue: string, + configuration: Required, + appName: string, + specFileSuffixValue: string, ): string { // CLI parameters have the highest priority if (specFileSuffixValue) { @@ -100,12 +100,12 @@ export function getSpecFileSuffix( } const specFileSuffixConfiguration = getValueOrDefault( - configuration, - 'generateOptions.specFileSuffix', - appName || '', - undefined, - undefined, - 'spec', + configuration, + 'generateOptions.specFileSuffix', + appName || '', + undefined, + undefined, + 'spec', ); if (typeof specFileSuffixConfiguration === 'string') { return specFileSuffixConfiguration; @@ -113,7 +113,6 @@ export function getSpecFileSuffix( return specFileSuffixValue; } - export async function askForProjectName( promptQuestion: string, projects: string[], @@ -141,8 +140,13 @@ export function moveDefaultProjectToStart( return projects; } -export function hasValidOptionFlag(queriedOptionName: string, options: Input[], queriedValue: string|number|boolean = true): boolean { +export function hasValidOptionFlag( + queriedOptionName: string, + options: Input[], + queriedValue: string | number | boolean = true, +): boolean { return options.some( - (option: Input) => option.name === queriedOptionName && option.value === queriedValue + (option: Input) => + option.name === queriedOptionName && option.value === queriedValue, ); -} \ No newline at end of file +} diff --git a/test/lib/package-managers/pnpm.package-manager.spec.ts b/test/lib/package-managers/pnpm.package-manager.spec.ts index 96a5db5ef..22fe330cf 100644 --- a/test/lib/package-managers/pnpm.package-manager.spec.ts +++ b/test/lib/package-managers/pnpm.package-manager.spec.ts @@ -39,7 +39,11 @@ describe('PnpmPackageManager', () => { const dirName = '/tmp'; const testDir = join(process.cwd(), dirName); packageManager.install(dirName, 'pnpm'); - expect(spy).toBeCalledWith('install --strict-peer-dependencies=false --reporter=silent', true, testDir); + expect(spy).toBeCalledWith( + 'install --strict-peer-dependencies=false --reporter=silent', + true, + testDir, + ); }); }); describe('addProduction', () => { diff --git a/test/lib/readers/file-system.reader.spec.ts b/test/lib/readers/file-system.reader.spec.ts index a98b4c8eb..208a8ad4f 100644 --- a/test/lib/readers/file-system.reader.spec.ts +++ b/test/lib/readers/file-system.reader.spec.ts @@ -1,14 +1,12 @@ import * as fs from 'fs'; import { FileSystemReader, Reader } from '../../../lib/readers'; -jest.mock('fs', () => - ({ - promises: { - readdir: jest.fn().mockResolvedValue([]), - readFile: jest.fn().mockResolvedValue('content'), - }, - }) -); +jest.mock('fs', () => ({ + promises: { + readdir: jest.fn().mockResolvedValue([]), + readFile: jest.fn().mockResolvedValue('content'), + }, +})); const dir: string = process.cwd(); const reader: Reader = new FileSystemReader(dir);