From 2bef1f5dad894c066a4992f7ced0b8be95ef2db6 Mon Sep 17 00:00:00 2001 From: galargh Date: Tue, 19 Nov 2024 11:17:31 +0100 Subject: [PATCH 1/4] test: reenable the install project dependencies test --- v-next/hardhat/test/internal/cli/init/init.ts | 79 +++++++++++-------- 1 file changed, 45 insertions(+), 34 deletions(-) diff --git a/v-next/hardhat/test/internal/cli/init/init.ts b/v-next/hardhat/test/internal/cli/init/init.ts index fa84e027a8..70537d99ae 100644 --- a/v-next/hardhat/test/internal/cli/init/init.ts +++ b/v-next/hardhat/test/internal/cli/init/init.ts @@ -152,34 +152,38 @@ describe("copyProjectFiles", () => { }); }); -describe("installProjectDependencies", () => { +describe("installProjectDependencies", async () => { useTmpDir("installProjectDependencies"); disableConsole(); - describe("when install is true", () => { - // This test is skipped because installing dependencies over the network is slow - it.skip("should install the project dependencies", async () => { - const template = await getTemplate("empty-typescript"); - await writeUtf8File("package.json", JSON.stringify({ type: "module" })); - await installProjectDependencies(process.cwd(), template, true); - assert.ok(await exists("node_modules"), "node_modules should exist"); - }); - }); - describe("when install is false", () => { - it("should not install the project dependencies", async () => { - const template = await getTemplate("mocha-ethers"); - await writeUtf8File("package.json", JSON.stringify({ type: "module" })); - await installProjectDependencies(process.cwd(), template, false); - assert.ok( - !(await exists("node_modules")), - "node_modules should not exist", - ); - }); + const templates = await getTemplates(); + + for (const template of templates) { + // NOTE: This test is slow because it installs dependencies over the network. + // It tests installation for all the templates, but only with the npm as the + // package manager. We also support pnpm and yarn. + it( + `should install all the ${template.name} template dependencies in an empty project if the user opts-in to the installation`, + { + skip: process.env.HARDHAT_DISABLE_SLOW_TESTS === "true", + }, + async () => { + await writeUtf8File("package.json", JSON.stringify({ type: "module" })); + await installProjectDependencies(process.cwd(), template, true); + assert.ok(await exists("node_modules"), "node_modules should exist"); + }, + ); + } + + it("should not install any template dependencies if the user opts-out of the installation", async () => { + const template = await getTemplate("mocha-ethers"); + await writeUtf8File("package.json", JSON.stringify({ type: "module" })); + await installProjectDependencies(process.cwd(), template, false); + assert.ok(!(await exists("node_modules")), "node_modules should not exist"); }); }); -// NOTE: This uses network to access the npm registry describe("initHardhat", async () => { useTmpDir("initHardhat"); @@ -188,18 +192,25 @@ describe("initHardhat", async () => { const templates = await getTemplates(); for (const template of templates) { - it(`should initialize the project using the ${template.name} template in an empty folder`, async () => { - await initHardhat({ - template: template.name, - workspace: process.cwd(), - force: false, - install: false, - }); - assert.ok(await exists("package.json"), "package.json should exist"); - for (const file of template.files) { - const pathToFile = path.join(process.cwd(), file); - assert.ok(await exists(pathToFile), `File ${file} should exist`); - } - }); + // NOTE: This test uses network to access the npm registry + it( + `should initialize the project using the ${template.name} template in an empty folder`, + { + skip: process.env.HARDHAT_DISABLE_SLOW_TESTS === "true", + }, + async () => { + await initHardhat({ + template: template.name, + workspace: process.cwd(), + force: false, + install: false, + }); + assert.ok(await exists("package.json"), "package.json should exist"); + for (const file of template.files) { + const pathToFile = path.join(process.cwd(), file); + assert.ok(await exists(pathToFile), `File ${file} should exist`); + } + }, + ); } }); From 3af4d63d972d2f75003252f046bcac6b3d2c4a85 Mon Sep 17 00:00:00 2001 From: galargh Date: Tue, 19 Nov 2024 11:18:01 +0100 Subject: [PATCH 2/4] chore: add the telemetry consent question to the init flow --- v-next/hardhat/src/internal/cli/init/init.ts | 21 +++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/v-next/hardhat/src/internal/cli/init/init.ts b/v-next/hardhat/src/internal/cli/init/init.ts index 1b5c0f19f8..2a7f8a4569 100644 --- a/v-next/hardhat/src/internal/cli/init/init.ts +++ b/v-next/hardhat/src/internal/cli/init/init.ts @@ -18,6 +18,7 @@ import chalk from "chalk"; import { findClosestHardhatConfig } from "../../config-loading.js"; import { getHardhatVersion } from "../../utils/package.js"; +import { ensureTelemetryConsent } from "../telemetry/telemetry-permissions.js"; import { HARDHAT_NAME } from "./constants.js"; import { @@ -59,16 +60,18 @@ export interface InitHardhatOptions { * The flow is as follows: * 1. Print the ascii logo. * 2. Print the welcome message. - * 3. Optionally, ask the user for the workspace to initialize the project in. - * 4. Optionally, ask the user for the template to use for the project initialization. - * 5. Create the package.json file if it does not exist. - * 6. Validate that the package.json file is an esm package. - * 7. Optionally, ask the user if files should be overwritten. - * 8. Copy the template files to the workspace. + * 3. Ensure telemetry consent. + * 4. Optionally, ask the user for the workspace to initialize the project in. + * 5. Optionally, ask the user for the template to use for the project initialization. + * 6. Create the package.json file if it does not exist. + * 7. Validate that the package.json file is an esm package. + * 8. Optionally, ask the user if files should be overwritten. + * 9. Copy the template files to the workspace. * 10. Print the commands to install the project dependencies. * 11. Optionally, ask the user if the project dependencies should be installed. * 12. Optionally, run the commands to install the project dependencies. - * 13. Print a message to star the project on GitHub. + * 13. Ensure telemetry consent. + * 14. Print a message to star the project on GitHub. */ export async function initHardhat(options?: InitHardhatOptions): Promise { try { @@ -76,6 +79,10 @@ export async function initHardhat(options?: InitHardhatOptions): Promise { await printWelcomeMessage(); + // Ensure telemetry consent first so that we are allowed to also track + // the unfinished init flows + await ensureTelemetryConsent(); + // Ask the user for the workspace to initialize the project in // if it was not provided, and validate that it is not already initialized const workspace = await getWorkspace(options?.workspace); From 4177b7a2832c1e7316c3a511936b2786d53bc681 Mon Sep 17 00:00:00 2001 From: galargh Date: Tue, 19 Nov 2024 21:43:47 +0100 Subject: [PATCH 3/4] feat: update existing dependencies during init --- v-next/hardhat/src/internal/cli/init/init.ts | 94 ++++++++++++++----- .../src/internal/cli/init/package-manager.ts | 7 +- .../hardhat/src/internal/cli/init/prompt.ts | 22 ++++- v-next/hardhat/test/internal/cli/init/init.ts | 23 ++++- .../test/internal/cli/init/package-manager.ts | 6 +- 5 files changed, 123 insertions(+), 29 deletions(-) diff --git a/v-next/hardhat/src/internal/cli/init/init.ts b/v-next/hardhat/src/internal/cli/init/init.ts index 2a7f8a4569..b382a67b0b 100644 --- a/v-next/hardhat/src/internal/cli/init/init.ts +++ b/v-next/hardhat/src/internal/cli/init/init.ts @@ -15,6 +15,7 @@ import { } from "@ignored/hardhat-vnext-utils/fs"; import { resolveFromRoot } from "@ignored/hardhat-vnext-utils/path"; import chalk from "chalk"; +import * as semver from "semver"; import { findClosestHardhatConfig } from "../../config-loading.js"; import { getHardhatVersion } from "../../utils/package.js"; @@ -30,6 +31,7 @@ import { promptForForce, promptForInstall, promptForTemplate, + promptForUpdate, promptForWorkspace, } from "./prompt.js"; import { spawn } from "./subprocess.js"; @@ -322,11 +324,13 @@ export async function copyProjectFiles( * @param workspace The path to the workspace to initialize the project in. * @param template The template to use for the project initialization. * @param install Whether to install the project dependencies. + * @param update Whether to update the project dependencies. */ export async function installProjectDependencies( workspace: string, template: Template, install?: boolean, + update?: boolean, ): Promise { const pathToWorkspacePackageJson = path.join(workspace, "package.json"); @@ -347,42 +351,42 @@ export async function installProjectDependencies( templateDependencies[name] = version; } } - const workspaceDependencies = workspacePkg.devDependencies ?? {}; - const dependenciesToInstall = Object.entries(templateDependencies) + + // Checking both workspace dependencies and dev dependencies in case the user + // installed a dev dependency as a dependency + const workspaceDependencies = { + ...(workspacePkg.dependencies ?? {}), + ...(workspacePkg.devDependencies ?? {}), + }; + + // We need to strip the optional workspace prefix from template dependency versions + const templateDependencyEntries = Object.entries(templateDependencies).map( + ([name, version]) => [name, version.replace(/^workspace:/, "")], + ); + + // Finding the dependencies that are not already installed + const dependenciesToInstall = templateDependencyEntries .filter(([name]) => workspaceDependencies[name] === undefined) - .map(([name, version]) => { - // Strip the workspace: prefix from the version - return `${name}@${version.replace(/^workspace:/, "")}`; - }); + .map(([name, version]) => `${name}@${version}`); // Try to install the missing dependencies if there are any if (Object.keys(dependenciesToInstall).length !== 0) { // Retrieve the package manager specific installation command - let command = getDevDependenciesInstallationCommand( + const command = getDevDependenciesInstallationCommand( packageManager, dependenciesToInstall, ); + const commandString = command.join(" "); - // We quote all the dependency identifiers to that it can be run on a shell - // without semver symbols interfering with the command - command = [ - command[0], - command[1], - command[2], - ...command.slice(3).map((arg) => `"${arg}"`), - ]; - - const formattedCommand = command.join(" "); - - // Ask the user for permission to install the project dependencies and install them if needed + // Ask the user for permission to install the project dependencies if (install === undefined) { - install = await promptForInstall(formattedCommand); + install = await promptForInstall(commandString); } // If the user grants permission to install the dependencies, run the installation command if (install) { console.log(); - console.log(formattedCommand); + console.log(commandString); await spawn(command[0], command.slice(1), { cwd: workspace, @@ -396,6 +400,54 @@ export async function installProjectDependencies( console.log(`✨ ${chalk.cyan(`Dependencies installed`)} ✨`); } } + + // NOTE: Even though the dependency updates are very similar to pure + // installations, they are kept separate to allow the user to skip one while + // proceeding with the other, and to allow us to design handling of these + // two processes independently. + + // Finding the installed dependencies that have an incompatible version + const dependenciesToUpdate = templateDependencyEntries + .filter(([name, version]) => { + const workspaceVersion = workspaceDependencies[name]; + return ( + workspaceVersion !== undefined && + !semver.satisfies(version, workspaceVersion) && + !semver.intersects(version, workspaceVersion) + ); + }) + .map(([name, version]) => `${name}@${version}`); + + // Try to update the missing dependencies if there are any. + if (dependenciesToUpdate.length !== 0) { + // Retrieve the package manager specific installation command + const command = getDevDependenciesInstallationCommand( + packageManager, + dependenciesToUpdate, + ); + const commandString = command.join(" "); + + // Ask the user for permission to update the project dependencies + if (update === undefined) { + update = await promptForUpdate(commandString); + } + + if (update) { + console.log(); + console.log(commandString); + + await spawn(command[0], command.slice(1), { + cwd: workspace, + // We need to run with `shell: true` for this to work on powershell, but + // we already enclosed every dependency identifier in quotes, so this + // is safe. + shell: true, + stdio: "inherit", + }); + + console.log(`✨ ${chalk.cyan(`Dependencies updated`)} ✨`); + } + } } function showStarOnGitHubMessage() { diff --git a/v-next/hardhat/src/internal/cli/init/package-manager.ts b/v-next/hardhat/src/internal/cli/init/package-manager.ts index 8a4dcfb85c..e064457a97 100644 --- a/v-next/hardhat/src/internal/cli/init/package-manager.ts +++ b/v-next/hardhat/src/internal/cli/init/package-manager.ts @@ -30,7 +30,8 @@ export async function getPackageManager( /** * getDevDependenciesInstallationCommand returns the command to install the given dependencies - * as dev dependencies using the given package manager. + * as dev dependencies using the given package manager. The returned command should + * be safe to run on the command line. * * @param packageManager The package manager to use. * @param dependencies The dependencies to install. @@ -46,7 +47,9 @@ export function getDevDependenciesInstallationCommand( pnpm: ["pnpm", "add", "--save-dev"], }; const command = packageManagerToCommand[packageManager]; - command.push(...dependencies); + // We quote all the dependency identifiers so that they can be run on a shell + // without semver symbols interfering with the command + command.push(...dependencies.map((d) => `"${d}"`)); return command; } diff --git a/v-next/hardhat/src/internal/cli/init/prompt.ts b/v-next/hardhat/src/internal/cli/init/prompt.ts index f42040d355..ba7e5fb7c7 100644 --- a/v-next/hardhat/src/internal/cli/init/prompt.ts +++ b/v-next/hardhat/src/internal/cli/init/prompt.ts @@ -1,6 +1,7 @@ import type { Template } from "./template.js"; import { HardhatError } from "@ignored/hardhat-vnext-errors"; +import chalk from "chalk"; export async function promptForWorkspace(): Promise { ensureTTY(); @@ -71,7 +72,7 @@ export async function promptForInstall( { name: "install", type: "confirm", - message: `You need to install the project dependencies using the following command:\n${safelyFormattedCommand}\n\nDo you want to run it now?`, + message: `You need to install the following dependencies using the following command:\n${chalk.italic(safelyFormattedCommand)}\n\nDo you want to run it now?`, initial: true, }, ]); @@ -79,6 +80,25 @@ export async function promptForInstall( return installResponse.install; } +export async function promptForUpdate( + safelyFormattedCommand: string, +): Promise { + ensureTTY(); + + const { default: enquirer } = await import("enquirer"); + + const updateResponse = await enquirer.prompt<{ update: boolean }>([ + { + name: "update", + type: "confirm", + message: `You need to update the following dependencies using the following command:\n${chalk.italic(safelyFormattedCommand)}\n\nDo you want to run it now?`, + initial: true, + }, + ]); + + return updateResponse.update; +} + /** * ensureTTY checks if the process is running in a TTY (i.e. a terminal). * If it is not, it throws and error. diff --git a/v-next/hardhat/test/internal/cli/init/init.ts b/v-next/hardhat/test/internal/cli/init/init.ts index 70537d99ae..203de3fabc 100644 --- a/v-next/hardhat/test/internal/cli/init/init.ts +++ b/v-next/hardhat/test/internal/cli/init/init.ts @@ -170,7 +170,7 @@ describe("installProjectDependencies", async () => { }, async () => { await writeUtf8File("package.json", JSON.stringify({ type: "module" })); - await installProjectDependencies(process.cwd(), template, true); + await installProjectDependencies(process.cwd(), template, true, false); assert.ok(await exists("node_modules"), "node_modules should exist"); }, ); @@ -179,9 +179,28 @@ describe("installProjectDependencies", async () => { it("should not install any template dependencies if the user opts-out of the installation", async () => { const template = await getTemplate("mocha-ethers"); await writeUtf8File("package.json", JSON.stringify({ type: "module" })); - await installProjectDependencies(process.cwd(), template, false); + await installProjectDependencies(process.cwd(), template, false, false); assert.ok(!(await exists("node_modules")), "node_modules should not exist"); }); + + it( + "should install any existing template dependencies that are out of date if the user opts-in to the update", + { + skip: process.env.HARDHAT_DISABLE_SLOW_TESTS === "true", + }, + async () => { + const template = await getTemplate("mocha-ethers"); + await writeUtf8File( + "package.json", + JSON.stringify({ + type: "module", + devDependencies: { "@ignored/hardhat-vnext": "0.0.0" }, + }), + ); + await installProjectDependencies(process.cwd(), template, false, true); + assert.ok(await exists("node_modules"), "node_modules should exist"); + }, + ); }); describe("initHardhat", async () => { diff --git a/v-next/hardhat/test/internal/cli/init/package-manager.ts b/v-next/hardhat/test/internal/cli/init/package-manager.ts index b2c0ad8e8d..5718cf7615 100644 --- a/v-next/hardhat/test/internal/cli/init/package-manager.ts +++ b/v-next/hardhat/test/internal/cli/init/package-manager.ts @@ -137,14 +137,14 @@ describe("installsPeerDependenciesByDefault", () => { describe("getDevDependenciesInstallationCommand", () => { it("should return the correct command for pnpm", async () => { const command = getDevDependenciesInstallationCommand("pnpm", ["a", "b"]); - assert.equal(command.join(" "), "pnpm add --save-dev a b"); + assert.equal(command.join(" "), 'pnpm add --save-dev "a" "b"'); }); it("should return the correct command for npm", async () => { const command = getDevDependenciesInstallationCommand("npm", ["a", "b"]); - assert.equal(command.join(" "), "npm install --save-dev a b"); + assert.equal(command.join(" "), 'npm install --save-dev "a" "b"'); }); it("should return the correct command for yarn", async () => { const command = getDevDependenciesInstallationCommand("yarn", ["a", "b"]); - assert.equal(command.join(" "), "yarn add --dev a b"); + assert.equal(command.join(" "), 'yarn add --dev "a" "b"'); }); }); From 3751e97acf3cfae90b35671b3d5711d1b18bde29 Mon Sep 17 00:00:00 2001 From: galargh Date: Tue, 19 Nov 2024 21:55:04 +0100 Subject: [PATCH 4/4] test: make node_modules checks in init tests more thorough --- v-next/hardhat/test/internal/cli/init/init.ts | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/v-next/hardhat/test/internal/cli/init/init.ts b/v-next/hardhat/test/internal/cli/init/init.ts index 203de3fabc..ca846f3cfb 100644 --- a/v-next/hardhat/test/internal/cli/init/init.ts +++ b/v-next/hardhat/test/internal/cli/init/init.ts @@ -172,6 +172,19 @@ describe("installProjectDependencies", async () => { await writeUtf8File("package.json", JSON.stringify({ type: "module" })); await installProjectDependencies(process.cwd(), template, true, false); assert.ok(await exists("node_modules"), "node_modules should exist"); + const dependencies = Object.keys( + template.packageJson.devDependencies ?? {}, + ); + for (const dependency of dependencies) { + const nodeModulesPath = path.join( + "node_modules", + ...dependency.split("/"), + ); + assert.ok( + await exists(nodeModulesPath), + `${nodeModulesPath} should exist`, + ); + } }, ); } @@ -199,6 +212,26 @@ describe("installProjectDependencies", async () => { ); await installProjectDependencies(process.cwd(), template, false, true); assert.ok(await exists("node_modules"), "node_modules should exist"); + const dependencies = Object.keys( + template.packageJson.devDependencies ?? {}, + ); + for (const dependency of dependencies) { + const nodeModulesPath = path.join( + "node_modules", + ...dependency.split("/"), + ); + if (dependency === "@ignored/hardhat-vnext") { + assert.ok( + await exists(nodeModulesPath), + `${nodeModulesPath} should exist`, + ); + } else { + assert.ok( + !(await exists(nodeModulesPath)), + `${nodeModulesPath} should not exist`, + ); + } + } }, ); });