Skip to content

Commit

Permalink
fix: fix dev and publish (#262)
Browse files Browse the repository at this point in the history
We introduced some bugs in recent PRs:

- In #196, we broke being able to pass an entrypoint directly to the cli. In this PR, I just reverted that fix. I'll reopen #78 and we'll tackle it again later. (cc @jgentes)
- In #215, we broke being able to publish a script by just passing `--latest` or `--compatibility-data` in the cli. This PR fixes that by reading the correct argument when choosing whether to publish.
- In #247, we broke how we made requests by passing headers to requests. This PR reverts the changes made in `cfetch/internal.ts`. (cc @petebacondarwin)
- In #244, we broke `dev` and it would immediately crash. This PR fixes the reference in `dev.tsx` that was breaking. (cc @petebacondarwin)
  • Loading branch information
threepointone committed Jan 18, 2022
1 parent 6c13ff9 commit 7494cf7
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 12 deletions.
12 changes: 12 additions & 0 deletions .changeset/strong-flies-add.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
"wrangler": patch
---

fix: fix `dev` and `publish`

We introduced some bugs in recent PRs

- In https://github.com/cloudflare/wrangler2/pull/196, we broke being able to pass an entrypoint directly to the cli. In this PR, I just reverted that fix. I'll reopen https://github.com/cloudflare/wrangler2/issues/78 and we'll tackle it again later. (cc @jgentes)
- In https://github.com/cloudflare/wrangler2/pull/215, we broke being able to publish a script by just passing `--latest` or `--compatibility-data` in the cli. This PR fixes that by reading the correct argument when choosing whether to publish.
- In https://github.com/cloudflare/wrangler2/pull/247, we broke how we made requests by passing headers to requests. This PR reverts the changes made in `cfetch/internal.ts`. (cc @petebacondarwin)
- In https://github.com/cloudflare/wrangler2/pull/244, we broke `dev` and it would immediately crash. This PR fixes the reference in `dev.tsx` that was breaking. (cc @petebacondarwin)
5 changes: 3 additions & 2 deletions packages/wrangler/src/__tests__/dev.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ describe("Dev component", () => {
accountId: "some-account-id",
public: "some/public/path",
});
expect(lastFrame()).toMatchInlineSnapshot(`
expect(lastFrame().split("\n").slice(0, 2).join("\n"))
.toMatchInlineSnapshot(`
"Something went wrong:
You cannot use the service worker format with a \`public\` directory."
Error: You cannot use the service worker format with a \`public\` directory."
`);
});
});
Expand Down
12 changes: 8 additions & 4 deletions packages/wrangler/src/cfetch/internal.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import fetch, { Headers } from "node-fetch";
import type { RequestInit } from "node-fetch";
import fetch from "node-fetch";
import type { RequestInit, HeadersInit } from "node-fetch";
import { getAPIToken, loginOrRefreshIfRequired } from "../user";

export const CF_API_BASE_URL =
Expand All @@ -21,7 +21,7 @@ export async function fetchInternal<ResponseType>(
): Promise<ResponseType> {
await requireLoggedIn();
const apiToken = requireApiToken();
const headers = new Headers(init.headers);
const headers = cloneHeaders(init.headers);
addAuthorizationHeader(headers, apiToken);

const queryString = queryParams ? `?${queryParams.toString()}` : "";
Expand All @@ -40,6 +40,10 @@ export async function fetchInternal<ResponseType>(
}
}

function cloneHeaders(headers: HeadersInit): HeadersInit {
return { ...headers };
}

async function requireLoggedIn(): Promise<void> {
const loggedIn = await loginOrRefreshIfRequired();
if (!loggedIn) {
Expand All @@ -55,7 +59,7 @@ function requireApiToken(): string {
return apiToken;
}

function addAuthorizationHeader(headers: Headers, apiToken: string): void {
function addAuthorizationHeader(headers: HeadersInit, apiToken: string): void {
if (headers["Authorization"]) {
throw new Error(
"The request already specifies an authorisation header - cannot add a new one."
Expand Down
10 changes: 6 additions & 4 deletions packages/wrangler/src/dev.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -484,9 +484,9 @@ function useEsbuild(props: {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const metafile = result.metafile!;
const outputEntry = Object.entries(metafile.outputs).find(
([_path, { entryPoint }]) =>
entryPoint === Object.keys(metafile.inputs)[0]
([_path, { entryPoint }]) => entryPoint === entry
); // assumedly only one entry point

if (outputEntry === undefined) {
throw new Error(
`Cannot find entry-point "${entry}" in generated bundle.`
Expand All @@ -508,7 +508,9 @@ function useEsbuild(props: {
// so this is a no-op error handler
});
return () => {
result.stop?.();
if (result && result.stop) {
result.stop();
}
};
}, [entry, destination, staticRoot, jsxFactory, jsxFragment]);
return bundle;
Expand Down Expand Up @@ -776,7 +778,7 @@ function ErrorFallback(props: { error: Error }) {
return (
<>
<Text>Something went wrong:</Text>
<Text>{props.error.message}</Text>
<Text>{props.error.stack}</Text>
</>
);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/wrangler/src/publish.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export default async function publish(props: Props): Promise<void> {
props.env && config.env ? config.env[props.env] || {} : config;

assert(
envRootObj.compatibility_date || props["compatibility-date"],
envRootObj.compatibility_date || props.compatibilityDate,
"A compatibility_date is required when publishing. Add one to your wrangler.toml file, or pass it in your terminal as --compatibility_date. See https://developers.cloudflare.com/workers/platform/compatibility-dates for more information."
);

Expand Down Expand Up @@ -139,7 +139,7 @@ export default async function publish(props: Props): Promise<void> {
const metafile = result.metafile!;
const expectedEntryPoint = props.public
? path.join(path.dirname(file), "static-asset-facade.js")
: Object.keys(metafile.inputs)[0];
: file;
const outputEntry = Object.entries(metafile.outputs).find(
([, { entryPoint }]) => entryPoint === expectedEntryPoint
);
Expand Down

0 comments on commit 7494cf7

Please sign in to comment.