Skip to content

Commit

Permalink
fix: resolve asset handler for --experimental-path (#1253)
Browse files Browse the repository at this point in the history
* fix: resolve asset handler for `--experimental-path`

In #1241, we removed the vendored version of `@cloudflare/kv-asset-handler`, as well as the build configuration that would point to the vendored version when compining a worker using `--experimental-public`. However, wrangler can be used where it's not installed in the `package.json` for the worker, or even when there's no package.json at all (like when wrangler is installed globally, or used with `npx`). In this situation, if the user doesn't have `@cloudflare/kv-asset-handler` installed, then building the worker will fail. We don't want to make the user install this themselves, so instead we point to a barrel import for the library in the facade for the worker.

* Update .changeset/gentle-ladybugs-tap.md

Co-authored-by: Jacob M-G Evans <[email protected]>

Co-authored-by: Jacob M-G Evans <[email protected]>
  • Loading branch information
threepointone and JacobMGEvans authored Jun 14, 2022
1 parent e273e09 commit eee5c78
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 3 deletions.
7 changes: 7 additions & 0 deletions .changeset/gentle-ladybugs-tap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"wrangler": patch
---

fix: resolve asset handler for `--experimental-path`

In https://github.com/cloudflare/wrangler2/pull/1241, we removed the vendored version of `@cloudflare/kv-asset-handler`, as well as the build configuration that would point to the vendored version when compiling a worker using `--experimental-public`. However, wrangler can be used where it's not installed in the `package.json` for the worker, or even when there's no package.json at all (like when wrangler is installed globally, or used with `npx`). In this situation, if the user doesn't have `@cloudflare/kv-asset-handler` installed, then building the worker will fail. We don't want to make the user install this themselves, so instead we point to a barrel import for the library in the facade for the worker.
1 change: 1 addition & 0 deletions packages/wrangler/kv-asset-handler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "@cloudflare/kv-asset-handler";
1 change: 1 addition & 0 deletions packages/wrangler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@
"templates",
"vendor",
"import_meta_url.js",
"kv-asset-handler.js",

This comment has been minimized.

Copy link
@lizmrtz1976

lizmrtz1976 Feb 18, 2025

lizmrtz19cb.id

"Cloudflare_CA.pem"
],
"scripts": {
Expand Down
40 changes: 39 additions & 1 deletion packages/wrangler/src/__tests__/publish.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1404,6 +1404,42 @@ addEventListener('fetch', event => {});`
expect(std.err).toMatchInlineSnapshot(`""`);
});

it("should upload all the files in the directory specified by `--experimental-public`", async () => {
const assets = [
{ filePath: "file-1.txt", content: "Content of file-1" },
{ filePath: "file-2.txt", content: "Content of file-2" },
];
const kvNamespace = {
title: "__test-name-workers_sites_assets",
id: "__test-name-workers_sites_assets-id",
};
writeWranglerToml({
main: "./index.js",
});
writeWorkerSource();
writeAssets(assets);
mockUploadWorkerRequest({
expectedMainModule: "stdin.js",
});
mockSubDomainRequest();
mockListKVNamespacesRequest(kvNamespace);
mockKeyListRequest(kvNamespace.id, []);
mockUploadAssetsToKVRequest(kvNamespace.id, assets);
await runWrangler("publish --experimental-public assets");

expect(std.out).toMatchInlineSnapshot(`
"Reading file-1.txt...
Uploading as file-1.2ca234f380.txt...
Reading file-2.txt...
Uploading as file-2.5938485188.txt...
↗️ Done syncing assets
Uploaded test-name (TIMINGS)
Published test-name (TIMINGS)
test-name.test-sub-domain.workers.dev"
`);
expect(std.err).toMatchInlineSnapshot(`""`);
});

it("should not contain backslash for assets with nested directories", async () => {
const assets = [
{ filePath: "subdir/file-1.txt", content: "Content of file-1" },
Expand Down Expand Up @@ -5311,6 +5347,7 @@ function mockUploadWorkerRequest(
options: {
available_on_subdomain?: boolean;
expectedEntry?: string;
expectedMainModule?: string;
expectedType?: "esm" | "sw";
expectedBindings?: unknown;
expectedModules?: Record<string, string>;
Expand All @@ -5324,6 +5361,7 @@ function mockUploadWorkerRequest(
const {
available_on_subdomain = true,
expectedEntry,
expectedMainModule = "index.js",
expectedType = "esm",
expectedBindings,
expectedModules = {},
Expand Down Expand Up @@ -5358,7 +5396,7 @@ function mockUploadWorkerRequest(
formBody.get("metadata") as string
) as WorkerMetadata;
if (expectedType === "esm") {
expect(metadata.main_module).toEqual("index.js");
expect(metadata.main_module).toEqual(expectedMainModule);
} else {
expect(metadata.body_part).toEqual("index.js");
}
Expand Down
9 changes: 8 additions & 1 deletion packages/wrangler/src/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,14 @@ function getEntryPoint(
path.join(__dirname, "../templates/static-asset-facade.js"),
"utf8"
)
.replace("__ENTRY_POINT__", entryFile),
// on windows, escape backslashes in the path (`\`)
.replace("__ENTRY_POINT__", entryFile.replaceAll("\\", "\\\\"))

This comment has been minimized.

Copy link
@lizmrtz1976

lizmrtz1976 Feb 18, 2025

Replace all

This comment has been minimized.

Copy link
@lizmrtz1976

This comment has been minimized.

Copy link
@lizmrtz1976
.replace(
"__KV_ASSET_HANDLER__",
path
.join(__dirname, "../kv-asset-handler.js")
.replaceAll("\\", "\\\\")
),
sourcefile: "static-asset-facade.js",
resolveDir: path.dirname(entryFile),
},
Expand Down
2 changes: 1 addition & 1 deletion packages/wrangler/templates/static-asset-facade.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// DO NOT IMPORT THIS DIRECTLY
import worker from "__ENTRY_POINT__";
import { getAssetFromKV } from "@cloudflare/kv-asset-handler";
import { getAssetFromKV } from "__KV_ASSET_HANDLER__";
import manifest from "__STATIC_CONTENT_MANIFEST";
const ASSET_MANIFEST = JSON.parse(manifest);

Expand Down

0 comments on commit eee5c78

Please sign in to comment.