Skip to content

Commit

Permalink
fix: allow assets in multiworker setup (#7290)
Browse files Browse the repository at this point in the history
* multidev assets

* assert rpc and named entrypoints do not work

* fixups

* fixup bindings earlier

* aaaaargh

* don't touch sockets
  • Loading branch information
emily-shen authored Feb 7, 2025
1 parent 7c05b1b commit 0c0374c
Show file tree
Hide file tree
Showing 5 changed files with 391 additions and 19 deletions.
10 changes: 10 additions & 0 deletions .changeset/poor-otters-drop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"miniflare": patch
"wrangler": patch
---

fix: add support for workers with assets when running multiple workers in one `wrangler dev` instance

https://github.com/cloudflare/workers-sdk/pull/7251 added support for running multiple Workers in one `wrangler dev`/miniflare session. e.g. `wrangler dev -c wrangler.toml -c ../worker2/wrangler.toml`, which among other things, allowed cross-service RPC to Durable Objects.

However this did not work in the same way as production when there was a Worker with assets - this PR should fix that.
31 changes: 21 additions & 10 deletions packages/miniflare/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1130,14 +1130,6 @@ export class Miniflare {
innerBindings: Worker_Binding[];
}[] = [];

if (this.#workerOpts[0].assets.assets) {
// This will be the UserWorker, or the vitest pool worker wrapping the UserWorker
// The asset plugin needs this so that it can set the binding between the RouterWorker and the UserWorker
// TODO: apply this to ever this.#workerOpts, not just the first (i.e this.#workerOpts[0])
this.#workerOpts[0].assets.assets.workerName =
this.#workerOpts[0].core.name;
}

for (let i = 0; i < allWorkerOpts.length; i++) {
const previousWorkerOpts = allPreviousWorkerOpts?.[i];
const workerOpts = allWorkerOpts[i];
Expand All @@ -1152,6 +1144,12 @@ export class Miniflare {
}
}

if (workerOpts.assets.assets) {
// This will be the UserWorker, or the vitest pool worker wrapping the UserWorker
// The asset plugin needs this so that it can set the binding between the RouterWorker and the UserWorker
workerOpts.assets.assets.workerName = workerOpts.core.name;
}

// Collect all bindings from this worker
const workerBindings: Worker_Binding[] = [];
allWorkerBindings.set(workerName, workerBindings);
Expand Down Expand Up @@ -1202,6 +1200,20 @@ export class Miniflare {
});
}
}
if ("service" in binding) {
const targetWorkerName = binding.service?.name?.replace(
"core:user:",
""
);
const maybeAssetTargetService = allWorkerOpts.find(
(worker) =>
worker.core.name === targetWorkerName && worker.assets.assets
);
if (maybeAssetTargetService) {
assert(binding.service?.name);
binding.service.name = `${ROUTER_SERVICE_NAME}:${targetWorkerName}`;
}
}
}
}
}
Expand Down Expand Up @@ -1305,7 +1317,7 @@ export class Miniflare {
!this.#workerOpts[0].core.name?.startsWith(
"vitest-pool-workers-runner-"
)
? ROUTER_SERVICE_NAME
? `${ROUTER_SERVICE_NAME}:${this.#workerOpts[0].core.name}`
: getUserServiceName(this.#workerOpts[0].core.name),
loopbackPort,
log: this.#log,
Expand Down Expand Up @@ -1340,7 +1352,6 @@ export class Miniflare {
"Ensure wrapped bindings don't have bindings to themselves."
);
}

return { services: servicesArray, sockets, extensions };
}

Expand Down
22 changes: 15 additions & 7 deletions packages/miniflare/src/plugins/assets/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ export const ASSETS_PLUGIN: Plugin<typeof AssetsOptionsSchema> = {
{
// binding between User Worker and Asset Worker
name: options.assets.binding,
service: { name: ASSETS_SERVICE_NAME },
service: {
name: `${ASSETS_SERVICE_NAME}:${options.assets.workerName}`,
},
},
];
},
Expand Down Expand Up @@ -68,8 +70,10 @@ export const ASSETS_PLUGIN: Plugin<typeof AssetsOptionsSchema> = {
options.assets.directory
);

const id = options.assets.workerName;

const namespaceService: Service = {
name: ASSETS_KV_SERVICE_NAME,
name: `${ASSETS_KV_SERVICE_NAME}:${id}`,
worker: {
compatibilityDate: "2023-07-24",
compatibilityFlags: ["nodejs_compat"],
Expand All @@ -93,7 +97,7 @@ export const ASSETS_PLUGIN: Plugin<typeof AssetsOptionsSchema> = {
};

const assetService: Service = {
name: ASSETS_SERVICE_NAME,
name: `${ASSETS_SERVICE_NAME}:${id}`,
worker: {
compatibilityDate: "2024-08-01",
modules: [
Expand All @@ -105,7 +109,9 @@ export const ASSETS_PLUGIN: Plugin<typeof AssetsOptionsSchema> = {
bindings: [
{
name: "ASSETS_KV_NAMESPACE",
kvNamespace: { name: ASSETS_KV_SERVICE_NAME },
kvNamespace: {
name: `${ASSETS_KV_SERVICE_NAME}:${id}`,
},
},
{
name: "ASSETS_MANIFEST",
Expand All @@ -120,7 +126,7 @@ export const ASSETS_PLUGIN: Plugin<typeof AssetsOptionsSchema> = {
};

const routerService: Service = {
name: ROUTER_SERVICE_NAME,
name: `${ROUTER_SERVICE_NAME}:${id}`,
worker: {
compatibilityDate: "2024-08-01",
modules: [
Expand All @@ -132,11 +138,13 @@ export const ASSETS_PLUGIN: Plugin<typeof AssetsOptionsSchema> = {
bindings: [
{
name: "ASSET_WORKER",
service: { name: ASSETS_SERVICE_NAME },
service: {
name: `${ASSETS_SERVICE_NAME}:${id}`,
},
},
{
name: "USER_WORKER",
service: { name: getUserServiceName(options.assets.workerName) },
service: { name: getUserServiceName(id) },
},
{
name: "CONFIG",
Expand Down
2 changes: 1 addition & 1 deletion packages/miniflare/src/plugins/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ function getCustomServiceDesignator(
} else if (service === kCurrentWorker) {
// Sets SELF binding to point to router worker instead if assets are present.
serviceName = hasAssetsAndIsVitest
? ROUTER_SERVICE_NAME
? `${ROUTER_SERVICE_NAME}:${refererName}`
: getUserServiceName(refererName);
} else {
// Regular user worker
Expand Down
Loading

0 comments on commit 0c0374c

Please sign in to comment.