diff --git a/.changeset/windows-npm-ctrl-c-prompt.md b/.changeset/windows-npm-ctrl-c-prompt.md new file mode 100644 index 0000000000..53d54e34f4 --- /dev/null +++ b/.changeset/windows-npm-ctrl-c-prompt.md @@ -0,0 +1,10 @@ +--- +"cf": patch +--- + +Exit cleanly after Ctrl+C when a framework command runs through npm on Windows + +`cf dev`, `cf build`, and `cf init` now start `npm` and `npx` commands through +npm's CLI script instead of the `npm.cmd` and `npx.cmd` launchers. Those +launchers keep a batch file running, so pressing Ctrl+C left cf waiting at +cmd's "Terminate batch job (Y/N)?" prompt after the dev server had stopped. diff --git a/packages/cli/src/__tests__/commands/dev/windows-console.test.ts b/packages/cli/src/__tests__/commands/dev/windows-console.test.ts index e373c92a51..4e1c40c373 100644 --- a/packages/cli/src/__tests__/commands/dev/windows-console.test.ts +++ b/packages/cli/src/__tests__/commands/dev/windows-console.test.ts @@ -1,6 +1,7 @@ import { execFile } from "node:child_process"; import { existsSync, + mkdirSync, mkdtempSync, readFileSync, rmSync, @@ -24,15 +25,42 @@ const harness = fileURLToPath( ); const spawnSource = new URL("../../../commands/dev/spawn.ts", import.meta.url); const projectSource = new URL("../../../lib/autoconfig.ts", import.meta.url); +// npm's bin/npx.cmd as shipped with Node.js. +const NPX_CMD = [ + ":: Created by npm, please don't edit manually.", + "@ECHO OFF", + "", + "SETLOCAL", + "", + 'SET "NODE_EXE=%~dp0\\node.exe"', + 'IF NOT EXIST "%NODE_EXE%" (', + ' SET "NODE_EXE=node"', + ")", + "", + 'SET "NPM_PREFIX_JS=%~dp0\\node_modules\\npm\\bin\\npm-prefix.js"', + 'SET "NPX_CLI_JS=%~dp0\\node_modules\\npm\\bin\\npx-cli.js"', + 'FOR /F "delims=" %%F IN (\'CALL "%NODE_EXE%" "%NPM_PREFIX_JS%"\') DO (', + ' SET "NPM_PREFIX_NPX_CLI_JS=%%F\\node_modules\\npm\\bin\\npx-cli.js"', + ")", + 'IF EXIST "%NPM_PREFIX_NPX_CLI_JS%" (', + ' SET "NPX_CLI_JS=%NPM_PREFIX_NPX_CLI_JS%"', + ")", + "", + '"%NODE_EXE%" "%NPX_CLI_JS%" %*', + "", +].join("\r\n"); describe.skipIf(process.platform !== "win32")("Windows console Ctrl+C", () => { - it.each<["implementation" | "framework", CommandOutput]>([ + it.each<["implementation" | "framework" | "npx", CommandOutput]>([ ["implementation", "inherit"], ["implementation", "stderr"], ["implementation", "silent"], ["framework", "inherit"], ["framework", "stderr"], ["framework", "silent"], + ["npx", "inherit"], + ["npx", "stderr"], + ["npx", "silent"], ])( "lets the %s delegate finish cleanup with %s output", async (launcher, output) => { @@ -40,9 +68,9 @@ describe.skipIf(process.platform !== "win32")("Windows console Ctrl+C", () => { const driver = join(directory, "parent.mts"); const delegate = join(directory, "delegate"); const framework = join(directory, "framework.cmd"); - writeFileSync( - delegate, - nodeScript(` + const npmDirectory = join(directory, "nodejs"); + const npmBin = join(npmDirectory, "node_modules", "npm", "bin"); + const delegateSource = nodeScript(` const { writeFileSync } = require("node:fs"); let signals = 0; process.on("SIGINT", () => { @@ -54,8 +82,8 @@ process.on("SIGINT", () => { }); setInterval(() => {}, 1000); writeFileSync("delegate-ready.json", JSON.stringify({ pid: process.pid })); -`) - ); +`); + writeFileSync(delegate, delegateSource); // Cover the extensionless shebang and npm's Windows .cmd launch paths. // npm ends the batch context before running Node to avoid its Ctrl+C // confirmation prompt: npm/cmd-shim's lib/index.js uses this tail. @@ -63,23 +91,34 @@ writeFileSync("delegate-ready.json", JSON.stringify({ pid: process.pid })); framework, `@echo off\r\nsetlocal\r\nendlocal & goto #_undefined_# 2>NUL || title %COMSPEC% & "${process.execPath}" "%~dp0delegate" %*\r\n` ); + // The npx.cmd bundled with Node has no such tail, so mirror it verbatim. + mkdirSync(npmBin, { recursive: true }); + writeFileSync(join(npmDirectory, "npx.cmd"), NPX_CMD); + writeFileSync(join(npmBin, "npx-cli.js"), delegateSource); + writeFileSync( + join(npmBin, "npm-prefix.js"), + `console.log(${JSON.stringify(directory)});\n` + ); + const executions = { + implementation: `spawnImpl({ + impl: { ecosystem: "npm", pkg: "test", installCommands: ["test"] }, + binary: ${JSON.stringify(delegate)}, + manifestPath: "test" +}, "dev", [], { output: ${JSON.stringify(output)} })`, + framework: `runProjectCommand(${JSON.stringify(`"${framework}"`)}, ${JSON.stringify(directory)}, { output: ${JSON.stringify(output)} })`, + npx: `runProjectCommand("npx astro dev", ${JSON.stringify(directory)}, { output: ${JSON.stringify(output)} })`, + }; writeFileSync( driver, ` import { existsSync, writeFileSync } from "node:fs"; +import { delimiter } from "node:path"; import { spawnImpl } from ${JSON.stringify(spawnSource.href)}; import { runProjectCommand } from ${JSON.stringify(projectSource.href)}; +process.env.PATH = ${JSON.stringify(npmDirectory)} + delimiter + process.env.PATH; const sigintListeners = process.listenerCount("SIGINT"); const sigtermListeners = process.listenerCount("SIGTERM"); -const execution = ${ - launcher === "implementation" - ? `spawnImpl({ - impl: { ecosystem: "npm", pkg: "test", installCommands: ["test"] }, - binary: ${JSON.stringify(delegate)}, - manifestPath: "test" -}, "dev", [], { output: ${JSON.stringify(output)} })` - : `runProjectCommand(${JSON.stringify(`"${framework}"`)}, ${JSON.stringify(directory)}, { output: ${JSON.stringify(output)} })` - }; +const execution = ${executions[launcher]}; writeFileSync("parent-ready.json", JSON.stringify({ pid: process.pid })); const result = await execution; writeFileSync("parent-result.json", JSON.stringify({ diff --git a/packages/cli/src/__tests__/lib/npm-launcher.test.ts b/packages/cli/src/__tests__/lib/npm-launcher.test.ts new file mode 100644 index 0000000000..5583125180 --- /dev/null +++ b/packages/cli/src/__tests__/lib/npm-launcher.test.ts @@ -0,0 +1,125 @@ +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { delimiter, dirname, join } from "node:path"; +import { describe, expect, it } from "vite-plus/test"; +import { resolveNpmLauncher } from "../../lib/npm-launcher.js"; + +function npmInstall(root: string, files: string[] = []): string { + const bin = join(root, "node_modules", "npm", "bin"); + mkdirSync(bin, { recursive: true }); + for (const name of ["npm", "npx"]) { + writeFileSync(join(bin, `${name}-cli.js`), ""); + } + for (const file of files) { + writeFileSync(join(root, file), ""); + } + return bin; +} + +function fakeExecutable(directory: string, file: string): string { + mkdirSync(directory, { recursive: true }); + writeFileSync(join(directory, file), ""); + return join(directory, file); +} + +// The empty extension lets the real node binary resolve on POSIX test hosts. +function windowsEnv(...path: string[]): NodeJS.ProcessEnv { + return { + Path: path.join(delimiter), + PATHEXT: [".exe", ".cmd", ""].join(delimiter), + }; +} + +describe("resolveNpmLauncher", () => { + it("runs npx through the CLI script with the node.exe beside npx.cmd", () => { + const root = mkdtempSync(join(tmpdir(), "cf-npm-launcher-")); + const nodejs = join(root, "nodejs"); + const bin = npmInstall(nodejs, ["npx.cmd", "node.exe"]); + const otherNode = fakeExecutable(join(root, "other"), "node.exe"); + + expect( + resolveNpmLauncher( + ["npx", "astro", "dev"], + root, + windowsEnv(join(root, "empty"), dirname(otherNode), nodejs), + "win32" + ) + ).toEqual([ + join(nodejs, "node.exe"), + join(bin, "npx-cli.js"), + "astro", + "dev", + ]); + }); + + it("falls back to node from PATH when npm.cmd has no adjacent node.exe", () => { + const root = mkdtempSync(join(tmpdir(), "cf-npm-launcher-")); + const prefix = join(root, "prefix"); + const bin = npmInstall(prefix, ["npm.cmd"]); + const node = fakeExecutable(join(root, "nodejs"), "node.exe"); + + expect( + resolveNpmLauncher( + ["npm", "run", "dev"], + root, + windowsEnv(prefix, dirname(node)), + "win32" + ) + ).toEqual([node, join(bin, "npm-cli.js"), "run", "dev"]); + }); + + it("leaves npm alone when an earlier PATH entry has a native npm.exe shim", () => { + const root = mkdtempSync(join(tmpdir(), "cf-npm-launcher-")); + const shim = fakeExecutable(join(root, "shims"), "npm.exe"); + const nodejs = join(root, "nodejs"); + npmInstall(nodejs, ["npm.cmd", "node.exe"]); + + expect( + resolveNpmLauncher( + ["npm", "run", "dev"], + root, + windowsEnv(dirname(shim), nodejs), + "win32" + ) + ).toEqual(["npm", "run", "dev"]); + }); + + it("prefers an npm upgraded into the global prefix, like npm.cmd", () => { + const root = mkdtempSync(join(tmpdir(), "cf-npm-launcher-")); + const nodejs = join(root, "nodejs"); + const prefix = join(root, "prefix"); + const bundledBin = npmInstall(nodejs, ["npm.cmd"]); + const prefixBin = npmInstall(prefix); + writeFileSync( + join(bundledBin, "npm-prefix.js"), + `console.log(${JSON.stringify(prefix)});` + ); + + expect( + resolveNpmLauncher( + ["npm.cmd", "run", "build"], + root, + windowsEnv(nodejs, dirname(process.execPath)), + "win32" + ) + ).toEqual([ + process.execPath, + join(prefixBin, "npm-cli.js"), + "run", + "build", + ]); + }); + + it("leaves other commands and other platforms unchanged", () => { + const root = mkdtempSync(join(tmpdir(), "cf-npm-launcher-")); + const nodejs = join(root, "nodejs"); + npmInstall(nodejs, ["npx.cmd", "pnpm.cmd", "node.exe"]); + + expect( + resolveNpmLauncher(["pnpm", "dev"], root, windowsEnv(nodejs), "win32") + ).toEqual(["pnpm", "dev"]); + expect( + resolveNpmLauncher(["npx", "astro"], root, windowsEnv(nodejs), "linux") + ).toEqual(["npx", "astro"]); + }); +}); diff --git a/packages/cli/src/lib/autoconfig.ts b/packages/cli/src/lib/autoconfig.ts index dd38fadfa5..6b437592e5 100644 --- a/packages/cli/src/lib/autoconfig.ts +++ b/packages/cli/src/lib/autoconfig.ts @@ -9,6 +9,7 @@ import { x } from "tinyexec"; import { CliExit } from "./cli-exit.js"; import { confirm, prompt, select } from "./dialog.js"; import { isNonInteractiveOrCI } from "./interactive.js"; +import { resolveNpmLauncher } from "./npm-launcher.js"; import { createChildProcessController } from "./process.js"; import { log } from "./ui/log.js"; import { maybeApplyVinextCommandOverrides } from "./vinext.js"; @@ -179,7 +180,12 @@ export async function runProjectCommand( `Delegating to ${command}${formattedArgs ? ` ${formattedArgs}` : ""}${environmentSuffix}` ); } - const [file, ...commandArgs] = parseProjectCommand(command); + // Match the environment tinyexec gives the child so both resolve the same npm. + const [file, ...commandArgs] = resolveNpmLauncher( + parseProjectCommand(command), + cwd, + { ...process.env, ...options.env } + ); const subprocess = x(file, [...commandArgs, ...(options.args ?? [])], { nodePath: false, nodeOptions: { diff --git a/packages/cli/src/lib/npm-launcher.ts b/packages/cli/src/lib/npm-launcher.ts new file mode 100644 index 0000000000..16709d9287 --- /dev/null +++ b/packages/cli/src/lib/npm-launcher.ts @@ -0,0 +1,109 @@ +import { execFileSync } from "node:child_process"; +import { existsSync, statSync } from "node:fs"; +import { basename, delimiter, dirname, join, resolve } from "node:path"; + +/** + * Run `npm` / `npx` on Windows through the CLI script their `.cmd` launcher + * would start. Node.js's bundled npm.cmd and npx.cmd keep a batch file active + * while npm runs, so a console Ctrl+C ends at cmd's "Terminate batch job + * (Y/N)?" prompt instead of exiting. Other commands are returned unchanged. + */ +export function resolveNpmLauncher( + command: [string, ...string[]], + cwd: string, + env: NodeJS.ProcessEnv = process.env, + platform: NodeJS.Platform = process.platform +): [string, ...string[]] { + const [file, ...args] = command; + if (platform !== "win32" || !/^(?:npm|npx)(?:\.cmd)?$/i.test(file)) { + return command; + } + + // Rewrite only when the child would start npm's batch launcher, so an + // earlier npm.exe shim (such as a version manager's) still wins. + const launcher = findExecutable(file, cwd, env); + if (launcher === undefined) { + return command; + } + const name = /^(npm|npx)\.cmd$/i.exec(basename(launcher))?.[1]?.toLowerCase(); + if (name === undefined) { + return command; + } + const directory = dirname(launcher); + const bin = join(directory, "node_modules", "npm", "bin"); + const script = join(bin, `${name}-cli.js`); + if (!existsSync(script)) { + return command; + } + + const adjacentNode = join(directory, "node.exe"); + let node: string | undefined; + if (existsSync(adjacentNode)) { + node = adjacentNode; + } else { + // npm.cmd falls back to `node` as cmd would resolve it, not cf's runtime. + node = findExecutable("node", cwd, env); + } + if (node === undefined) { + return command; + } + return [node, globalPrefixScript(node, bin, name) ?? script, ...args]; +} + +// Mirrors tinyexec's Windows lookup so the rewrite targets the same launcher. +function findExecutable( + command: string, + cwd: string, + env: NodeJS.ProcessEnv +): string | undefined { + const pathKey = Object.keys(env).find((key) => /^path$/i.test(key)); + const path = pathKey === undefined ? "" : (env[pathKey] ?? ""); + let extensions = env.PATHEXT + ? env.PATHEXT.split(delimiter) + : [".EXE", ".CMD", ".BAT", ".COM"]; + if (command.includes(".")) { + extensions = ["", ...extensions]; + } + for (const directory of [cwd, ...path.split(delimiter)]) { + for (const extension of extensions) { + const candidate = resolve(cwd, directory, command + extension); + if (isFile(candidate)) { + return candidate; + } + } + } + return undefined; +} + +function isFile(path: string): boolean { + try { + return statSync(path).isFile(); + } catch { + return false; + } +} + +/** Like the launchers, prefer an npm installed into the global prefix. */ +function globalPrefixScript( + node: string, + bin: string, + name: string +): string | undefined { + const prefixScript = join(bin, "npm-prefix.js"); + if (!existsSync(prefixScript)) { + return undefined; + } + try { + const prefix = execFileSync(node, [prefixScript], { + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + // A hung probe would otherwise block the command from ever starting. + timeout: 5000, + windowsHide: true, + }).trim(); + const script = join(prefix, "node_modules", "npm", "bin", `${name}-cli.js`); + return prefix !== "" && existsSync(script) ? script : undefined; + } catch { + return undefined; + } +}