feat: support for raw "unsafe bindings" #411

ObsidianMinor · 2022-02-08T02:33:15Z

Implements support for raw bindings as described in #358

Raw bindings can be defined in the unsafe.bindings table array in wrangler.toml as -

[[unsafe.bindings]]
name = "my-binding"
type = "binding-type"

The TOML table will then be directly inserted into the metadata upload for a Worker in JSON format as though it were a binding made by wrangler itself. Note, these bindings cannot be detected by tools like miniflare, so support for specific binding types is sparse and limited.

changeset-bot · 2022-02-08T02:33:17Z

🦋 Changeset detected

Latest commit: 9b5a796

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

This PR includes changesets to release 1 package

Name	Type
wrangler	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

threepointone

Some quick notes -

Please add a changeset (https://github.com/cloudflare/wrangler2/blob/main/CONTRIBUTING.md#changesets)
Please add tests
Please use the word "unsafe" instead of "raw" everywhere

packages/wrangler/src/api/form_data.ts

packages/wrangler/src/index.tsx

threepointone

This is looking good to me! A couple of nits, and we can merge this.

threepointone · 2022-02-09T10:14:43Z

packages/wrangler/src/index.tsx

+  for (const binding of config.unsafe?.bindings ?? []) {
+    if (knownBindings.includes(binding.type)) {
+      console.warn(
+        `Raw '${binding.type}' bindings are not directly supported by wrangler. Consider migrating to a ` +


Suggested change

`Raw '${binding.type}' bindings are not directly supported by wrangler. Consider migrating to a ` +

`Unsafe '${binding.type}' bindings are not directly supported by wrangler. Consider migrating to a ` +

I was thinking for documentation purposes we'd call these "raw bindings" (because that's what it is) and going forward any experimental features would be placed in the unsafe table for consistency. I'd be open to writing the warning as Unsafe raw 'x' bindings though. Just thinking if we take this path that "unsafe bindings" are vague because other binding types could be added to the unsafe table but wouldn't be raw bindings.

They're unsafe because we may break this at any point and don't commit to supporting it forever, so the word unsafe should probably be there. If we want to use this for actually stable bindings, then we should add them as first class bindings, regardless of whether they're internal only. We can simply not document them.

threepointone · 2022-02-09T10:15:14Z

packages/wrangler/src/__tests__/publish.test.ts

+      expect(std.err).toMatchInlineSnapshot(`""`);
+      expect(std.warn).toMatchInlineSnapshot(`
+        "'unsafe' fields are experimental and may change or break at any time.
+        Raw 'plain_text' bindings are not directly supported by wrangler. Consider migrating to a format for 'plain_text' bindings that is supported by wrangler for optimal support: https://developers.cloudflare.com/workers/cli-wrangler/configuration"


Suggested change

Raw 'plain_text' bindings are not directly supported by wrangler. Consider migrating to a format for 'plain_text' bindings that is supported by wrangler for optimal support: https://developers.cloudflare.com/workers/cli-wrangler/configuration"

Unsafe 'plain_text' bindings are not directly supported by wrangler. Consider migrating to a format for 'plain_text' bindings that is supported by wrangler for optimal support: https://developers.cloudflare.com/workers/cli-wrangler/configuration"

threepointone · 2022-02-09T12:13:04Z

.vscode/settings.json

@@ -28,5 +28,6 @@
    "xxhash"
  ],
  "cSpell.ignoreWords": ["yxxx"],
-  "eslint.runtime": "node"
+  "eslint.runtime": "node",
+  "files.trimTrailingWhitespace": false


I have it configured to trim trailing whitespace on save on VS Code (which makes sense for most projects I work on). But in this project from what I can tell trailing whitespace is necessary for tests that test console output (since they make empty lines).

I would really like to have all trailing whitespace removed in this project.
Which tests were failing when trimmed?

Any test with a snapshot with an empty line. It makes an inline snapshot like this

expect(std.out).toMatchInlineSnapshot(` "Uploaded test-name (TIMINGS) Deployed test-name (TIMINGS) test-name.test-sub-domain.workers.dev" `);

Which if you have trim whitespace on save enabled will immediately remove the spaces that are actually needed for the snapshot. An alternative solution to this is moving the snapshots to their own files instead and just not editing them manually.

I'm not sure I follow, does it trim whitespace inside the snapshot string?

Yes, there's significant whitespace on this "empty" line

(TIMINGS) test-name.test-sub-domain.workers.dev"

Remove trailing whitespace would delete all the whitespace on the line, including the significant whitespace required for the test.

Ugh!!! That's really horrible.

We should fix the std.out to remove such blank lines, since we are already do other processing on this output for testing.

Either way, we should not allow trailing whitespace in our code.

threepointone

Let's land this and iterate. Thank you for the PR!

threepointone · 2022-02-10T20:49:23Z

Could you rebase please?

ObsidianMinor · 2022-02-10T21:50:11Z

Ah there we go

Now that we have `[[unsafe.bindings]]` (as of #411), we should use that for experimental features. This removes support for `[experimental_services]`, and adds a helpful message for how to rewrite their configuration. This error is temporary, until the internal teams that were using this rewrite their configs. We'll remove it before GA. What the error looks like - ``` Error: The "experimental_services" field is no longer supported. Instead, use [[unsafe.bindings]] to enable experimental features. Add this to your wrangler.toml: [[unsafe.bindings]] name = "SomeService" type = "service" service = "some-service" environment = "staging" [[unsafe.bindings]] name = "SomeOtherService" type = "service" service = "some-other-service" environment = "qa" ```

ObsidianMinor requested a review from threepointone February 8, 2022 02:33

ObsidianMinor force-pushed the feat/unsafe-bindings branch from 2fd9775 to 34c814a Compare February 8, 2022 02:35

threepointone requested changes Feb 8, 2022

View reviewed changes

packages/wrangler/src/api/form_data.ts Outdated Show resolved Hide resolved

packages/wrangler/src/api/form_data.ts Outdated Show resolved Hide resolved

threepointone reviewed Feb 8, 2022

View reviewed changes

packages/wrangler/src/index.tsx Show resolved Hide resolved

Electroid reviewed Feb 8, 2022

View reviewed changes

packages/wrangler/src/index.tsx Outdated Show resolved Hide resolved

ObsidianMinor force-pushed the feat/unsafe-bindings branch 2 times, most recently from 58a99db to a0d88cf Compare February 8, 2022 20:59

threepointone reviewed Feb 9, 2022

View reviewed changes

threepointone approved these changes Feb 10, 2022

View reviewed changes

ObsidianMinor force-pushed the feat/unsafe-bindings branch 2 times, most recently from f8c985e to 1411ddb Compare February 10, 2022 21:36

ObsidianMinor added 2 commits February 10, 2022 15:44

feat: support for raw "unsafe bindings"

15d7578

Turn off automatic trim trailing whitespace for the workspace

9b5a796

ObsidianMinor force-pushed the feat/unsafe-bindings branch from 1411ddb to 9b5a796 Compare February 10, 2022 21:44

threepointone merged commit a52f0e0 into cloudflare:main Feb 10, 2022

github-actions bot mentioned this pull request Feb 10, 2022

Version Packages #407

Merged

This was referenced Feb 11, 2022

chore: remove experimental_services from configuration #445

Merged

Support 'raw' bindings for bindings not directly supported by wrangler #358

Closed

danielrs mentioned this pull request Mar 23, 2022

feat: add service bindings support #683

Closed

aaronlisman mentioned this pull request Jul 7, 2023

dispatch namespace features cloudflare/cloudflare-go#1330

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support for raw "unsafe bindings" #411

feat: support for raw "unsafe bindings" #411

ObsidianMinor commented Feb 8, 2022

changeset-bot bot commented Feb 8, 2022 •

edited

threepointone left a comment

threepointone left a comment

threepointone Feb 9, 2022

ObsidianMinor Feb 9, 2022

threepointone Feb 10, 2022

threepointone Feb 9, 2022

threepointone Feb 9, 2022

ObsidianMinor Feb 9, 2022

petebacondarwin Feb 9, 2022

ObsidianMinor Feb 9, 2022

threepointone Feb 10, 2022 •

edited

ObsidianMinor Feb 10, 2022

petebacondarwin Feb 10, 2022

threepointone left a comment

threepointone commented Feb 10, 2022

ObsidianMinor commented Feb 10, 2022

	`Raw '${binding.type}' bindings are not directly supported by wrangler. Consider migrating to a ` +
	`Unsafe '${binding.type}' bindings are not directly supported by wrangler. Consider migrating to a ` +

	Raw 'plain_text' bindings are not directly supported by wrangler. Consider migrating to a format for 'plain_text' bindings that is supported by wrangler for optimal support: https://developers.cloudflare.com/workers/cli-wrangler/configuration"
	Unsafe 'plain_text' bindings are not directly supported by wrangler. Consider migrating to a format for 'plain_text' bindings that is supported by wrangler for optimal support: https://developers.cloudflare.com/workers/cli-wrangler/configuration"

feat: support for raw "unsafe bindings" #411

feat: support for raw "unsafe bindings" #411

Conversation

ObsidianMinor commented Feb 8, 2022

changeset-bot bot commented Feb 8, 2022 • edited

🦋 Changeset detected

threepointone left a comment

Choose a reason for hiding this comment

threepointone left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

threepointone Feb 10, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

threepointone left a comment

Choose a reason for hiding this comment

threepointone commented Feb 10, 2022

ObsidianMinor commented Feb 10, 2022

changeset-bot bot commented Feb 8, 2022 •

edited

threepointone Feb 10, 2022 •

edited