Skip to content

refactor(miniflare): use CorePaths for local explorer routes#13090

Merged
petebacondarwin merged 1 commit intomainfrom
edmundhung/remove-explorer-path-aliases
Mar 30, 2026
Merged

refactor(miniflare): use CorePaths for local explorer routes#13090
petebacondarwin merged 1 commit intomainfrom
edmundhung/remove-explorer-path-aliases

Conversation

@edmundhung
Copy link
Copy Markdown
Member

@edmundhung edmundhung commented Mar 27, 2026

Fixes #12686 (comment).


  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because: covered by existing tests
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: internal refactor

A picture of a cute animal (not mandatory, but encouraged)


Open with Devin

@edmundhung edmundhung requested a review from a team as a code owner March 27, 2026 14:24
@edmundhung edmundhung requested a review from NuroDev March 27, 2026 14:25
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 27, 2026

🦋 Changeset detected

Latest commit: 98934ce

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

@workers-devprod
Copy link
Copy Markdown
Contributor

workers-devprod commented Mar 27, 2026

Codeowners approval required for this PR:

  • ✅ @cloudflare/wrangler
Show detailed file reviewers

@github-actions
Copy link
Copy Markdown
Contributor

⚠️ Issues found

File: .changeset/warm-foxes-shave.md

Issue: Changeset may not be required

The README states that "Changes that are purely internal refactoring with no user-facing impact" do not require a changeset. The description explicitly says these were internal constants ("All internal consumers now use CorePaths.EXPLORER directly"), suggesting they were never part of the public API. If LOCAL_EXPLORER_BASE_PATH and LOCAL_EXPLORER_API_PATH were not exported from the miniflare package's public entry point, this changeset should be removed rather than included.

If they were part of the public API (i.e., exported and usable by external consumers), then the changeset is warranted — but the version type should likely be minor (not patch), since removing exported symbols is a breaking change. Per the README, breaking changes to experimental/beta features can use minor rather than major, but the version type should reflect that impact.

Issue: Changelog description quality

The current description is written from an internal implementation perspective ("redundant aliases introduced before CorePaths was centralized"). A changelog entry should describe the user-facing impact. If this change does belong in the changelog, the description should clarify:

  • Whether these were public API symbols
  • What users (if any) need to do in response (e.g., migrate from the removed constants to CorePaths.EXPLORER)

Recommendation:

  • If internal-only: remove the changeset entirely.
  • If public API: update the version type to minor and rewrite the description to explain the migration path for consumers who used these constants.

Copy link
Copy Markdown
Contributor

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

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

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Mar 27, 2026

UnknownError: "Internal server error"

github run

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Mar 27, 2026

@edmundhung Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Mar 27, 2026

create-cloudflare

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

@cloudflare/kv-asset-handler

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

miniflare

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

@cloudflare/pages-shared

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

@cloudflare/unenv-preset

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

@cloudflare/vite-plugin

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

@cloudflare/vitest-pool-workers

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

@cloudflare/workers-editor-shared

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

wrangler

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

commit: 98934ce

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Mar 27, 2026

UnknownError: "Internal server error"

github run

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Mar 27, 2026

The tests import CorePaths from the source path ../../../src/workers/core/constants, while the fixture test imports it from the public miniflare package. Both are correct.

Let me now assess the changeset concern. Since LOCAL_EXPLORER_BASE_PATH and LOCAL_EXPLORER_API_PATH were publicly exported from the miniflare package but were never actually released (the restructuring that added them to the export list happened 2 days ago and no miniflare release has occurred since), this is effectively a pre-release change. The patch level is reasonable since these symbols never shipped in a published version.

LGTM

github run

@github-project-automation github-project-automation bot moved this from Untriaged to Approved in workers-sdk Mar 27, 2026
@petebacondarwin petebacondarwin merged commit a532eea into main Mar 30, 2026
51 of 56 checks passed
@petebacondarwin petebacondarwin deleted the edmundhung/remove-explorer-path-aliases branch March 30, 2026 13:24
@github-project-automation github-project-automation bot moved this from Approved to Done in workers-sdk Mar 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants