Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: custom builds should allow multiple commands #311

Merged
merged 1 commit into from
Jan 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/ten-fireants-camp.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"wrangler": patch
---

fix: custom builds should allow multiple commands

We were running custom builds as a regular command with `execa`. This would fail whenever we tried to run compound commands like `cargo install -q worker-build && worker-build --release` (via https://github.com/cloudflare/wrangler2/issues/236). The fix is to use `shell: true`, so that the command is run in a shell and can thus use bash-y syntax like `&&`, and so on. I also switched to using `execaCommand` which splits a command string into parts correctly by itself.
31 changes: 31 additions & 0 deletions packages/wrangler/src/__tests__/publish.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -748,6 +748,37 @@ describe("publish", () => {
);
});
});

describe("custom builds", () => {
it("should run a custom build before publishing", async () => {
writeWranglerToml({
build: {
command: `echo "custom build" && echo "export default { fetch(){ return new Response(123)} }" > index.js`,
},
});

mockUploadWorkerRequest({
expectedBody: "return new Response(123)",
});
mockSubDomainRequest();

await runWrangler("publish index.js");
expect(stripTimings(std.out)).toMatchInlineSnapshot(`
"running:
echo \\"custom build\\" && echo \\"export default { fetch(){ return new Response(123)} }\\" > index.js
Uploaded
test-name
(TIMINGS)
Deployed
test-name
(TIMINGS)

test-name.test-sub-domain.workers.dev"
`);
expect(std.err).toMatchInlineSnapshot(`""`);
expect(std.warn).toMatchInlineSnapshot(`""`);
});
});
});

/** Write a mock wrangler.toml file to disk. */
Expand Down
13 changes: 7 additions & 6 deletions packages/wrangler/src/dev.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { Box, Text, useApp, useInput } from "ink";
import { watch } from "chokidar";
import clipboardy from "clipboardy";
import commandExists from "command-exists";
import { execa } from "execa";
import { execaCommand } from "execa";
import fetch from "node-fetch";
import open from "open";
import React, { useState, useEffect, useRef } from "react";
Expand Down Expand Up @@ -371,20 +371,21 @@ function useCustomBuild(
if (!command) return;
let cmd, interval;
console.log("running:", command);
const commandPieces = command.split(" ");
cmd = execa(commandPieces[0], commandPieces.slice(1), {
cmd = execaCommand(command, {
...(cwd && { cwd }),
shell: true,
stderr: "inherit",
stdout: "inherit",
});
if (watch_dir) {
watch(watch_dir, { persistent: true, ignoreInitial: true }).on(
"all",
(_event, _path) => {
console.log(`The file ${path} changed, restarting build...`);
(_event, filePath) => {
console.log(`The file ${filePath} changed, restarting build...`);
cmd.kill();
cmd = execa(commandPieces[0], commandPieces.slice(1), {
cmd = execaCommand(command, {
...(cwd && { cwd }),
shell: true,
stderr: "inherit",
stdout: "inherit",
});
Expand Down
6 changes: 3 additions & 3 deletions packages/wrangler/src/publish.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import path from "node:path";
import { readFile } from "node:fs/promises";
import * as esbuild from "esbuild";
import type { Metafile } from "esbuild";
import { execa } from "execa";
import { execaCommand } from "execa";
import tmp from "tmp-promise";
import type { CfWorkerInit } from "./api/worker";
import { toFormData } from "./api/form_data";
Expand Down Expand Up @@ -114,8 +114,8 @@ export default async function publish(props: Props): Promise<void> {
if (props.config.build?.command) {
// TODO: add a deprecation message here?
console.log("running:", props.config.build.command);
const buildCommandPieces = props.config.build.command.split(" ");
await execa(buildCommandPieces[0], buildCommandPieces.slice(1), {
await execaCommand(props.config.build.command, {
shell: true,
stdout: "inherit",
stderr: "inherit",
...(props.config.build?.cwd && { cwd: props.config.build.cwd }),
Expand Down