Skip to content

Commit

Permalink
Deprecate pages dev's proxy features (#5317)
Browse files Browse the repository at this point in the history
  • Loading branch information
GregBrimble committed Mar 20, 2024
1 parent b73571f commit 9fd7eba
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 4 deletions.
7 changes: 7 additions & 0 deletions .changeset/tidy-dodos-pull.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"wrangler": patch
---

chore: Deprecate `-- <command>`, `--proxy` and `--script-path` options from `wrangler pages dev`.

Build your application to a directory and run the `wrangler pages dev <directory>` instead. This results in a more faithful emulation of production behavior.
11 changes: 10 additions & 1 deletion fixtures/pages-dev-proxy-with-script/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,18 @@ import { describe, expect, it } from "vitest";
describe("Pages dev with proxy and a script file", () => {
it("should handle requests using a script from the default _worker.js path", async () => {
const { port, childProcess } = await startWranglerPagesDevProxy();
let stderr = "";
childProcess.stderr?.on("data", (chunk) => {
stderr += chunk.toString();
});
const combinedResponse = await waitUntilReady(`http://127.0.0.1:${port}/`);
const respText = await combinedResponse.text();
expect(respText).toMatchInlineSnapshot('"hello from _worker.js"');
expect(
stderr.includes(
"Specifying a `-- <command>` or `--proxy` is deprecated and will be removed in a future version of Wrangler."
)
).toBeTruthy();
await terminateChildProcess(childProcess);
});

Expand Down Expand Up @@ -37,7 +46,7 @@ async function startWranglerPagesDevProxy(extraArgs: string[] = []): Promise<{
{
cwd: path.resolve(__dirname, ".."),
env: { BROWSER: "none", ...process.env },
stdio: ["ignore", "ignore", "ignore", "ipc"],
stdio: ["pipe", "pipe", "pipe", "ipc"],
}
).on("message", (message) => {
const parsedMessage = JSON.parse(message.toString());
Expand Down
41 changes: 41 additions & 0 deletions packages/wrangler/src/__tests__/pages/pages.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,47 @@ describe("pages", () => {
`);
});

describe("deprecation message for deprecated options", () => {
it("should display for 'pages dev -- <command>'", async () => {
await expect(
runWrangler("pages dev -- echo 'hi'")
).rejects.toThrowErrorMatchingInlineSnapshot(
`"Could not automatically determine proxy port. Please specify the proxy port with --proxy."`
);

expect(std.warn).toMatchInlineSnapshot(`
"▲ [WARNING] Specifying a \`-- <command>\` or \`--proxy\` is deprecated and will be removed in a future version of Wrangler.
Build your application to a directory and run the \`wrangler pages dev <directory>\` instead.
This results in a more faithful emulation of production behavior.
"
`);
});
it("should display for 'pages dev --script-path'", async () => {
await expect(
runWrangler("pages dev --script-path=_worker.js -- echo 'hi'")
).rejects.toThrowErrorMatchingInlineSnapshot(
`"Could not automatically determine proxy port. Please specify the proxy port with --proxy."`
);

expect(std.warn).toMatchInlineSnapshot(`
"▲ [WARNING] \`--script-path\` is deprecated and will be removed in a future version of Wrangler.
The Worker script should be named \`_worker.js\` and located in the build output directory of your
project (specified with \`wrangler pages dev <directory>\`).
▲ [WARNING] Specifying a \`-- <command>\` or \`--proxy\` is deprecated and will be removed in a future version of Wrangler.
Build your application to a directory and run the \`wrangler pages dev <directory>\` instead.
This results in a more faithful emulation of production behavior.
"
`);
});
});

describe("beta message for subcommands", () => {
it("should display for pages:dev", async () => {
await expect(
Expand Down
22 changes: 19 additions & 3 deletions packages/wrangler/src/pages/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ const SERVICE_BINDING_REGEXP = new RegExp(
/^(?<binding>[^=]+)=(?<service>[^@\s]+)(@(?<environment>.*)$)?$/
);

const DEFAULT_SCRIPT_PATH = "_worker.js";

export function Options(yargs: CommonYargsArgv) {
return yargs
.positional("directory", {
Expand All @@ -77,7 +79,7 @@ export function Options(yargs: CommonYargsArgv) {
.positional("command", {
type: "string",
demandOption: undefined,
description: "The proxy command to run",
description: "The proxy command to run [deprecated]", // no official way to deprecate a positional argument
})
.options({
local: {
Expand Down Expand Up @@ -121,12 +123,14 @@ export function Options(yargs: CommonYargsArgv) {
proxy: {
type: "number",
description: "The port to proxy (where the static assets are served)",
deprecated: true,
},
"script-path": {
type: "string",
default: "_worker.js",
description:
"The location of the single Worker script if not using functions",
"The location of the single Worker script if not using functions [default: _worker.js]",
// hacking in a fake default message here so we can detect when user is setting this and show a deprecation message
deprecated: true,
},
bundle: {
type: "boolean",
Expand Down Expand Up @@ -269,6 +273,14 @@ export const Handler = async ({
throw new FatalError("Pages does not support wrangler.toml", 1);
}

if (singleWorkerScriptPath !== undefined) {
logger.warn(
`\`--script-path\` is deprecated and will be removed in a future version of Wrangler.\nThe Worker script should be named \`_worker.js\` and located in the build output directory of your project (specified with \`wrangler pages dev <directory>\`).`
);
}

singleWorkerScriptPath ??= DEFAULT_SCRIPT_PATH;

const command = remaining;

let proxyPort: number | undefined;
Expand All @@ -279,6 +291,10 @@ export const Handler = async ({
1
);
} else if (directory === undefined) {
logger.warn(
`Specifying a \`-- <command>\` or \`--proxy\` is deprecated and will be removed in a future version of Wrangler.\nBuild your application to a directory and run the \`wrangler pages dev <directory>\` instead.\nThis results in a more faithful emulation of production behavior.`
);

proxyPort = await spawnProxyProcess({
port: requestedProxyPort,
command,
Expand Down

0 comments on commit 9fd7eba

Please sign in to comment.