Skip to content

attempt 2 at new miniflare config - #14954

Draft
emily-shen wants to merge 9 commits into
mainfrom
emily/new-config-rebased
Draft

attempt 2 at new miniflare config#14954
emily-shen wants to merge 9 commits into
mainfrom
emily/new-config-rebased

Conversation

@emily-shen

Copy link
Copy Markdown
Contributor

Fixes #[insert GH or internal issue link(s)].

Describe your change...


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

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

@github-actions

Copy link
Copy Markdown
Contributor

✅ All changesets look good

@ask-bonk ask-bonk 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.

I'm Bonk, and I've done a quick review of your PR.

Large refactor of miniflare's config format, moving from plugin-namespaced options to a config/dev/legacy structure, plus exporting binding/export schemas from @cloudflare/config. The core index.ts and plugin refactors look semantically faithful. However, there is a build-breaking inconsistency in the @cloudflare/config workflow re-enablement (the head commit is named temp, so this may be a WIP/rebase artifact).

Ranked issues:

  1. (blocker) packages/config/src/types.ts:55 imports WorkflowExport from ./exports, but exports.ts does not define or export WorkflowExport (there are zero workflow references in that file). This is an unresolved type import and will fail the type-check/build.
  2. (correctness) The workflow binding is re-enabled in the TS type layer (bindings.ts, types.ts) but packages/config/src/schema.ts still has the workflow entry commented out in both KnownBindingSchema and ExportSchema. Result: bindings.workflow(...) type-checks, but a { type: "workflow" } binding/export is rejected at runtime by the zod schema (falls through KnownBindingSchema -> validation error). The type and runtime schema must agree.
  3. (process) None of the changesets mention re-enabling workflow bindings/exports; the two config changesets only cover schema exports. If the workflow re-enablement is intentional it needs a changeset; if it's an accidental rebase artifact it should be reverted.

Comment thread packages/config/src/types.ts Outdated
DurableObjectRenamedExport,
DurableObjectTransferredExport,
WorkerEntrypointExport,
WorkflowExport,

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.

WorkflowExport is imported here but is never defined or exported from ./exports (that file has no workflow references at all). This is an unresolved import and will break the type-check/build. Either add and export WorkflowExport in exports.ts, or drop this import until workflow exports are actually implemented:

Suggested change
WorkflowExport,
WorkerEntrypointExport,

(and remove the corresponding | WorkflowExport on line 128).

Comment thread packages/config/src/types.ts Outdated
| DurableObjectExpectingTransferExport
| WorkerEntrypointExport;
| WorkerEntrypointExport
| WorkflowExport;

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.

Depends on WorkflowExport (line 55), which is not defined/exported anywhere. Until WorkflowExport exists in exports.ts this member of the Export union won't compile:

Suggested change
| WorkflowExport;
| WorkerEntrypointExport;

Comment thread packages/config/src/bindings.ts Outdated
// * `exportName` must be a valid `WorkflowEntrypoint` export for the given Worker.
// */
// workflow(options: WorkflowBindingOptions): WorkflowBinding;
workflow(options: WorkflowBindingOptions): WorkflowBinding;

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.

This re-enables the workflow binding in the type layer, but KnownBindingSchema in packages/config/src/schema.ts still has the workflow variant commented out. So bindings.workflow(...) type-checks, yet a { type: "workflow" } binding is rejected at runtime by zod validation. Please make the schema and types consistent — either re-enable workflow in schema.ts (KnownBindingSchema and ExportSchema) as well, or keep this commented out. If this re-enablement is intentional, it also needs a changeset.

@ask-bonk

ask-bonk Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Review posted successfully to PR #14954.

Summary of what I found and posted:

  1. Blockerpackages/config/src/types.ts:55 imports WorkflowExport from ./exports, but that type is never defined or exported anywhere in the config package. This is an unresolved import that breaks the type-check/build.

  2. Correctness — The workflow binding was re-enabled in the TS type layer (bindings.ts:837, types.ts:128), but schema.ts still keeps workflow commented out in both KnownBindingSchema and ExportSchema. This makes types and runtime validation disagree: bindings.workflow(...) type-checks but a { type: "workflow" } binding is rejected at runtime.

  3. Process — No changeset covers the workflow re-enablement; likely an accidental rebase artifact (head commit is named temp).

The core miniflare index.ts and plugin refactors reviewed as semantically faithful to the prior behavior.

github run

@emily-shen
emily-shen force-pushed the emily/new-config-rebased branch from 48a3e7b to d0f1fde Compare August 1, 2026 20:32
@changeset-bot

changeset-bot Bot commented Aug 1, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 943ccc4

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

This PR includes changesets to release 9 packages
Name Type
@cloudflare/config Minor
miniflare Major
@cloudflare/vite-plugin Patch
@cloudflare/build-output-utils Patch
wrangler Patch
@cloudflare/deploy-helpers Patch
@cloudflare/pages-shared Patch
@cloudflare/remote-bindings Patch
@cloudflare/runtime-types 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Untriaged

Development

Successfully merging this pull request may close these issues.

2 participants