Skip to content

Various fixes/improvements around remote bindings being used as local #11162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 21 commits into from
Nov 5, 2025
Merged

Various fixes/improvements around remote bindings being used as local #11162

merged 21 commits into from
Nov 5, 2025

Conversation

@dario-piotrowicz
Copy link
Member

@dario-piotrowicz dario-piotrowicz commented Nov 3, 2025 •
edited
Loading

This PR introduces various fixes/improvements around disabled remote bindings, namely:

  • updates wrangler dev's --local flag's description to clarify that it disables remote bindings. It also undeprecates it and un-hides it.
  • it addresses the fact that wrangler dev --local currently prints local bindings (configured with remote: true) as remote
  • it addresses the fact that some remote bindings, such as AI still worked as remote under wrangler dev --local
  • ensures that remote: false set in startWorker disables remote bindings
  • adds a remoteBindings option to getPlatformProxy and also to the vite plugin to disable the remote aspect of remote bindings (aligned with vitest-pool-workers)

@changeset-bot
Copy link

changeset-bot bot commented Nov 3, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: 3db45f9

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 Nov 3, 2025
@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 3, 2025 •
edited
Loading

create-cloudflare

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

@cloudflare/kv-asset-handler

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

miniflare

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

@cloudflare/pages-shared

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

@cloudflare/unenv-preset

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

@cloudflare/vite-plugin

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

@cloudflare/vitest-pool-workers

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

@cloudflare/workers-editor-shared

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

@cloudflare/workers-utils

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

wrangler

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

commit: 3db45f9

@dario-piotrowicz
fixup! Update the description of the --local flag for the `wrangler…
2230f82 
… dev` command to clarify that it disables remote bindings, also un-deprecate and un-hide it

fix incorrect implementation
@dario-piotrowicz
add changeset and disable remote bindings when localBindingsOnly is…
4ebead9 
… set
@dario-piotrowicz
make sure startWorker doesn't connect to remote bindings when localBi…
443b6cb 
…ndingsOnly is set
@dario-piotrowicz dario-piotrowicz changed the title Minor improvements/fixes for the wrangler dev's --local flag Various fixes/improvements around remote bindings being used as local Nov 4, 2025
@dario-piotrowicz dario-piotrowicz added the skip-v3-pr Skip validation of presence of a v3 backport PR label Nov 4, 2025
@dario-piotrowicz dario-piotrowicz marked this pull request as ready for review November 4, 2025 09:24
@dario-piotrowicz dario-piotrowicz requested a review from a team as a code owner November 4, 2025 09:24
@dario-piotrowicz dario-piotrowicz force-pushed the dario/remote-wrangler-dev--local branch from 160ed68 to ba50b6a Compare November 4, 2025 10:10
@penalosa
Copy link
Contributor

penalosa commented Nov 4, 2025

I think we should unify flags with the Vitest integration, which AFAIK uses remoteBindings: false to disable remote bindings?

@dario-piotrowicz
Copy link
Member Author

dario-piotrowicz commented Nov 4, 2025

I think we should unify flags with the Vitest integration, which AFAIK uses remoteBindings: false to disable remote bindings?

you're right, I thought we completely removed that when we released remote bindings 😕

Ok, thanks 🙂, I can use remoteBindings across the board 👍

penalosa
penalosa requested changes Nov 4, 2025
View reviewed changes
@github-project-automation github-project-automation bot moved this from Untriaged to In Review in workers-sdk Nov 4, 2025
@dario-piotrowicz dario-piotrowicz requested a review from a team as a code owner November 4, 2025 16:56
penalosa
penalosa reviewed Nov 5, 2025
View reviewed changes
packages/vite-plugin-cloudflare/e2e/remote-bindings.test.ts Outdated
});
}

describe.only("remote bindings disabled", () => {
Copy link
Contributor

@penalosa penalosa Nov 5, 2025

Choose a reason for hiding this comment

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

.only?

Copy link
Member Author

@dario-piotrowicz dario-piotrowicz Nov 5, 2025

Choose a reason for hiding this comment

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

sorry, ignore it, I'm just trying to debug a CI windows failure 😓 (before just adding a skipIf(process.platform === 'win32') 😅 )

Copy link
Member Author

@dario-piotrowicz dario-piotrowicz Nov 5, 2025

Choose a reason for hiding this comment

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

yeah I ended up just skipping the test on windows: https://github.com/cloudflare/workers-sdk/pull/11162/files#diff-d9b32f69a202cbf69dc77c2777a78758abe37967f4015456c65d005d6117c33dR146-R150

penalosa
penalosa reviewed Nov 5, 2025
View reviewed changes
penalosa
penalosa reviewed Nov 5, 2025
View reviewed changes
penalosa
penalosa reviewed Nov 5, 2025
View reviewed changes
penalosa
penalosa approved these changes Nov 5, 2025
View reviewed changes
Copy link
Contributor

@penalosa penalosa left a comment

Choose a reason for hiding this comment

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

Generally looks good, with some nits

@github-project-automation github-project-automation bot moved this from In Review to Approved in workers-sdk Nov 5, 2025
@dario-piotrowicz dario-piotrowicz force-pushed the dario/remote-wrangler-dev--local branch from 46d6edb to dbdb75a Compare November 5, 2025 10:58
@dario-piotrowicz dario-piotrowicz force-pushed the dario/remote-wrangler-dev--local branch 2 times, most recently from aae4e2b to fa36841 Compare November 5, 2025 12:22
@dario-piotrowicz dario-piotrowicz merged commit c3ed531 into main Nov 5, 2025
39 of 40 checks passed
@github-project-automation github-project-automation bot moved this from Approved to Done in workers-sdk Nov 5, 2025
@dario-piotrowicz dario-piotrowicz deleted the dario/remote-wrangler-dev--local branch November 5, 2025 14:36
@workers-devprod workers-devprod mentioned this pull request Nov 5, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@penalosa penalosa penalosa approved these changes

Assignees

No one assigned

Labels

skip-v3-pr Skip validation of presence of a v3 backport PR

Projects

workers-sdk
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dario-piotrowicz @penalosa