Skip to content

Commit

Permalink
fix: ensure shell scripts work on Windows
Browse files Browse the repository at this point in the history
Our use of `shell-quote` was causing problems on Windows where it was
escaping character (such as `@`) by placing a backslash in front.
This made Windows think that such path arguments, were at the root.

For example, `npm install -D @cloudflare/workers-types` was being converted to
`npm install -D \@cloudflare/workers-types`, which resulted in errors like:

```
npm ERR! enoent ENOENT: no such file or directory, open 'D:\@cloudflare\workers-types\package.json'
```

Now we just rely directly on the Node.js `spawn` API to avoid any shell quoting
concerns. This has resulted in a slightly less streamlined experience for people
writing C3 plugins, but has the benefit that the developer doesn't have to worry
about quoting spawn arguments.

Closes #4282
  • Loading branch information
petebacondarwin committed Nov 15, 2023
1 parent be36619 commit c7d528d
Show file tree
Hide file tree
Showing 25 changed files with 149 additions and 176 deletions.
23 changes: 23 additions & 0 deletions .changeset/angry-walls-cheer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
"create-cloudflare": patch
---

fix: ensure shell scripts work on Windows

Our use of `shell-quote` was causing problems on Windows where it was
escaping character (such as `@`) by placing a backslash in front.
This made Windows think that such path arguments, were at the root.

For example, `npm install -D @cloudflare/workers-types` was being converted to
`npm install -D \@cloudflare/workers-types`, which resulted in errors like:

```
npm ERR! enoent ENOENT: no such file or directory, open 'D:\@cloudflare\workers-types\package.json'
```

Now we just rely directly on the Node.js `spawn` API to avoid any shell quoting
concerns. This has resulted in a slightly less streamlined experience for people
writing C3 plugins, but has the benefit that the developer doesn't have to worry
about quoting spawn arguments.

Closes https://github.com/cloudflare/workers-sdk/issues/4282
13 changes: 8 additions & 5 deletions packages/create-cloudflare/e2e-tests/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
beforeAll,
} from "vitest";
import { version } from "../package.json";
import * as shellquote from "../src/helpers/shell-quote";
import { frameworkToTest } from "./frameworkToTest";
import { isQuarantineMode, keys, recreateLogFolder, runC3 } from "./helpers";
import type { Suite } from "vitest";
Expand Down Expand Up @@ -43,15 +42,19 @@ describe.skipIf(frameworkToTest || isQuarantineMode())(
});

test("--version with positionals", async (ctx) => {
const argv = shellquote.parse("foo bar baz --version");
const argv = ["foo", "bar", "baz", "--version"];
const { output } = await runC3({ ctx, argv });
expect(output).toEqual(version);
});

test("--version with flags", async (ctx) => {
const argv = shellquote.parse(
"foo --type webFramework --no-deploy --version"
);
const argv = [
"foo",
"--type",
"webFramework",
"--no-deploy",
"--version",
];
const { output } = await runC3({ ctx, argv });
expect(output).toEqual(version);
});
Expand Down
2 changes: 0 additions & 2 deletions packages/create-cloudflare/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
"@types/esprima": "^4.0.3",
"@types/node": "^18.15.3",
"@types/semver": "^7.5.1",
"@types/shell-quote": "^1.7.2",
"@types/which-pm-runs": "^1.0.0",
"@types/yargs": "^17.0.22",
"@typescript-eslint/eslint-plugin": "^5.55.0",
Expand All @@ -72,7 +71,6 @@
"pnpm": "^8.10.0",
"recast": "^0.22.0",
"semver": "^7.5.1",
"shell-quote": "^1.8.1",
"typescript": "^5.0.2",
"undici": "5.20.0",
"vite-tsconfig-paths": "^4.0.8",
Expand Down
12 changes: 5 additions & 7 deletions packages/create-cloudflare/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { detectPackageManager } from "helpers/packages";
import semver from "semver";
import { version } from "../package.json";
import { validateProjectDirectory } from "./common";
import * as shellquote from "./helpers/shell-quote";
import { templateMap } from "./templateMap";
import type { C3Args } from "types";

Expand Down Expand Up @@ -43,7 +42,7 @@ const isUpdateAvailable = async () => {
const s = spinner(spinnerFrames.vertical, blue);
s.start("Checking if a newer version is available");
const latestVersion = await runCommand(
`npm info create-cloudflare@latest dist-tags.latest`,
["npm", "info", "create-cloudflare@latest", "dist-tags.latest"],
{ silent: true, useSpinner: false }
);
s.stop();
Expand All @@ -60,12 +59,11 @@ export const runLatest = async () => {

// the parsing logic of `npm create` requires `--` to be supplied
// before any flags intended for the target command.
const argString =
npm === "npm"
? `-- ${shellquote.quote(args)}`
: `${shellquote.quote(args)}`;
if (npm === "npm") {
args.unshift("--");
}

await runCommand(`${npm} create cloudflare@latest ${argString}`);
await runCommand([npm, "create", "cloudflare@latest", ...args]);
};

// Entrypoint to c3
Expand Down
26 changes: 14 additions & 12 deletions packages/create-cloudflare/src/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import { detectPackageManager } from "helpers/packages";
import { poll } from "helpers/poll";
import { version as wranglerVersion } from "wrangler/package.json";
import { version } from "../package.json";
import * as shellquote from "./helpers/shell-quote";
import type { C3Args, PagesGeneratorContext } from "types";

const { name, npm } = detectPackageManager();
Expand Down Expand Up @@ -192,7 +191,7 @@ export const runDeploy = async (ctx: PagesGeneratorContext) => {
env: { CLOUDFLARE_ACCOUNT_ID: ctx.account.id, NODE_ENV: "production" },
startText: "Deploying your application",
doneText: `${brandColor("deployed")} ${dim(
`via \`${shellquote.quote(baseDeployCmd)}\``
`via \`${baseDeployCmd.join(" ")}\``
)}`,
});

Expand Down Expand Up @@ -254,7 +253,7 @@ export const printSummary = async (ctx: PagesGeneratorContext) => {
const nextSteps = [
[
`Navigate to the new directory`,
`cd ${shellquote.quote([relative(ctx.originalCWD, ctx.project.path)])}`,
`cd ${relative(ctx.originalCWD, ctx.project.path)}`,
],
[
`Run the development server`,
Expand Down Expand Up @@ -378,7 +377,10 @@ export const gitCommit = async (ctx: PagesGeneratorContext) => {
await runCommands({
silent: true,
cwd: ctx.project.path,
commands: ["git add .", ["git", "commit", "-m", commitMessage]],
commands: [
["git", "add", "."],
["git", "commit", "-m", commitMessage],
],
startText: "Committing new files",
doneText: `${brandColor("git")} ${dim(`commit`)}`,
});
Expand Down Expand Up @@ -431,7 +433,7 @@ const createCommitMessage = async (ctx: PagesGeneratorContext) => {
*/
async function getGitVersion() {
try {
const rawGitVersion = await runCommand("git --version", {
const rawGitVersion = await runCommand(["git", "--version"], {
useSpinner: false,
silent: true,
});
Expand All @@ -452,13 +454,13 @@ export async function isGitInstalled() {

export async function isGitConfigured() {
try {
const userName = await runCommand("git config user.name", {
const userName = await runCommand(["git", "config", "user.name"], {
useSpinner: false,
silent: true,
});
if (!userName) return false;

const email = await runCommand("git config user.email", {
const email = await runCommand(["git", "config", "user.email"], {
useSpinner: false,
silent: true,
});
Expand All @@ -476,7 +478,7 @@ export async function isGitConfigured() {
*/
export async function isInsideGitRepo(cwd: string) {
try {
const output = await runCommand("git status", {
const output = await runCommand(["git", "status"], {
cwd,
useSpinner: false,
silent: true,
Expand All @@ -499,26 +501,26 @@ export async function initializeGit(cwd: string) {
try {
// Get the default init branch name
const defaultBranchName = await runCommand(
"git config --get init.defaultBranch",
["git", "config", "--get", "init.defaultBranch"],
{ useSpinner: false, silent: true, cwd }
);

// Try to create the repository with the HEAD branch of defaultBranchName ?? `main`.
await runCommand(
`git init --initial-branch ${defaultBranchName.trim() ?? "main"}`, // branch names can't contain spaces, so this is safe
["git", "init", "--initial-branch", defaultBranchName.trim() ?? "main"], // branch names can't contain spaces, so this is safe
{ useSpinner: false, silent: true, cwd }
);
} catch {
// Unable to create the repo with a HEAD branch name, so just fall back to the default.
await runCommand(`git init`, { useSpinner: false, silent: true, cwd });
await runCommand(["git", "init"], { useSpinner: false, silent: true, cwd });
}
}

export async function getProductionBranch(cwd: string) {
try {
const productionBranch = await runCommand(
// "git branch --show-current", // git@^2.22
"git rev-parse --abbrev-ref HEAD", // git@^1.6.3
["git", "rev-parse", "--abbrev-ref", "HEAD"], // git@^1.6.3
{
silent: true,
cwd,
Expand Down
4 changes: 2 additions & 2 deletions packages/create-cloudflare/src/frameworks/angular/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import type { FrameworkConfig, PagesGeneratorContext } from "types";
const { npm } = detectPackageManager();

const generate = async (ctx: PagesGeneratorContext) => {
await runFrameworkGenerator(ctx, `${ctx.project.name} --ssr`);
await runFrameworkGenerator(ctx, [ctx.project.name, "--ssr"]);

logRaw("");
};
Expand All @@ -35,7 +35,7 @@ const config: FrameworkConfig = {
}),
deployCommand: "deploy",
devCommand: "start",
testFlags: ["--ssr", "--style", "sass"],
testFlags: ["--style", "sass"],
};
export default config;

Expand Down
4 changes: 2 additions & 2 deletions packages/create-cloudflare/src/frameworks/astro/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type { FrameworkConfig, PagesGeneratorContext } from "types";
const { npx } = detectPackageManager();

const generate = async (ctx: PagesGeneratorContext) => {
await runFrameworkGenerator(ctx, `${ctx.project.name} --no-install`);
await runFrameworkGenerator(ctx, [ctx.project.name, "--no-install"]);

logRaw(""); // newline
};
Expand All @@ -20,7 +20,7 @@ const configure = async (ctx: PagesGeneratorContext) => {
// Need to ensure install first so `astro` works
await npmInstall();

await runCommand(`${npx} astro add cloudflare -y`, {
await runCommand([npx, "astro", "add", "cloudflare", "-y"], {
silent: true,
startText: "Installing adapter",
doneText: `${brandColor("installed")} ${dim(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type { PagesGeneratorContext, FrameworkConfig } from "types";
const { npm } = detectPackageManager();

const generate = async (ctx: PagesGeneratorContext) => {
await runFrameworkGenerator(ctx, `${ctx.project.name} classic`);
await runFrameworkGenerator(ctx, [ctx.project.name, "classic"]);
};

const config: FrameworkConfig = {
Expand Down
2 changes: 1 addition & 1 deletion packages/create-cloudflare/src/frameworks/gatsby/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const generate = async (ctx: PagesGeneratorContext) => {
});
}

await runFrameworkGenerator(ctx, `new ${ctx.project.name} ${templateUrl}`);
await runFrameworkGenerator(ctx, ["new", ctx.project.name, templateUrl]);
};

const config: FrameworkConfig = {
Expand Down
9 changes: 5 additions & 4 deletions packages/create-cloudflare/src/frameworks/hono/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ import { runFrameworkGenerator } from "helpers/command";
import type { FrameworkConfig, PagesGeneratorContext } from "types";

const generate = async (ctx: PagesGeneratorContext) => {
await runFrameworkGenerator(
ctx,
`${ctx.project.name} --template cloudflare-workers`
);
await runFrameworkGenerator(ctx, [
ctx.project.name,
"--template",
"cloudflare-workers",
]);

logRaw(""); // newline
};
Expand Down
2 changes: 1 addition & 1 deletion packages/create-cloudflare/src/frameworks/next/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const { npm, npx } = detectPackageManager();
const generate = async (ctx: PagesGeneratorContext) => {
const projectName = ctx.project.name;

await runFrameworkGenerator(ctx, `${projectName}`);
await runFrameworkGenerator(ctx, [projectName]);
};

const getApiTemplate = (
Expand Down
11 changes: 7 additions & 4 deletions packages/create-cloudflare/src/frameworks/nuxt/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,13 @@ const { npm } = detectPackageManager();
const generate = async (ctx: PagesGeneratorContext) => {
const gitFlag = ctx.args.git ? `--gitInit` : `--no-gitInit`;

await runFrameworkGenerator(
ctx,
`init ${ctx.project.name} --packageManager ${npm} ${gitFlag}`
);
await runFrameworkGenerator(ctx, [
"init",
ctx.project.name,
"--packageManager",
npm,
gitFlag,
]);

logRaw(""); // newline
};
Expand Down
6 changes: 3 additions & 3 deletions packages/create-cloudflare/src/frameworks/qwik/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type { FrameworkConfig, PagesGeneratorContext } from "types";
const { npm, npx } = detectPackageManager();

const generate = async (ctx: PagesGeneratorContext) => {
await runFrameworkGenerator(ctx, `basic ${ctx.project.name}`);
await runFrameworkGenerator(ctx, ["basic", ctx.project.name]);
};

const configure = async (ctx: PagesGeneratorContext) => {
Expand All @@ -16,8 +16,8 @@ const configure = async (ctx: PagesGeneratorContext) => {
await npmInstall();

// Add the pages integration
const cmd = `${npx} qwik add cloudflare-pages`;
endSection(`Running ${cmd}`);
const cmd = [npx, "qwik", "add", "cloudflare-pages"];
endSection(`Running ${cmd.join(" ")}`);
await runCommand(cmd);
};

Expand Down
2 changes: 1 addition & 1 deletion packages/create-cloudflare/src/frameworks/react/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type { FrameworkConfig, PagesGeneratorContext } from "types";
const { npm } = detectPackageManager();

const generate = async (ctx: PagesGeneratorContext) => {
await runFrameworkGenerator(ctx, `${ctx.project.name}`);
await runFrameworkGenerator(ctx, [ctx.project.name]);

logRaw("");
};
Expand Down
9 changes: 5 additions & 4 deletions packages/create-cloudflare/src/frameworks/remix/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ import type { FrameworkConfig, PagesGeneratorContext } from "types";
const { npm } = detectPackageManager();

const generate = async (ctx: PagesGeneratorContext) => {
await runFrameworkGenerator(
ctx,
`${ctx.project.name} --template https://github.com/remix-run/remix/tree/main/templates/cloudflare-pages`
);
await runFrameworkGenerator(ctx, [
ctx.project.name,
"--template",
"https://github.com/remix-run/remix/tree/main/templates/cloudflare-pages",
]);

logRaw(""); // newline
};
Expand Down
2 changes: 1 addition & 1 deletion packages/create-cloudflare/src/frameworks/solid/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const { npm } = detectPackageManager();

const generate = async (ctx: PagesGeneratorContext) => {
// Run the create-solid command
await runFrameworkGenerator(ctx, `${ctx.project.name}`);
await runFrameworkGenerator(ctx, [ctx.project.name]);

logRaw("");
};
Expand Down
2 changes: 1 addition & 1 deletion packages/create-cloudflare/src/frameworks/svelte/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import type { FrameworkConfig, PagesGeneratorContext } from "types";
const { npm } = detectPackageManager();

const generate = async (ctx: PagesGeneratorContext) => {
await runFrameworkGenerator(ctx, `${ctx.project.name}`);
await runFrameworkGenerator(ctx, [ctx.project.name]);

logRaw("");
};
Expand Down
2 changes: 1 addition & 1 deletion packages/create-cloudflare/src/frameworks/vue/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type { PagesGeneratorContext, FrameworkConfig } from "types";
const { npm } = detectPackageManager();

const generate = async (ctx: PagesGeneratorContext) => {
await runFrameworkGenerator(ctx, `${ctx.project.name}`);
await runFrameworkGenerator(ctx, [ctx.project.name]);
};

const config: FrameworkConfig = {
Expand Down
Loading

0 comments on commit c7d528d

Please sign in to comment.