clean up local testing paths in miniflare v5#14599
Conversation
🦋 Changeset detectedLatest commit: c4d58ed The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 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 |
|
✅ All changesets look good |
There was a problem hiding this comment.
I'm Bonk, and I've done a quick review of your PR.
This PR removes Miniflare's built-in live reload mechanism (the /cdn-cgi/mf/reload WebSocket endpoint, the liveReload option, and the HTML <script> injection), and drops the now-unused liveReload pass-through in Wrangler's Miniflare code paths. The Wrangler-side --live-reload flag still works via the ProxyWorker, which is correct.
One actionable issue:
- Stale public API docs —
packages/miniflare/README.md(lines 769-773) still documents theliveReload?: booleanoption that this PR removes. Since this is amajorbreaking change to Miniflare's public API, the README should be updated to drop this entry so users aren't pointed at a removed option. This file isn't part of the diff, so I can't attach an inline suggestion, but the block to delete is:
- `liveReload?: boolean`
If `true`, Miniflare will inject a script into HTML responses that
automatically reloads the page in-browser whenever the Miniflare instance's
options are updated.
Minor (non-blocking) note: loopbackPort remains in GlobalServicesOptions and is still passed by callers, but is no longer read inside getGlobalServices now that the live reload script (its only consumer) is gone. Safe to leave, but could be cleaned up if it's not needed elsewhere.
|
Review posted successfully to PR #14599. Summary of my review: The PR cleanly removes Miniflare's built-in live reload mechanism. I verified:
I flagged one actionable issue: the miniflare README still documents the removed |
@cloudflare/autoconfig
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
19196e2 to
f2691df
Compare
211dfb2 to
903e280
Compare
c56c406 to
cb00840
Compare
334b15b to
6f0faa5
Compare
| * | ||
| * These mirror the values in packages/miniflare/src/workers/core/constants.ts | ||
| * but cannot be imported directly due to circular dependency | ||
| * (miniflare depends on @cloudflare/local-explorer-ui). |
There was a problem hiding this comment.
Can Miniflare just import them from here?
| // ## Experimental Feature Flags | ||
|
|
||
| /** Enable the local explorer UI at /cdn-cgi/explorer (experimental, default: false). */ | ||
| /** Enable the local explorer UI at /cdn-cgi/local/explorer (experimental, default: false). */ |
There was a problem hiding this comment.
Is it still default:false?
There was a problem hiding this comment.
yep doing that in a separate change
Updates miniflare to use these paths:
/cdn-cgi/platform-proxy→/cdn-cgi/local/platform-proxy/cdn-cgi/handler/scheduled→/cdn-cgi/local/scheduled/cdn-cgi/handler/email→/cdn-cgi/local/email/cdn-cgi/explorer/*→/cdn-cgi/local/explorer/*/cdn-cgi/mf/scheduled→ removed (was already deprecated) from miniflare, redirects to/cdn-cgi/local/scheduledclient-side/cdn-cgi/mf/stream/*→/__cf_local/stream/*/cdn-cgi/mf/imagedelivery/*→/__cf_local/imagedelivery/*Vite and wrangler rewrite requests to the old paths, so there should be no breaking changes.
And removes
/cdn-cgi/mf/reload, which was not being used by vite or wrangler (they have their own live reload mechanisms).A picture of a cute animal (not mandatory, but encouraged)