Skip to content

Commit

Permalink
refactor: use helpers to manage npm commands
Browse files Browse the repository at this point in the history
This also speeds up tests and avoids us checking that npm did what it is supposed to do.
  • Loading branch information
petebacondarwin committed Feb 1, 2022
1 parent e1d2198 commit ac168f4
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 40 deletions.
7 changes: 7 additions & 0 deletions .changeset/pretty-flowers-bathe.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"wrangler": patch
---

refactor: use helpers to manage npm commands

This change speeds up tests and avoids us checking that npm did what it is supposed to do.
27 changes: 11 additions & 16 deletions packages/wrangler/src/__tests__/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import * as fs from "node:fs";
import * as fsp from "node:fs/promises";
import * as TOML from "@iarna/toml";
import { version as wranglerVersion } from "../../package.json";
import { npm } from "../npm-installer";
import { mockConsoleMethods } from "./helpers/mock-console";
import { mockConfirm } from "./helpers/mock-dialogs";
import { runInTempDir } from "./helpers/run-in-tmp";
Expand All @@ -9,6 +11,11 @@ import { runWrangler } from "./helpers/run-wrangler";
describe("wrangler", () => {
runInTempDir();

beforeEach(() => {
jest.spyOn(npm, "addDevDep").mockResolvedValue();
jest.spyOn(npm, "install").mockResolvedValue();
});

const std = mockConsoleMethods();

describe("no command", () => {
Expand Down Expand Up @@ -153,6 +160,7 @@ describe("wrangler", () => {
wrangler: expect.any(String),
});
expect(fs.existsSync("./tsconfig.json")).toBe(false);
expect(npm.install).toHaveBeenCalled();
});

it("should not touch an existing package.json in the same directory", async () => {
Expand Down Expand Up @@ -205,9 +213,7 @@ describe("wrangler", () => {
);
expect(packageJson.name).toEqual("test");
expect(packageJson.version).toEqual("1.0.0");
expect(packageJson.devDependencies).toEqual({
wrangler: expect.any(String),
});
expect(npm.addDevDep).toHaveBeenCalledWith("wrangler", wranglerVersion);
});

it("should not touch an existing package.json in an ancestor directory", async () => {
Expand Down Expand Up @@ -265,13 +271,7 @@ describe("wrangler", () => {
expect(tsconfigJson.compilerOptions.types).toEqual([
"@cloudflare/workers-types",
]);
const packageJson = JSON.parse(
fs.readFileSync("./package.json", "utf-8")
);
expect(packageJson.devDependencies).toEqual({
"@cloudflare/workers-types": expect.any(String),
wrangler: expect.any(String),
});
expect(npm.addDevDep).toHaveBeenCalledWith("@cloudflare/workers-types");
});

it("should not touch an existing tsconfig.json in the same directory", async () => {
Expand Down Expand Up @@ -331,12 +331,7 @@ describe("wrangler", () => {
);
// unchanged tsconfig
expect(tsconfigJson.compilerOptions).toEqual({});
const packageJson = JSON.parse(
fs.readFileSync("./package.json", "utf-8")
);
expect(packageJson.devDependencies).toEqual({
"@cloudflare/workers-types": expect.any(String),
});
expect(npm.addDevDep).toHaveBeenCalledWith("@cloudflare/workers-types");
});

it("should not touch an existing tsconfig.json in an ancestor directory", async () => {
Expand Down
30 changes: 6 additions & 24 deletions packages/wrangler/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { readFile, writeFile } from "node:fs/promises";
import path from "node:path/posix";
import { setTimeout } from "node:timers/promises";
import TOML from "@iarna/toml";
import { execa } from "execa";
import { findUp } from "find-up";
import { render } from "ink";
import React from "react";
Expand All @@ -25,6 +24,7 @@ import {
createNamespace,
isValidNamespaceBinding,
} from "./kv";
import { npm } from "./npm-installer";
import { pages } from "./pages";
import publish from "./publish";
import { getAssetPaths } from "./sites";
Expand All @@ -38,8 +38,8 @@ import {
getAccountId,
validateScopeKeys,
} from "./user";

import { whoami } from "./whoami";

import type { Config } from "./config";
import type Yargs from "yargs";

Expand Down Expand Up @@ -234,9 +234,7 @@ export async function main(argv: string[]): Promise<void> {
" "
) + "\n"
);
await execa("npm", ["install"], {
stdio: "inherit",
});
await npm.install();
console.log(`✨ Created package.json`);
pathToPackageJson = path.join(process.cwd(), "package.json");
} else {
Expand All @@ -258,13 +256,7 @@ export async function main(argv: string[]): Promise<void> {
"Would you like to install wrangler into your package.json?"
);
if (shouldInstall) {
await execa(
"npm",
["install", `wrangler@${wranglerVersion}`, "--save-dev"],
{
stdio: "inherit",
}
);
await npm.addDevDep("wrangler", wranglerVersion);
console.log(`✨ Installed wrangler`);
}
}
Expand Down Expand Up @@ -298,11 +290,7 @@ export async function main(argv: string[]): Promise<void> {
" "
) + "\n"
);
await execa(
"npm",
["install", "@cloudflare/workers-types", "--save-dev"],
{ stdio: "inherit" }
);
await npm.addDevDep("@cloudflare/workers-types");
console.log(
`✨ Created tsconfig.json, installed @cloudflare/workers-types into devDependencies`
);
Expand All @@ -324,13 +312,7 @@ export async function main(argv: string[]): Promise<void> {
"Would you like to install the type definitions for Workers into your package.json?"
);
if (shouldInstall) {
await execa(
"npm",
["install", "@cloudflare/workers-types", "--save-dev"],
{
stdio: "inherit",
}
);
await npm.addDevDep("@cloudflare/workers-types");
// We don't update the tsconfig.json because
// it could be complicated in existing projects
// and we don't want to break them. Instead, we simply
Expand Down
24 changes: 24 additions & 0 deletions packages/wrangler/src/npm-installer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { execa } from "execa";

/**
* Helpers for running npm commands
*/
export const npm = {
/** Add and install a new devDependency into the local package.json. */
async addDevDep(packageName: string, versionRange = "latest"): Promise<void> {
await execa(
"npm",
["install", `${packageName}@${versionRange}`, "--save-dev"],
{
stdio: "inherit",
}
);
},

/** Install all the dependencies in the local package.json. */
async install(): Promise<void> {
await execa("npm", ["install"], {
stdio: "inherit",
});
},
};

0 comments on commit ac168f4

Please sign in to comment.