fix: ensure that pinned versions are not converted to ranges when using npm (#10575)

Author: petebacondarwin

.changeset/cool-rocks-bet.md

@@ -0,0 +1,5 @@
+---
+"create-cloudflare": patch
+---
+
+fix: ensure that pinned versions are not converted to ranges when using npm

packages/create-cloudflare/src/helpers/__tests__/packages.test.ts

@@ -1,23 +1,33 @@
-import { existsSync } from "fs";
+import { existsSync } from "node:fs";
+import { resolve } from "node:path";
 import { runCommand } from "helpers/command";
 import { installPackages, installWrangler, npmInstall } from "helpers/packages";
 import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
 import whichPMRuns from "which-pm-runs";
 import { createTestContext } from "../../__tests__/helpers";
+import * as files from "../files";
 import { mockPackageManager } from "./mocks";
 
 vi.mock("fs");
 vi.mock("which-pm-runs");
 vi.mock("which-pm-runs");
 vi.mock("helpers/command");
+vi.mock("../files", () => ({
+	readJSON: vi.fn(),
+	writeJSON: vi.fn(),
+}));
+const mockReadJSON = vi.mocked(files.readJSON);
+const mockWriteJSON = vi.mocked(files.writeJSON);
 
 describe("Package Helpers", () => {
 	beforeEach(() => {
 		vi.mocked(whichPMRuns).mockReturnValue({ name: "npm", version: "8.3.1" });
 		vi.mocked(existsSync).mockImplementation(() => false);
 	});
 
-	afterEach(() => {});
+	afterEach(() => {
+		vi.clearAllMocks();
+	});
 
 	describe("npmInstall", () => {
 		test("npm", async () => {
@@ -62,13 +72,37 @@ describe("Package Helpers", () => {
 			"with $pm",
 			async ({ pm, initialArgs, additionalArgs }) => {
 				mockPackageManager(pm);
-				const packages = ["foo", "bar", "baz"];
+				mockReadJSON.mockReturnValue({
+					["dependencies"]: {
+						foo: "^1.0.0",
+						bar: "^2.0.0",
+						baz: "^1.2.3",
+					},
+				});
+				const packages = ["foo", "bar@latest", "[email protected]"];
 				await installPackages(packages);
 
 				expect(vi.mocked(runCommand)).toHaveBeenCalledWith(
 					[...initialArgs, ...packages, ...(additionalArgs ?? [])],
 					expect.anything(),
 				);
+
+				if (pm === "npm") {
+					// Check that package.json was updated for npm
+					expect(mockReadJSON).toHaveBeenCalledWith(
+						resolve(process.cwd(), "package.json"),
+					);
+					expect(mockWriteJSON).toHaveBeenCalledWith(
+						resolve(process.cwd(), "package.json"),
+						expect.objectContaining({
+							["dependencies"]: {
+								foo: "^1.0.0",
+								bar: "^2.0.0",
+								baz: "1.2.3",
+							},
+						}),
+					);
+				}
 			},
 		);
 
@@ -87,18 +121,47 @@ describe("Package Helpers", () => {
 			"with $pm (dev = true)",
 			async ({ pm, initialArgs, additionalArgs }) => {
 				mockPackageManager(pm);
-				const packages = ["foo", "bar", "baz"];
+				mockReadJSON.mockReturnValue({
+					["devDependencies"]: {
+						foo: "^1.0.0",
+						bar: "^2.0.0",
+						baz: "^1.2.3",
+					},
+				});
+				const packages = ["foo", "bar@latest", "[email protected]"];
 				await installPackages(packages, { dev: true });
 
 				expect(vi.mocked(runCommand)).toHaveBeenCalledWith(
 					[...initialArgs, ...packages, ...(additionalArgs ?? [])],
 					expect.anything(),
 				);
+
+				if (pm === "npm") {
+					// Check that package.json was updated for npm
+					expect(mockReadJSON).toHaveBeenCalledWith(
+						resolve(process.cwd(), "package.json"),
+					);
+					expect(mockWriteJSON).toHaveBeenCalledWith(
+						resolve(process.cwd(), "package.json"),
+						expect.objectContaining({
+							["devDependencies"]: {
+								foo: "^1.0.0",
+								bar: "^2.0.0",
+								baz: "1.2.3",
+							},
+						}),
+					);
+				}
 			},
 		);
 	});
 
 	test("installWrangler", async () => {
+		mockReadJSON.mockReturnValue({
+			["devDependencies"]: {
+				wrangler: "^4.0.0",
+			},
+		});
 		await installWrangler();
 
 		expect(vi.mocked(runCommand)).toHaveBeenCalledWith(

packages/create-cloudflare/src/helpers/packages.ts

@@ -1,10 +1,12 @@
+import assert from "assert";
 import { existsSync } from "fs";
 import path from "path";
 import { brandColor, dim } from "@cloudflare/cli/colors";
 import { fetch } from "undici";
 import { runCommand } from "./command";
+import { readJSON, writeJSON } from "./files";
 import { detectPackageManager } from "./packageManagers";
-import type { C3Context } from "types";
+import type { C3Context, PackageJson } from "types";
 
 type InstallConfig = {
 	startText?: string;
@@ -24,6 +26,10 @@ export const installPackages = async (
 	packages: string[],
 	config: InstallConfig = {},
 ) => {
+	if (packages.length === 0) {
+		return;
+	}
+
 	const { npm } = detectPackageManager();
 
 	let saveFlag;
@@ -61,6 +67,27 @@ export const installPackages = async (
 			silent: true,
 		},
 	);
+
+	if (npm === "npm") {
+		// Npm install will update the package.json with a hat-range rather than the exact version/range we asked for.
+		// So let's just fix that up now by rewriting the package.json.
+		const pkgJsonPath = path.join(process.cwd(), "package.json");
+		const pkgJson = readJSON(pkgJsonPath) as PackageJson;
+		const deps = config.dev ? pkgJson.devDependencies : pkgJson.dependencies;
+		assert(deps, "dependencies should be defined");
+		for (const pkg of packages) {
+			const versionMarker = pkg.lastIndexOf("@");
+			if (versionMarker > 0) {
+				// (if versionMarker was 0 then this would indicate a scoped package with no version)
+				const pkgName = pkg.slice(0, versionMarker);
+				const pkgVersion = pkg.slice(versionMarker + 1);
+				if (pkgVersion !== "latest") {
+					deps[pkgName] = pkgVersion;
+				}
+			}
+		}
+		writeJSON(pkgJsonPath, pkgJson);
+	}
 };
 
 /**