-
Notifications
You must be signed in to change notification settings - Fork 969
fix(vite-plugin-cloudflare): ensure cross-process service bindings route through assets #9556
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: b5a2904 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
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 |
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
wrangler
commit: |
d1884b9
to
5a59cca
Compare
@@ -118,6 +118,7 @@ const UnusableStringSchema = z.string().transform(() => undefined); | |||
export const UnsafeDirectSocketSchema = z.object({ | |||
host: z.ostring(), | |||
port: z.onumber(), | |||
serviceName: z.ostring(), |
There was a problem hiding this comment.
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)
d7f5dfb
to
0af7e1a
Compare
// 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))); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this 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.
// 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))); |
There was a problem hiding this comment.
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?
I have reviewed the tail events and only the |
Thanks for the changes @edmundhung! |
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.