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: resolve raw file bindings correctly in wrangler dev local mode #756

Merged
merged 1 commit into from
Apr 4, 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
10 changes: 10 additions & 0 deletions .changeset/flat-ads-flash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"wrangler": patch
---

fix: resolve raw file bindings correctly in `wrangler dev` local mode

For `wasm_modules`/`text_blobs`/`data_blobs` in local mode, we need to rewrite the paths as absolute so that they're resolved correctly by miniflare. This also expands some coverage for local mode `wrangler dev`.

Fixes https://github.com/cloudflare/wrangler2/issues/740
Fixes https://github.com/cloudflare/wrangler2/issues/416
4 changes: 4 additions & 0 deletions examples/local-mode-tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,12 @@
"description": "",
"main": "index.js",
"scripts": {
"check:type": "tsc",
"test": "npx jest --forceExit"
},
"devDependencies": {
"@cloudflare/workers-types": "^3.2.0"
},
"keywords": [],
"author": "",
"license": "ISC",
Expand Down
1 change: 1 addition & 0 deletions examples/local-mode-tests/some-data.bin
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Here be some data
1 change: 1 addition & 0 deletions examples/local-mode-tests/some-text.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Here be some text
5 changes: 0 additions & 5 deletions examples/local-mode-tests/src/index.ts

This file was deleted.

23 changes: 23 additions & 0 deletions examples/local-mode-tests/src/module.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// @ts-expect-error non standard module
import data from "../some-data.bin";
// @ts-expect-error non standard module
import text from "../some-text.txt";

export default {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
async fetch(_request: Request, env: any): Promise<Response> {
return new Response(
JSON.stringify(
{
VAR1: env.VAR1,
VAR2: env.VAR2,
VAR3: env.VAR3,
text,
data: new TextDecoder().decode(data),
},
null,
2
)
);
},
};
31 changes: 31 additions & 0 deletions examples/local-mode-tests/src/sw.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// @ts-expect-error non standard module
import data from "../some-data.bin";
// @ts-expect-error non standard module
import text from "../some-text.txt";

addEventListener("fetch", (event: FetchEvent) => {
event.respondWith(handleRequest(event.request));
});

async function handleRequest(_req: Request): Promise<Response> {
return new Response(
JSON.stringify(
{
// @ts-expect-error binding
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yurgh I should declare an env.d.ts later/soon

VAR1,
// @ts-expect-error binding
VAR2,
// @ts-expect-error binding
VAR3,
text,
data: new TextDecoder().decode(data),
// @ts-expect-error binding
TEXT,
// @ts-expect-error binding
DATA: new TextDecoder().decode(DATA),
},
null,
2
)
);
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
name = "local-mode-tests"
main = "src/index.ts"
compatibility_date = "2022-03-27"

[vars]
VAR1 = "value1"
VAR2 = 123
VAR3 = {abc = "def"}
13 changes: 13 additions & 0 deletions examples/local-mode-tests/src/wrangler.sw.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
name = "local-mode-tests"
compatibility_date = "2022-03-27"

[vars]
VAR1 = "value1"
VAR2 = 123
VAR3 = {abc = "def"}

[text_blobs]
TEXT = "../some-text.txt"

[data_blobs]
DATA = "../some-data.bin"
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,23 @@ const isWindows = process.platform === "win32";
let wranglerProcess: ChildProcess;

beforeAll(async () => {
wranglerProcess = spawn("npx", ["wrangler", "dev", "--local"], {
shell: isWindows,
});
wranglerProcess = spawn(
"npx",
[
"wrangler",
"dev",
"src/module.ts",
"--local",
"--config",
"src/wrangler.module.toml",
"--port",
"9001",
petebacondarwin marked this conversation as resolved.
Show resolved Hide resolved
],
{
shell: isWindows,
stdio: "inherit",
}
);
});

afterAll(async () => {
Expand All @@ -40,7 +54,17 @@ afterAll(async () => {
});

it("renders", async () => {
const response = await waitUntilReady("http://localhost:8787/");
const response = await waitUntilReady("http://localhost:9001/");
const text = await response.text();
expect(text).toContain("Hello World!");
expect(text).toMatchInlineSnapshot(`
"{
\\"VAR1\\": \\"value1\\",
\\"VAR2\\": 123,
\\"VAR3\\": {
\\"abc\\": \\"def\\"
},
\\"text\\": \\"Here be some text\\",
\\"data\\": \\"Here be some data\\"
}"
`);
});
72 changes: 72 additions & 0 deletions examples/local-mode-tests/tests/sw.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { spawn } from "child_process";
import { fetch } from "undici";
import type { ChildProcess } from "child_process";
import type { Response } from "undici";

const waitUntilReady = async (url: string): Promise<Response> => {
let response: Response | undefined = undefined;

while (response === undefined) {
await new Promise((resolvePromise) => setTimeout(resolvePromise, 100));

try {
response = await fetch(url);
} catch {}
}

return response as Response;
};
const isWindows = process.platform === "win32";

let wranglerProcess: ChildProcess;

beforeAll(async () => {
wranglerProcess = spawn(
"npx",
[
"wrangler",
"dev",
"src/sw.ts",
"--local",
"--config",
"src/wrangler.sw.toml",
"--port",
"9002",
],
{
shell: isWindows,
stdio: "inherit",
}
);
});

afterAll(async () => {
await new Promise((resolve, reject) => {
wranglerProcess.once("exit", (code) => {
if (!code) {
resolve(code);
} else {
reject(code);
}
});
wranglerProcess.kill();
});
});

it("renders", async () => {
const response = await waitUntilReady("http://localhost:9002/");
const text = await response.text();
expect(text).toMatchInlineSnapshot(`
"{
\\"VAR1\\": \\"value1\\",
\\"VAR2\\": 123,
\\"VAR3\\": {
\\"abc\\": \\"def\\"
},
\\"text\\": \\"Here be some text\\",
\\"data\\": \\"Here be some data\\",
\\"TEXT\\": \\"Here be some text\\",
\\"DATA\\": \\"Here be some data\\"
}"
`);
});
1 change: 1 addition & 0 deletions examples/local-mode-tests/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"target": "esnext",
"strict": true,
"noEmit": true,
"types": ["@cloudflare/workers-types", "jest"],
"skipLibCheck": true
}
}
10 changes: 8 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 30 additions & 4 deletions packages/wrangler/src/dev/local.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,35 @@ function useLocalWorker({

const scriptPath = realpathSync(bundle.path);

const wasmBindings = { ...bindings.wasm_modules };
const textBlobBindings = { ...bindings.text_blobs };
const dataBlobBindings = { ...bindings.data_blobs };
// the wasm_modules/text_blobs/data_blobs bindings are
// relative to process.cwd(), but the actual worker bundle
// is in the temp output directory; so we rewrite the paths to be absolute,
// letting miniflare resolve them correctly
threepointone marked this conversation as resolved.
Show resolved Hide resolved

// wasm
const wasmBindings: Record<string, string> = {};
for (const [name, filePath] of Object.entries(
bindings.wasm_modules || {}
)) {
wasmBindings[name] = path.join(process.cwd(), filePath);
}

// text
const textBlobBindings: Record<string, string> = {};
for (const [name, filePath] of Object.entries(
bindings.text_blobs || {}
)) {
textBlobBindings[name] = path.join(process.cwd(), filePath);
}

// data
const dataBlobBindings: Record<string, string> = {};
for (const [name, filePath] of Object.entries(
bindings.data_blobs || {}
)) {
dataBlobBindings[name] = path.join(process.cwd(), filePath);
}

if (format === "service-worker") {
for (const { type, name } of bundle.modules) {
if (type === "compiled-wasm") {
Expand Down Expand Up @@ -187,7 +213,7 @@ function useLocalWorker({
});

local.current.stdout?.on("data", (data: Buffer) => {
console.log(`${data.toString()}`);
process.stdout.write(data);
threepointone marked this conversation as resolved.
Show resolved Hide resolved
});

local.current.stderr?.on("data", (data: Buffer) => {
Expand Down