Skip to content

Add support for worker connect handler in miniflare - #12775

Merged
fhanau merged 1 commit into
mainfrom
felix/030526-connect-handler
Mar 11, 2026
Merged

Add support for worker connect handler in miniflare#12775
fhanau merged 1 commit into
mainfrom
felix/030526-connect-handler

Conversation

@fhanau

@fhanau fhanau commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

This is needed to add initial support for the connect handler in workerd (experimental-only for now).

Part of EW-9330. Updates miniflare so that the connect handler is marked as reserved. This is needed for cloudflare/workerd#6059 to land, additional changes are included in #12802 which needs the workerd PR to be merged first. This PR should be sufficient for allowing the workerd PR to land: Reordering the steps in workerd CI shows that while miniflare tests are failing, vite and vitest tests are not so this one change should be sufficient to allow merging the workerd PR.


  • Tests
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: Feature is behind experimental compat flag for now.

Open with Devin

@changeset-bot

changeset-bot Bot commented Mar 5, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: e3d301d

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

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

Comment thread packages/vite-plugin-cloudflare/src/workers/runner-worker/keys.ts Outdated
@pkg-pr-new

pkg-pr-new Bot commented Mar 6, 2026

Copy link
Copy Markdown
create-cloudflare

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

@cloudflare/kv-asset-handler

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

miniflare

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

@cloudflare/pages-shared

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

@cloudflare/unenv-preset

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

@cloudflare/vite-plugin

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

@cloudflare/vitest-pool-workers

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

@cloudflare/workers-editor-shared

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

wrangler

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

commit: 103460e

@fhanau
fhanau force-pushed the felix/030526-connect-handler branch 2 times, most recently from e2dff1c to 270bd12 Compare March 7, 2026 14:23
@fhanau
fhanau marked this pull request as ready for review March 7, 2026 20:12
@fhanau
fhanau requested a review from a team as a code owner March 7, 2026 20:12
@workers-devprod

workers-devprod commented Mar 7, 2026

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • ✅ @cloudflare/wrangler
Show detailed file reviewers

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

@fhanau fhanau added the ci:no-changeset-required Skip pull request checks for a valid changeset label Mar 9, 2026

@jamesopstad jamesopstad left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please could you add a changeset. It can just describe the change very briefly.

@fhanau

fhanau commented Mar 9, 2026

Copy link
Copy Markdown
Contributor Author

Please could you add a changeset. It can just describe the change very briefly.

This is a one-line change for a test to enable a feature in another repo that won't be available externally for now. I can add a change set, but wouldn't this be the kind of PR that doesn't require one?

@jamesopstad

Copy link
Copy Markdown
Contributor

Please could you add a changeset. It can just describe the change very briefly.

This is a one-line change for a test to enable a feature in another repo that won't be available externally for now. I can add a change set, but wouldn't this be the kind of PR that doesn't require one?

What do you mean about it being a "change for a test"? CREATE_PROXY_PROTOTYPE_CLASS_HELPER_SCRIPT isn't only used for testing, or am I misunderstanding?

We generally add changesets for any change that should trigger a version bump and release.

@fhanau fhanau changed the title Add support for connect handler in miniflare tests Add support for worker connect handler in miniflare Mar 10, 2026
This is needed to add initial support for the connect handler in workerd
(experimental-only for now).
@fhanau
fhanau force-pushed the felix/030526-connect-handler branch from b80577f to e3d301d Compare March 10, 2026 16:12
@fhanau

fhanau commented Mar 10, 2026

Copy link
Copy Markdown
Contributor Author

Right, the change here isn't test-only, that's case with the follow-up change in #12802. Corrected the PR and commit title and added a change set.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Issues found

File: .changeset/ten-dancers-know.md

  1. Missing body: The changeset contains only a title with no body. The README requires "one or more paragraphs explaining the reason for the change and anything notable about the approach." A description like "Add support for worker connect handler in miniflare" tells users what changed but not why it was needed, how it works, or what users can do with it.

  2. Version type (minor consideration): The description says "Add support for..." which indicates new functionality. Per the README, new features should be minor. If this is adding a new capability to miniflare (e.g., implementing a previously unsupported Workers runtime handler), it likely warrants minor rather than patch. Please confirm whether this is a bug fix/alignment with existing behavior (patch) or genuinely new functionality (minor).

@fhanau

fhanau commented Mar 10, 2026

Copy link
Copy Markdown
Contributor Author

⚠️ Issues found

File: .changeset/ten-dancers-know.md

  1. Missing body: The changeset contains only a title with no body. The README requires "one or more paragraphs explaining the reason for the change and anything notable about the approach." A description like "Add support for worker connect handler in miniflare" tells users what changed but not why it was needed, how it works, or what users can do with it.
  2. Version type (minor consideration): The description says "Add support for..." which indicates new functionality. Per the README, new features should be minor. If this is adding a new capability to miniflare (e.g., implementing a previously unsupported Workers runtime handler), it likely warrants minor rather than patch. Please confirm whether this is a bug fix/alignment with existing behavior (patch) or genuinely new functionality (minor).
  • missing body? There is precedent for having a one-line change set description, see e.g .changeset/late-humans-win.md.
  • This does not introduce any new user-facing functionality since connect() will be behind the "experimental" compat flag, so patch should be appropriate.

@github-project-automation github-project-automation Bot moved this from Untriaged to Approved in workers-sdk Mar 10, 2026
@fhanau
fhanau merged commit 1dda1c8 into main Mar 11, 2026
48 of 58 checks passed
@fhanau
fhanau deleted the felix/030526-connect-handler branch March 11, 2026 13:24
@github-project-automation github-project-automation Bot moved this from Approved to Done in workers-sdk Mar 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:no-changeset-required Skip pull request checks for a valid changeset

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants