Skip to content

Commit

Permalink
fix: allow URL object to be passed to local fetch() calls (#1769)
Browse files Browse the repository at this point in the history
* fix: allow URL object to be passed to local `fetch()` calls

Due to the local fetch check, URL objects are wrongly not accepted by `fetch()` calls,
amend the `checkedFetch` function in order to allow such scenario (which
anyways works after deployment).

resolves #1588
  • Loading branch information
dario-piotrowicz authored and GregBrimble committed Jan 13, 2023
1 parent 3f82434 commit 217e0a3
Show file tree
Hide file tree
Showing 8 changed files with 27 additions and 17 deletions.
5 changes: 5 additions & 0 deletions .changeset/clever-timers-teach.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

chore: Rename `--bundle` to `--no-bundle` in Pages commands to make similar to Workers
5 changes: 5 additions & 0 deletions .changeset/warm-nails-clap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

allow `fetch()` calls locally to accept URL Objects
4 changes: 2 additions & 2 deletions fixtures/pages-workerjs-app/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ describe.concurrent("Pages _worker.js", () => {
).toThrowError();
});

it("should not throw an error when the _worker.js file imports something if --bundle is true", async ({
it("should not throw an error when the _worker.js file imports something if --no-bundle is false", async ({
expect,
}) => {
const { ip, port, stop } = await runWranglerPagesDev(
resolve(__dirname, ".."),
"./workerjs-test",
["--bundle"]
["--no-bundle=false", "--port=0"]
);
await expect(
fetch(`http://${ip}:${port}/`).then((resp) => resp.text())
Expand Down
4 changes: 3 additions & 1 deletion fixtures/shared/src/run-wrangler-long-lived.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,14 @@ async function runLongLivedWrangler(command: string[], cwd: string) {
"../../packages/wrangler/bin/wrangler.js",
command,
{
stdio: ["ignore", "ignore", "ignore", "ipc"],
stdio: ["pipe", "pipe", "pipe", "ipc"],
cwd,
}
).on("message", (message) => {
resolveReadyPromise(JSON.parse(message.toString()));
});
wranglerProcess.stderr?.on("data", (data) => console.log(`${data}`));
wranglerProcess.stdout?.on("data", (data) => console.log(`${data}`));

async function stop() {
return new Promise((resolve, reject) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/wrangler/src/__tests__/pages.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ describe("pages", () => {
--commit-message The commit message to attach to this deployment [string]
--commit-dirty Whether or not the workspace should be considered dirty for this deployment [boolean]
--skip-caching Skip asset caching which speeds up builds [boolean]
--bundle Whether to run bundling on \`_worker.js\` before deploying [boolean] [default: false]
--no-bundle Whether to run bundling on \`_worker.js\` before deploying [boolean] [default: true]
🚧 'wrangler pages <command>' is a beta command. Please report any issues to https://github.com/cloudflare/wrangler2/issues/new/choose"
`);
Expand Down
8 changes: 4 additions & 4 deletions packages/wrangler/src/pages/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ export function Options(yargs: Argv) {
description:
"The location of the single Worker script if not using functions",
},
bundle: {
"no-bundle": {
type: "boolean",
default: false,
default: true,
description: "Whether to run bundling on `_worker.js`",
},
binding: {
Expand Down Expand Up @@ -168,7 +168,7 @@ export const Handler = async ({
"inspector-port": inspectorPort,
proxy: requestedProxyPort,
"script-path": singleWorkerScriptPath,
bundle,
noBundle,
binding: bindings = [],
kv: kvs = [],
do: durableObjects = [],
Expand Down Expand Up @@ -264,7 +264,7 @@ export const Handler = async ({
await checkRawWorker(workerScriptPath, () => scriptReadyResolve());
};

if (bundle) {
if (!noBundle) {
// We want to actually run the `_worker.js` script through the bundler
// So update the final path to the script that will be uploaded and
// change the `runBuild()` function to bundle the `_worker.js`.
Expand Down
12 changes: 6 additions & 6 deletions packages/wrangler/src/pages/publish.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ export function Options(yargs: Argv) {
type: "boolean",
description: "Skip asset caching which speeds up builds",
},
bundle: {
"no-bundle": {
type: "boolean",
default: false,
default: true,
description: "Whether to run bundling on `_worker.js` before deploying",
},
config: {
Expand All @@ -85,7 +85,7 @@ export const Handler = async ({
commitMessage,
commitDirty,
skipCaching,
bundle,
noBundle,
config: wranglerConfig,
}: PublishArgs) => {
if (wranglerConfig) {
Expand Down Expand Up @@ -384,7 +384,9 @@ export const Handler = async ({
*/
if (_workerJS) {
let workerFileContents = _workerJS;
if (bundle) {
if (noBundle) {
await checkRawWorker(workerScriptPath, () => {});
} else {
const outfile = join(tmpdir(), `./bundledWorker-${Math.random()}.mjs`);
await buildRawWorker({
workerScriptPath,
Expand All @@ -396,8 +398,6 @@ export const Handler = async ({
onEnd: () => {},
});
workerFileContents = readFileSync(outfile, "utf8");
} else {
await checkRawWorker(workerScriptPath, () => {});
}

formData.append("_worker.js", new File([workerFileContents], "_worker.js"));
Expand Down
4 changes: 1 addition & 3 deletions packages/wrangler/templates/checked-fetch.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
const urls = new Set();

export function checkedFetch(request, init) {
const url = new URL(
(typeof request === "string" ? new Request(request, init) : request).url
);
const url = new URL(new Request(request, init).url);
if (url.port && url.port !== "443" && url.protocol === "https:") {
if (!urls.has(url.toString())) {
urls.add(url.toString());
Expand Down

0 comments on commit 217e0a3

Please sign in to comment.