Skip to content

Conversation

edmundhung
Copy link
Member

@edmundhung edmundhung commented Jun 11, 2025

Fixes DEVX-2120

This adds an asset proxy worker in front of the user worker when registering the worker in the dev registry which route the request through the vite dev server for assets.


  • Tests
    • TODO (before merge)
    • Tests included
    • Tests not necessary because:
  • Wrangler / Vite E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
    • I don't know
    • Required
    • Not required because:
  • Public documentation
    • TODO (before merge)
    • Cloudflare docs PR(s):
    • Documentation not necessary because: bugfix
  • Wrangler V3 Backport
    • TODO (before merge)
    • Wrangler PR:
    • Not necessary because: no impact to wrangler

Copy link

changeset-bot bot commented Jun 11, 2025

🦋 Changeset detected

Latest commit: b5a2904

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
@cloudflare/vite-plugin Patch
miniflare Minor
@cloudflare/pages-shared Patch
@cloudflare/vitest-pool-workers Patch
wrangler Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

pkg-pr-new bot commented Jun 11, 2025

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@9556

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@9556

miniflare

npm i https://pkg.pr.new/miniflare@9556

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@9556

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@9556

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@9556

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@9556

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@9556

wrangler

npm i https://pkg.pr.new/wrangler@9556

commit: b5a2904

@edmundhung edmundhung force-pushed the edmundhung/vite-registry-asset-proxy branch 2 times, most recently from d1884b9 to 5a59cca Compare July 23, 2025 11:15
@@ -118,6 +118,7 @@ const UnusableStringSchema = z.string().transform(() => undefined);
export const UnsafeDirectSocketSchema = z.object({
host: z.ostring(),
port: z.onumber(),
serviceName: z.ostring(),
Copy link
Member Author

Choose a reason for hiding this comment

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

The serviceName option will enable us to register a worker in the dev registry with the name of the current worker but pointed to a different service (i.e. The PRC Proxy worker in this case)

@edmundhung edmundhung force-pushed the edmundhung/vite-registry-asset-proxy branch from d7f5dfb to 0af7e1a Compare July 23, 2025 12:44
@edmundhung edmundhung changed the title fix(vite-plugin-cloudflare): setup asset proxy worker fix(vite-plugin-cloudflare): ensure cross-process service bindings route through asset worker Jul 23, 2025
@edmundhung edmundhung added the e2e Run wrangler + vite-plugin e2e tests on a PR label Jul 23, 2025
@edmundhung edmundhung marked this pull request as ready for review July 23, 2025 15:06
@edmundhung edmundhung requested review from a team as code owners July 23, 2025 15:06
Comment on lines 374 to 378
// Temporary workaround: the tail event is not serializable,
// so we are serializing it to JSON and parsing it back to make it transferable.
// This loses non-serializable data, but allows us to forward basic info
// to the target worker until native support is available.
return this.env.RPC_WORKER.tail(JSON.parse(JSON.stringify(event)));
Copy link
Member Author

@edmundhung edmundhung Jul 23, 2025

Choose a reason for hiding this comment

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

We are expecting native support from the runtime team when cloudflare/workerd#4595 is landed

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we call out the limitations in the changeset?

@edmundhung edmundhung requested review from jamesopstad and removed request for jamesopstad July 23, 2025 15:13
Copy link
Contributor

@jamesopstad jamesopstad left a comment

Choose a reason for hiding this comment

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

This is great! Thanks @edmundhung.

Comment on lines 374 to 378
// Temporary workaround: the tail event is not serializable,
// so we are serializing it to JSON and parsing it back to make it transferable.
// This loses non-serializable data, but allows us to forward basic info
// to the target worker until native support is available.
return this.env.RPC_WORKER.tail(JSON.parse(JSON.stringify(event)));
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we call out the limitations in the changeset?

@github-project-automation github-project-automation bot moved this from Untriaged to Approved in workers-sdk Jul 30, 2025
@edmundhung
Copy link
Member Author

Should we call out the limitations in the changeset?

I have reviewed the tail events and only the getUnredacted() API from the TraceItemFetchEventInfoRequest will be missing after serializing. As we are expecting native support soon, I think it should be fine not mentioning it to avoid confusing.

@jamesopstad
Copy link
Contributor

Thanks for the changes @edmundhung!

@edmundhung edmundhung merged commit 8ba7736 into main Aug 1, 2025
74 of 80 checks passed
@edmundhung edmundhung deleted the edmundhung/vite-registry-asset-proxy branch August 1, 2025 09:43
@github-project-automation github-project-automation bot moved this from Approved to Done in workers-sdk Aug 1, 2025
@jamesopstad jamesopstad changed the title fix(vite-plugin-cloudflare): ensure cross-process service bindings route through asset worker fix(vite-plugin-cloudflare): ensure cross-process service bindings route through assets Aug 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e2e Run wrangler + vite-plugin e2e tests on a PR
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants