Skip to content

Part 1️⃣ of removing CfWorkerInit["bindings"]#12449

Merged
penalosa merged 2 commits intomainfrom
penalosa/granular-refactor
Feb 9, 2026
Merged

Part 1️⃣ of removing CfWorkerInit["bindings"]#12449
penalosa merged 2 commits intomainfrom
penalosa/granular-refactor

Conversation

@penalosa
Copy link
Copy Markdown
Contributor

@penalosa penalosa commented Feb 6, 2026

First part of landing #12151 in stages. This part:

  • Moves the Binding type to @cloudflare/workers-utils
  • Fixes unsafe metadata printing in printBindings()

  • 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: internal refactor

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

@penalosa penalosa requested a review from a team as a code owner February 6, 2026 13:51
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Feb 6, 2026

🦋 Changeset detected

Latest commit: d83f2a2

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

@github-project-automation github-project-automation Bot moved this to Untriaged in workers-sdk Feb 6, 2026
@penalosa penalosa changed the title Part 1️⃣ of #12151 Part 1️⃣ of removing CfWorkerInit["bindings"] Feb 6, 2026
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 found 1 potential issue.

View 5 additional findings in Devin Review.

Open in Devin Review

Comment thread packages/wrangler/src/api/startDevWorker/BundlerController.ts
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Feb 6, 2026

create-cloudflare

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

@cloudflare/kv-asset-handler

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

miniflare

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

@cloudflare/pages-shared

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

@cloudflare/unenv-preset

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

@cloudflare/vite-plugin

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

@cloudflare/vitest-pool-workers

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

@cloudflare/workers-editor-shared

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

@cloudflare/workers-utils

npm i https://pkg.pr.new/@cloudflare/workers-utils@12449

wrangler

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

commit: d83f2a2

Comment thread packages/wrangler/src/__tests__/helpers/mock-upload-worker.ts
const bindings = (
await convertBindingsToCfWorkerInitBindings(config.bindings)
).bindings;
const doBindings = extractBindingsOfType(
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.

super nit: i would swap the parameter order, to look alike bindings["durable_object_namespace"]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's a fair comment. I'm going to leave it for now because this is an existing function that's used in lots of different places, but that would be a good followup.

Comment thread packages/wrangler/src/utils/print-bindings.ts Outdated
Copy link
Copy Markdown
Contributor

@vicb vicb left a comment

Choose a reason for hiding this comment

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

Good move to split the huge review, thanks!

LGTM, only a few nits

@github-project-automation github-project-automation Bot moved this from Untriaged to Approved in workers-sdk Feb 6, 2026
Copy link
Copy Markdown
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

What is the bug in printBindings that is being fixed? If it is an actual bug, then this should have a changeset, no?

};

export type File = {
export type ParseFile = {
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.

Why this rename? Is it because File is very general? You could say the same about Location and Message?

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.

Or another way of asking the question... why not reuse the File type that is defined in types.ts?

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.

Final weird comment: Do we even need to export Location and (Parse)File? We could just inline these into the Message type probably.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The File type in types.ts is different, but was conflicting with this ParseFile type which is why I renamed it. It can't be inlined because it's referenced outside of Message, and it has to be exported or dts bundling breaks because TS can't find the symbol.

@github-project-automation github-project-automation Bot moved this from Approved to In Review in workers-sdk Feb 6, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Feb 6, 2026

Claude finished @penalosa's task —— View job


Changeset Review

  • Read changeset guidelines
  • Review changeset file
  • Check for cute animal images
  • Provide feedback

All changesets look good

The changeset .changeset/unsafe-metadata-json-format.md is well-formatted and follows all guidelines:

  1. Version Type: patch is appropriate for a bug fix
  2. Changelog Quality: Clear description with helpful before/after examples showing the fix
  3. Markdown Headers: No h1/h2/h3 headers used
  4. Format: Proper title and body with good explanation
  5. Package Coverage: Correctly targets wrangler

The changeset clearly explains that unsafe metadata was incorrectly being displayed in the bindings table and is now properly displayed as a separate JSON block, which is indeed a bug fix warranting a patch version.

@github-project-automation github-project-automation Bot moved this from In Review to Approved in workers-sdk Feb 6, 2026
@penalosa penalosa merged commit bfd17cd into main Feb 9, 2026
50 of 53 checks passed
@github-project-automation github-project-automation Bot moved this from Approved to Done in workers-sdk Feb 9, 2026
@penalosa penalosa deleted the penalosa/granular-refactor branch February 9, 2026 13:32
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