Merge pull request #2046 from micalevisk/file-system-reader-refactor

refactor: use native fs promise-based API instead of promisify & fix formatting

actions/add.action.ts

@@ -3,20 +3,20 @@ 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';
 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),
+      );
     }
   }
 

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';

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 = () => {

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<Schematic[]> {
-    const abstractCollection: AbstractCollection = CollectionFactory.create(
-      collection,
-    );
+    const abstractCollection: AbstractCollection =
+      CollectionFactory.create(collection);
     return abstractCollection.getSchematics();
   }
 }

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)

lib/compiler/helpers/get-value-or-default.ts

@@ -5,7 +5,13 @@ export function getValueOrDefault<T = any>(
   configuration: Required<Configuration>,
   propertyPath: string,
   appName: string,
-  key?: 'path' | 'webpack' | 'webpackPath' | 'entryFile' | 'sourceRoot' | 'exec',
+  key?:
+    | 'path'
+    | 'webpack'
+    | 'webpackPath'
+    | 'entryFile'
+    | 'sourceRoot'
+    | 'exec',
   options: Input[] = [],
   defaultValue?: T,
 ): T {

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,24 @@ export class PackageManagerFactory {
   }
 
   public static async find(): Promise<AbstractPackageManager> {
-    return new Promise<AbstractPackageManager>((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);
+    }
   }
 }

lib/readers/file-system.reader.ts

@@ -1,37 +1,16 @@
 import * as fs from 'fs';
+import * as path from 'path';
 import { Reader } from './reader';
 
 export class FileSystemReader implements Reader {
   constructor(private readonly directory: string) {}
 
-  public async list(): Promise<string[]> {
-    return new Promise<string[]>((resolve, reject) => {
-      fs.readdir(
-        this.directory,
-        (error: NodeJS.ErrnoException | null, filenames: string[]) => {
-          if (error) {
-            reject(error);
-          } else {
-            resolve(filenames);
-          }
-        },
-      );
-    });
+  public list(): Promise<string[]> {
+    return fs.promises.readdir(this.directory);
   }
 
-  public async read(name: string): Promise<string> {
-    return new Promise<string>((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<string> {
+    return fs.promises.readFile(path.join(this.directory, name), 'utf8');
   }
 
   public async readAnyOf(filenames: string[]): Promise<string | undefined> {

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);

lib/utils/project-utils.ts

@@ -69,19 +69,19 @@ export function shouldGenerateSpec(
 }
 
 export function shouldGenerateFlat(
-    configuration: Required<Configuration>,
-    appName: string,
-    flatValue: boolean,
+  configuration: Required<Configuration>,
+  appName: string,
+  flatValue: boolean,
 ): boolean {
   // CLI parameters have the highest priority
   if (flatValue === true) {
     return flatValue;
   }
 
   const flatConfiguration = getValueOrDefault(
-      configuration,
-      'generateOptions.flat',
-      appName || '',
+    configuration,
+    'generateOptions.flat',
+    appName || '',
   );
   if (typeof flatConfiguration === 'boolean') {
     return flatConfiguration;
@@ -90,30 +90,29 @@ export function shouldGenerateFlat(
 }
 
 export function getSpecFileSuffix(
-    configuration: Required<Configuration>,
-    appName: string,
-    specFileSuffixValue: string,
+  configuration: Required<Configuration>,
+  appName: string,
+  specFileSuffixValue: string,
 ): string {
   // CLI parameters have the highest priority
   if (specFileSuffixValue) {
     return specFileSuffixValue;
   }
 
   const specFileSuffixConfiguration = getValueOrDefault(
-      configuration,
-      'generateOptions.specFileSuffix',
-      appName || '',
-      undefined,
-      undefined,
-      'spec',
+    configuration,
+    'generateOptions.specFileSuffix',
+    appName || '',
+    undefined,
+    undefined,
+    'spec',
   );
   if (typeof specFileSuffixConfiguration === 'string') {
     return specFileSuffixConfiguration;
   }
   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,
   );
-}
+}

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,
+        );
+      });
+    });
+  });
+});

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', () => {