From ff8808a742e4936fe6f4974c151c12b9aa1d6f80 Mon Sep 17 00:00:00 2001 From: Daniel Huber <30466471+daniel0611@users.noreply.github.com> Date: Wed, 5 Oct 2022 11:45:42 +0200 Subject: [PATCH 1/2] feat: Bump minimum node.js version from 12 to 14.14.0 With nodecg-io 0.3.0 node.js 14 or higher is required for development installations. This minimum version is required by the husky dependency but I think some other dependencies also want node.js 14+. I think it is reasonable to bump the minimum supported node.js version to 14 now. Most people should be able to upgrade to node.js 14 without any problems. Updates the check at cli startup to require node.js 14 and also updates node.js version in the GitHub Actions CI configuration. --- .github/workflows/ci.yaml | 6 +++--- src/index.ts | 4 ++-- src/utils/cli.ts | 23 ++++++++++++++--------- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index a3f5539..9dda9e3 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -7,7 +7,7 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest, windows-2019] - node: [12, 14, 16, 17] + node: [14, 16, 18] runs-on: ${{ matrix.os }} steps: @@ -31,7 +31,7 @@ jobs: matrix: os: [ubuntu-latest, windows-2019] version: ['0.1', '0.2', 'development'] - node: [14, 16] + node: [14, 18] runs-on: ${{ matrix.os }} steps: @@ -77,7 +77,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v3 with: - node-version: 16 + node-version: 18 - name: Install dependencies run: npm ci diff --git a/src/index.ts b/src/index.ts index 2119be4..41abe28 100755 --- a/src/index.ts +++ b/src/index.ts @@ -2,7 +2,7 @@ import yargs from "yargs"; import { installModule } from "./install"; import { uninstallModule } from "./uninstall"; import { version } from "../package.json"; -import { checkForCliUpdate, ensureNode12 } from "./utils/cli"; +import { checkForCliUpdate, ensureMinimumNodeVersion } from "./utils/cli"; import { generateModule } from "./generate"; // This file gets imported by the index.js file of the repository root. @@ -22,7 +22,7 @@ const args = yargs(process.argv.slice(2)) }) .parse(); -ensureNode12(); +ensureMinimumNodeVersion(); (async () => { const opts = await args; if (!opts["disable-updates"]) { diff --git a/src/utils/cli.ts b/src/utils/cli.ts index 08b02ae..a3aca62 100644 --- a/src/utils/cli.ts +++ b/src/utils/cli.ts @@ -6,20 +6,25 @@ import * as semver from "semver"; import { SemVer } from "semver"; /** - * Ensures that the cli is executed with at least node 12. This is the minimum version that is required - * and if it is older the cli will exit. + * Minimum node.js version that is required to use nodecg-io and this cli. */ -export function ensureNode12(): void { +const minimumNodeVersion = "14.14.0"; + +/** + * Ensures that the node.js installation that is used to execute the cli + * meets the required minimum node.js version for nodecg-io, + * If it is older the cli will log an error about it and exit. + */ +export function ensureMinimumNodeVersion(): void { const nodeVersion = process.versions.node; - const range = new semver.Range(">=12"); + const range = new semver.Range(`>=${minimumNodeVersion}`); if (!semver.satisfies(nodeVersion, range)) { logger.error("Please update your node installation."); - logger.error( - `nodecg-io-cli requires at least node ${chalk.yellowBright("12")}. You have ${chalk.yellowBright( - nodeVersion, - )}`, - ); + + const minVer = chalk.yellowBright(minimumNodeVersion); + const curVer = chalk.yellowBright(nodeVersion); + logger.error(`nodecg-io-cli requires at least node ${minVer}. You have ${curVer}`); process.exit(1); } } From 8046fe9e1d81000e60be7c3ad973e895d167429f Mon Sep 17 00:00:00 2001 From: Daniel Huber <30466471+daniel0611@users.noreply.github.com> Date: Wed, 5 Oct 2022 13:05:31 +0200 Subject: [PATCH 2/2] refactor(fs): Replace custom directory delete implementation with node fs method With ff8808a742e4936fe6f4974c151c12b9aa1d6f80 we switched the cli to node.js 14.14.0 or higher. With this version the `recursive` option was added to the rm method in the node.js fs module. Now we can use this integrated method as we don't need to support the older node.js versions anymore. --- src/install/development.ts | 6 +++--- src/install/index.ts | 5 +++-- src/uninstall/index.ts | 7 ++++--- src/utils/fs.ts | 30 ------------------------------ src/utils/npm.ts | 4 ++-- test/install/development.ts | 4 ++-- test/install/production.ts | 10 ++++++---- test/uninstall/index.ts | 10 +++++----- test/utils/fs.ts | 20 +------------------- test/utils/npm/pkgManagement.ts | 7 +++++-- 10 files changed, 31 insertions(+), 72 deletions(-) diff --git a/src/install/development.ts b/src/install/development.ts index 95d4ca2..4656f34 100644 --- a/src/install/development.ts +++ b/src/install/development.ts @@ -2,7 +2,7 @@ import * as chalk from "chalk"; import * as git from "isomorphic-git"; import * as fs from "fs"; import * as http from "isomorphic-git/http/node"; -import { directoryExists, removeDirectory } from "../utils/fs"; +import { directoryExists } from "../utils/fs"; import { DevelopmentInstallation, writeInstallInfo } from "../utils/installation"; import { logger } from "../utils/log"; import * as path from "path"; @@ -43,7 +43,7 @@ async function manageDocs(nodecgIODir: string, cloneDocs: boolean): Promise delete logger.debug("Removing docs..."); - await removeDirectory(docsPath); + await fs.promises.rm(docsPath, { recursive: true, force: true }); } } @@ -122,7 +122,7 @@ async function deleteNodeModuleDirectories(nodecgIODir: string): Promise { for (const nodeModuleDir of nodeModuleDirs) { if (await directoryExists(nodeModuleDir)) { - await removeDirectory(nodeModuleDir); + await fs.promises.rm(nodeModuleDir, { recursive: true, force: true }); } } diff --git a/src/install/index.ts b/src/install/index.ts index da72ff0..ca48c18 100644 --- a/src/install/index.ts +++ b/src/install/index.ts @@ -1,6 +1,7 @@ import { CommandModule } from "yargs"; import * as path from "path"; -import { directoryExists, removeDirectory } from "../utils/fs"; +import * as fs from "fs"; +import { directoryExists } from "../utils/fs"; import { createDevInstall } from "./development"; import { manageBundleDir } from "../utils/nodecgConfig"; import { promptForInstallInfo } from "./prompt"; @@ -76,7 +77,7 @@ async function install(opts: InstallCommandOptions): Promise { // If the minor version changed and we already have another one installed, we need to delete it, so it can be properly installed. if (currentInstall && currentInstall.version !== requestedInstall.version && (await directoryExists(nodecgIODir))) { logger.info(`Deleting nodecg-io version ${currentInstall.version}...`); - await removeDirectory(nodecgIODir); + await fs.promises.rm(nodecgIODir, { recursive: true, force: true }); currentInstall = undefined; } diff --git a/src/uninstall/index.ts b/src/uninstall/index.ts index 33d07a0..0e36ce4 100644 --- a/src/uninstall/index.ts +++ b/src/uninstall/index.ts @@ -1,6 +1,7 @@ -import path = require("path"); +import * as path from "path"; +import * as fs from "fs"; import { CommandModule } from "yargs"; -import { directoryExists, removeDirectory } from "../utils/fs"; +import { directoryExists } from "../utils/fs"; import { logger } from "../utils/log"; import { manageBundleDir } from "../utils/nodecgConfig"; import { findNodeCGDirectory, getNodeCGIODirectory } from "../utils/nodecgInstallation"; @@ -36,7 +37,7 @@ export async function uninstall(): Promise { // Delete directory logger.debug(`Uninstalling nodecg-io from nodecg installation at ${nodecgDir}...`); - await removeDirectory(nodecgIODir); + await fs.promises.rm(nodecgIODir, { recursive: true, force: true }); logger.success("Successfully uninstalled nodecg-io."); } diff --git a/src/utils/fs.ts b/src/utils/fs.ts index 314d900..a93521a 100644 --- a/src/utils/fs.ts +++ b/src/utils/fs.ts @@ -29,36 +29,6 @@ export async function ensureDirectory(dir: string): Promise { } } -/** - * Deletes a directory at the specified path in the filesystem. - * @param dirPath the directory which should be deleted. - */ -export async function removeDirectory(dirPath: string): Promise { - // Delete all contents of this directory because otherwise we cannot remove it (why can't that be part of fs, oh node 14+ only...) - const contents = await fs.readdir(dirPath); // get entries of directory - - const rmPromises = contents.reverse().map(async (f) => { - const subpath = path.join(dirPath, f); - - try { - const stat = await fs.lstat(subpath); - // rm if file or symlink and use this function recursively if directory - if (stat.isDirectory() && !stat.isSymbolicLink()) { - await removeDirectory(subpath); - } else { - await fs.unlink(subpath); - } - } catch (_e) { - // ignore that file cannot be removed. Maybe was already removed. - } - }); - - await Promise.all(rmPromises); - - // now that the directory is empty we can delete it. - await fs.rmdir(dirPath); -} - /** * Executes the given command and optionally streams the output to the console. * @param command the command that should be executed. diff --git a/src/utils/npm.ts b/src/utils/npm.ts index 02b31d2..bdfa1f6 100644 --- a/src/utils/npm.ts +++ b/src/utils/npm.ts @@ -1,7 +1,7 @@ import axios, { AxiosRequestConfig, AxiosResponse } from "axios"; import * as fs from "fs"; import * as path from "path"; -import { executeCommand, removeDirectory } from "./fs"; +import { executeCommand } from "./fs"; import { exec } from "child_process"; import { maxSatisfying, satisfies, SemVer } from "semver"; import * as zlib from "zlib"; @@ -223,7 +223,7 @@ export async function createNpmSymlinks(packages: NpmPackage[], nodecgIODir: str * @param nodecgIODir the directory in which nodecg-io is installed */ export async function removeNpmPackage(pkg: NpmPackage, nodecgIODir: string): Promise { - await removeDirectory(buildNpmPackagePath(pkg, nodecgIODir)); + await fs.promises.rm(buildNpmPackagePath(pkg, nodecgIODir), { recursive: true, force: true }); } /** diff --git a/test/install/development.ts b/test/install/development.ts index fca17dd..e37a217 100644 --- a/test/install/development.ts +++ b/test/install/development.ts @@ -1,9 +1,9 @@ import { vol } from "memfs"; import * as git from "isomorphic-git"; +import * as fs from "fs"; import * as fsUtils from "../../src/utils/fs"; import { fsRoot, validDevInstall, nodecgIODir } from "../test.util"; import * as dev from "../../src/install/development"; -import { removeDirectory } from "../../src/utils/fs"; const defaultFetchResult: git.FetchResult = { defaultBranch: "main", @@ -69,7 +69,7 @@ describe("getGitRepo", () => { test("should clone repo if directory does not exists", async () => { // remove dir so it should clone - await removeDirectory(nodecgIODir); + await fs.promises.rm(nodecgIODir, { recursive: true, force: true }); await dev.getGitRepo(nodecgIODir, "nodecg-io"); expect(cloneSpy).toHaveBeenCalled(); diff --git a/test/install/production.ts b/test/install/production.ts index bfbe43e..310e066 100644 --- a/test/install/production.ts +++ b/test/install/production.ts @@ -1,5 +1,6 @@ import { vol } from "memfs"; import * as path from "path"; +import * as fs from "fs"; import { corePkg, dashboardPkg, nodecgIODir, twitchChatPkg, validProdInstall } from "../test.util"; import { diffPackages, installPackages, removePackages, validateInstall } from "../../src/install/production"; import * as installation from "../../src/utils/installation"; @@ -56,13 +57,14 @@ describe("diffPackages", () => { describe("removePackages", () => { test("should rm each package directory", async () => { - const rmMock = jest.spyOn(fsUtils, "removeDirectory").mockClear().mockResolvedValue(); + const rmMock = jest.spyOn(fs.promises, "rm").mockClear().mockResolvedValue(); const i = { ...validProdInstall, packages: [...packages] }; await removePackages(packages, i, nodecgIODir); expect(rmMock).toHaveBeenCalledTimes(2); - expect(rmMock).toHaveBeenCalledWith(path.join(nodecgIODir, corePkg.path)); - expect(rmMock).toHaveBeenLastCalledWith(path.join(nodecgIODir, twitchChatPkg.path)); + const rmOpts = { recursive: true, force: true }; + expect(rmMock).toHaveBeenCalledWith(path.join(nodecgIODir, corePkg.path), rmOpts); + expect(rmMock).toHaveBeenLastCalledWith(path.join(nodecgIODir, twitchChatPkg.path), rmOpts); expect(i.packages.length).toBe(0); }); @@ -106,7 +108,7 @@ describe("installPackages", () => { test("should revert changes if npm install fails", async () => { npmInstallMock.mockRejectedValue(new Error("random error")); - const rmMock = jest.spyOn(fsUtils, "removeDirectory").mockClear().mockResolvedValue(); + const rmMock = jest.spyOn(fs.promises, "rm").mockClear().mockResolvedValue(); // should return the error await expect(installPackages(packages, createInstall(), nodecgIODir)).rejects.toThrow("random error"); diff --git a/test/uninstall/index.ts b/test/uninstall/index.ts index 6ed2e0b..deb000c 100644 --- a/test/uninstall/index.ts +++ b/test/uninstall/index.ts @@ -1,10 +1,10 @@ import { vol } from "memfs"; import { uninstall } from "../../src/uninstall"; -import * as fsUtils from "../../src/utils/fs"; import * as nodecgInstall from "../../src/utils/nodecgInstallation"; import * as nodecgConfig from "../../src/utils/nodecgConfig"; import { fsRoot } from "../test.util"; import * as path from "path"; +import * as fs from "fs"; jest.mock("fs", () => vol); afterEach(() => vol.reset()); @@ -13,18 +13,18 @@ const nodecgIODir = path.join(fsRoot, "nodecg-io"); jest.spyOn(nodecgInstall, "findNodeCGDirectory").mockResolvedValue(fsRoot); const spyManageBundleDir = jest.spyOn(nodecgConfig, "manageBundleDir"); -const spyRemoveDirectory = jest.spyOn(fsUtils, "removeDirectory"); +const spyRm = jest.spyOn(fs.promises, "rm"); afterEach(() => { spyManageBundleDir.mockClear(); - spyRemoveDirectory.mockClear(); + spyRm.mockClear(); }); describe("uninstall", () => { test("should not do anything if there is no nodecg-io directory", async () => { await uninstall(); - expect(spyRemoveDirectory).not.toHaveBeenCalled(); + expect(spyRm).not.toHaveBeenCalled(); expect(spyManageBundleDir).not.toHaveBeenCalled(); }); @@ -48,6 +48,6 @@ describe("uninstall", () => { await vol.promises.mkdir(nodecgIODir); await uninstall(); - expect(spyRemoveDirectory).toHaveBeenCalledWith(nodecgIODir); + expect(spyRm).toHaveBeenCalledWith(nodecgIODir, { recursive: true, force: true }); }); }); diff --git a/test/utils/fs.ts b/test/utils/fs.ts index 36c558e..8d2c94e 100644 --- a/test/utils/fs.ts +++ b/test/utils/fs.ts @@ -1,5 +1,5 @@ import { vol } from "memfs"; -import { directoryExists, ensureDirectory, executeCommand, removeDirectory } from "../../src/utils/fs"; +import { directoryExists, ensureDirectory, executeCommand } from "../../src/utils/fs"; import * as path from "path"; import * as child_process from "child_process"; import { testDir } from "../test.util"; @@ -40,24 +40,6 @@ describe("ensureDirectory", () => { }); }); -describe("removeDirectory", () => { - test("should remove directory recursively", async () => { - await vol.promises.mkdir(testDir); - await vol.promises.writeFile(path.join(testDir, "test.txt"), "abc"); - await vol.promises.mkdir(path.join(testDir, "sub-dir")); - - await removeDirectory(testDir); - - // Directory should not be there anymore. - await expect(vol.promises.stat(testDir)).rejects.toThrow("no such file or directory"); - }); - - test("should fail if directory does not exist", async () => { - // should fail because the directory does not exist - await expect(removeDirectory(testDir)).rejects.toThrow("no such file or directory"); - }); -}); - describe("executeCommand", () => { test("should not error if the command successfully exits (code 0)", async () => { await executeCommand("exit", ["0"]); diff --git a/test/utils/npm/pkgManagement.ts b/test/utils/npm/pkgManagement.ts index 9228a39..855d4c0 100644 --- a/test/utils/npm/pkgManagement.ts +++ b/test/utils/npm/pkgManagement.ts @@ -9,6 +9,7 @@ import { import { tempDir, corePkg, fsRoot, twitchChatPkg, dashboardPkg } from "../../test.util"; import * as fsUtils from "../../../src/utils/fs"; import * as path from "path"; +import * as fs from "fs"; jest.mock("fs", () => createFsFromVolume(vol)); afterEach(() => vol.reset()); @@ -69,10 +70,12 @@ describe("createNpmSymlinks", () => { }); describe("removeNpmPackage", () => { - test("should call to fsUtils.removeDirectory", async () => { - const spy = jest.spyOn(fsUtils, "removeDirectory").mockResolvedValue(); + test("should call to fs.promises.rm to delete directory", async () => { + const spy = jest.spyOn(fs.promises, "rm").mockResolvedValue(); await removeNpmPackage(corePkg, await tempDir()); expect(spy).toHaveBeenCalled(); + // Ensure that recursive deletion is enabled + expect(spy.mock.calls[0]?.[1]?.recursive).toBe(true); }); });