feat: Add support for "json" bindings #438
Conversation
🦋 Changeset detectedLatest commit: 2e87c36 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
wrangler prerelease is available for testing: npm install --save-dev https://prerelease-registry.developers.workers.dev/runs/1828147535/wrangler |
There was a problem hiding this comment.
I tested this with wrangler dev and it does seem to work, despite my misgivings around the JSON encoding of the values.
I added an extra commit with a publish test, which picked up a missing change to the config.ts. (Testing FTW!)
Great quick win @Electroid!
| [vars] | ||
| text = "plain ol' string" | ||
| count = 1 | ||
| complex = { enabled = true, id = 123 } |
There was a problem hiding this comment.
Strictly this object literal is not JSON (since it is in TOML).
Is the fact that it will be stringified to JSON in order to upload it is an implementation concern?
Perhaps we should just say that we now support more complex literals? Or perhaps more nuanced bindings that can be stringified as JSON?
| if (typeof value === "string") { | ||
| metadataBindings.push({ name: key, type: "plain_text", text: value }); | ||
| } else { | ||
| metadataBindings.push({ name: key, type: "json", json: value }); | ||
| } |
There was a problem hiding this comment.
I had a long think about this, because it looked kinda dangerous to just rely upon the typeof here. But I came to the conclusion that it is reasonable.
I am slightly concerned about the upstream consumption of these "JSON" values. We simply JSON.stringify() the whole metadata object. So I think we would end up sending a string to the server that is like:
wrangler.toml
[vars]
text = "plain ol' string"
count = 1
complex = { enabled = true, id = 123 }
upload metadata string
'{"main_module":"index.js",' +
'"bindings":[{"name":"text","type":"plain_text","text":"plain ol' string"},' +
`{"name":"count","type":"json","json":1},' +
`{"name":"complex","type":"json","json":{"enabled":true,"id":123}}],' +
'"compatibility_date":"2022-01-12"}'
Note that the actual text that gets uploaded contains:
{"name":"complex","type":"json","json":{"enabled":true,"id":123}}
I would have expected the value of the complex var to be double encoded in JSON. so it would look like:
{"name":"complex","type":"json","json":"{\\"enabled\\":true,\\"id\\":123}"}
But perhaps I am over thinking this??
There was a problem hiding this comment.
The real test is whether env.complex resolves correctly, and if you're saying you tested it with wrangler dev, then that would means it's all good.
|
This looks dope, let's ship it! |
|
Hey! Thanks for the mention @Electroid. Quick question on this, Wrangler 1 used to stringify all numeric/boolean |
|
^ This was my experience with wrangler 1 too, which threw me for a loop. |
We've had support for "json" bindings for a long time, but never added them to wrangler1.. oops! This is nice because you can include complex values, and you don't need to use things like
parseInteger(env.variable)in your code./cc @mrbbot Since you probably didn't know this existed! 😅