Respond to PR feedback

cloudflare · Nov 7, 2023 · ece618d · ece618d
1 parent d539ad4
commit ece618d
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 15 deletions.
diff --git a/.changeset/dirty-bobcats-eat.md b/.changeset/dirty-bobcats-eat.md
@@ -2,4 +2,5 @@
 "create-cloudflare": patch
 ---
 
-Fixes c3 usage with yarn
+Changes c3 to use `npx` for running framework creation tools when it is invoked with `yarn`. This is
+needed since yarn can't `yarn create some-package@some-particular-version`.
diff --git a/packages/create-cloudflare/src/helpers/command.ts b/packages/create-cloudflare/src/helpers/command.ts
@@ -253,17 +253,33 @@ export const installPackages = async (
 	});
 };
 
-export const npmInstall = async () => {
+// Resets the package manager context for a project by clearing out existing dependencies
+// and lock files then re-installing.
+// This is needed in situations where npm is automatically used by framework creators for the initial
+// install, since using other package managers in a folder with an existing npm install will cause failures
+// when installing subsequent packages
+export const resetPackageManager = async (ctx: PagesGeneratorContext) => {
 	const { npm } = detectPackageManager();
 
+	if (!needsPackageManagerReset(ctx)) {
+		return;
+	}
+
+	const nodeModulesPath = path.join(ctx.project.path, "node_modules");
+	if (existsSync(nodeModulesPath)) rmSync(nodeModulesPath, { recursive: true });
+
+	const lockfilePath = path.join(ctx.project.path, "package-lock.json");
+	if (existsSync(lockfilePath)) rmSync(lockfilePath);
+
 	await runCommand(`${npm} install`, {
 		silent: true,
+		cwd: ctx.project.path,
 		startText: "Installing dependencies",
 		doneText: `${brandColor("installed")} ${dim(`via \`${npm} install\``)}`,
 	});
 };
 
-const needsReset = (ctx: PagesGeneratorContext) => {
+const needsPackageManagerReset = (ctx: PagesGeneratorContext) => {
 	const { npm } = detectPackageManager();
 	const projectPath = ctx.project.path;
 
@@ -279,22 +295,11 @@ const needsReset = (ctx: PagesGeneratorContext) => {
 	}
 };
 
-// Resets the package manager context for a project by clearing out existing dependencies
-// and lock files then re-installing.
-export const resetPackageManager = async (ctx: PagesGeneratorContext) => {
+export const npmInstall = async () => {
 	const { npm } = detectPackageManager();
 
-	if (!needsReset(ctx)) return;
-
-	const nodeModulesPath = path.join(ctx.project.path, "node_modules");
-	if (existsSync(nodeModulesPath)) rmSync(nodeModulesPath, { recursive: true });
-
-	const lockfilePath = path.join(ctx.project.path, "package-lock.json");
-	if (existsSync(lockfilePath)) rmSync(lockfilePath);
-
 	await runCommand(`${npm} install`, {
 		silent: true,
-		cwd: ctx.project.path,
 		startText: "Installing dependencies",
 		doneText: `${brandColor("installed")} ${dim(`via \`${npm} install\``)}`,
 	});