Skip to content
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.

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

feat: implement json bindings for wrangler dev --local #449

Closed
threepointone opened this issue Feb 11, 2022 · 7 comments
Closed

feat: implement json bindings for wrangler dev --local #449

threepointone opened this issue Feb 11, 2022 · 7 comments
Assignees

Comments

@threepointone
Copy link
Contributor

threepointone commented Feb 11, 2022

We implemented json bindings in #438, but we'll need it for --local mode as well. The complication here is that this is actually a proper breaking change, and might require changes in miniflare too, as well as some form of warning/messaging.

@threepointone threepointone moved this to Must-have in workers-sdk Feb 14, 2022
@Electroid Electroid added this to the 2.0 milestone Feb 18, 2022
@JacobMGEvans
Copy link
Contributor

Dependency: cloudflare/miniflare#218

@JacobMGEvans
Copy link
Contributor

JacobMGEvans commented Mar 15, 2022

Considering the conversation (in miniflare#218), implementation of Miniflare API, I would like to consider reducing all the flag to --bindings which parses the TOML config still and remove the other flags, to more closely match how remote handles the bindings.

Cc: @petebacondarwin

@petebacondarwin
Copy link
Contributor

@mrbbot suggested an alternative approach, which is to use Miniflare programmatically, where we have full access to set the bindings with the correct types.

@JacobMGEvans
Copy link
Contributor

@mrbbot suggested an alternative approach, which is to use Miniflare programmatically, where we have full access to set the bindings with the correct types.

Correct. I was thinking about removing the extraneous flags and having the Miniflare programmatically (API) handle the TOML file vars and other bindings. Rather than always needing to add a flag when features are added, it would just consume the TOML file.

@petebacondarwin petebacondarwin moved this from Must-have to In Progress in workers-sdk Mar 17, 2022
@petebacondarwin
Copy link
Contributor

I believe this is now supported as of #633

Repository owner moved this from In Progress to Done in workers-sdk Mar 22, 2022
@threepointone
Copy link
Contributor Author

Should we just not have any messaging around the breaking change? Specifically, in wrangler1, x = 1 would previously have env.x be a string, and now it's a number. It might be fine, but something to remember.

@petebacondarwin
Copy link
Contributor

I think the breaking change should have been with this change: 64d62be

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants