Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
May 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions actions/add.action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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),
Expand All @@ -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),
);
}
}

Expand Down
2 changes: 1 addition & 1 deletion actions/build.action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
3 changes: 1 addition & 2 deletions actions/new.action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 = () => {
Expand Down
9 changes: 3 additions & 6 deletions commands/generate.command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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))
);
}
Expand Down Expand Up @@ -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();
}
}
2 changes: 1 addition & 1 deletion commands/new.command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 7 additions & 1 deletion lib/compiler/helpers/get-value-or-default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
38 changes: 20 additions & 18 deletions lib/package-managers/package-manager.factory.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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);
}
}
}
31 changes: 5 additions & 26 deletions lib/readers/file-system.reader.ts
Original file line number Diff line number Diff line change
@@ -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> {
Expand Down
4 changes: 3 additions & 1 deletion lib/schematics/collection.factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
42 changes: 23 additions & 19 deletions lib/utils/project-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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[],
Expand Down Expand Up @@ -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,
);
}
}
64 changes: 64 additions & 0 deletions test/lib/package-managers/package-manager.factory.spec.ts
Original file line number Diff line number Diff line change
@@ -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,
);
});
});
});
});
6 changes: 5 additions & 1 deletion test/lib/package-managers/pnpm.package-manager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Loading