Refactor how we pass bindings from config to the worker upload api. (#…

…163)

* Refactor how we pass bindings from config to the worker upload api.

There are 3 types of 'bindings' that we upload with a worker definition: `vars`, `kv_namespaces`, and `durable_objects`. These are configured in `wrangler.toml`, and get passed through to the api that uploads a worker definition (in `form_data.ts`), when using `dev` and `publish` commands.

We currently serialise it as a big `variables` object, where the key of the objects defines the binding "name", and the value contains information to be bound. (https://github.com/cloudflare/wrangler2/blob/0330ecf1b54c92dfe86cb3f38394f453ed418381/packages/wrangler/src/index.tsx#L507-L530) This code sucks for many reasons: it loses some type information, hard to read/grok, hard to add new types of bindings, and mostly it's unnecessary.

This PR refactors this code to instead mostly preserve the configuration style structure all the way until we actually serialise it when uploading a worker definition. I also added some more types along the way, and cleared some dead code.

I will follow up this PR with 2 PRs soon: one, which introduces service bindings (as a fresh take on #156), and another one that introduces tests for configuration.

* correctly pass the name of a kv namespace when binding

oops.

* Pass a couple of lint warnings, un-export some unused exports

.changeset/young-buses-try.md

@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+Refactor the way we convert configurations for bindings all the way through to the API where we upload a worker definition. This commit preserves the configuration structure (mostly) until the point we serialise it for the API. This prevents the way we use duck typing to detect a binding type when uploading, makes the types a bit simpler, and makes it easier to add other types of bindings in the future (notably, the upcoming service bindings.)

packages/wrangler/src/api/form_data.ts

@@ -1,72 +1,11 @@
 import type {
   CfWorkerInit,
   CfModuleType,
-  CfVariable,
   CfModule,
+  CfDurableObjectMigrations,
 } from "./worker.js";
 import { FormData, Blob } from "formdata-node";
 
-// Credit: https://stackoverflow.com/a/9458996
-function toBase64(source: BufferSource): string {
-  let result = "";
-  const buffer = source instanceof ArrayBuffer ? source : source.buffer;
-  const bytes = new Uint8Array(buffer);
-  for (let i = 0; i < bytes.byteLength; i++) {
-    result += String.fromCharCode(bytes[i]);
-  }
-  return btoa(result);
-}
-
-function toBinding(
-  name: string,
-  variable: CfVariable
-): Record<string, unknown> {
-  if (typeof variable === "string") {
-    return { name, type: "plain_text", text: variable };
-  }
-
-  if ("namespaceId" in variable) {
-    return {
-      name,
-      type: "kv_namespace",
-      namespace_id: variable.namespaceId,
-    };
-  }
-
-  if ("class_name" in variable) {
-    return {
-      name,
-      type: "durable_object_namespace",
-      class_name: variable.class_name,
-      ...(variable.script_name && {
-        script_name: variable.script_name,
-      }),
-    };
-  }
-
-  const { format, algorithm, usages, data } = variable;
-  if (format) {
-    let key_base64;
-    let key_jwk;
-    if (data instanceof ArrayBuffer || ArrayBuffer.isView(data)) {
-      key_base64 = toBase64(data);
-    } else {
-      key_jwk = data;
-    }
-    return {
-      name,
-      type: "secret_key",
-      format,
-      algorithm,
-      usages,
-      key_base64,
-      key_jwk,
-    };
-  }
-
-  throw new TypeError("Unsupported variable: " + variable);
-}
-
 export function toMimeType(type: CfModuleType): string {
   switch (type) {
     case "esm":
@@ -91,6 +30,23 @@ function toModule(module: CfModule, entryType?: CfModuleType): Blob {
   return new Blob([content], { type });
 }
 
+interface WorkerMetadata {
+  compatibility_date?: string;
+  compatibility_flags?: string[];
+  usage_model?: "bundled" | "unbound";
+  migrations?: CfDurableObjectMigrations;
+  bindings: (
+    | { type: "kv_namespace"; name: string; namespace_id: string }
+    | { type: "plain_text"; name: string; text: string }
+    | {
+        type: "durable_object_namespace";
+        name: string;
+        class_name: string;
+        script_name?: string;
+      }
+  )[];
+}
+
 /**
  * Creates a `FormData` upload from a `CfWorkerInit`.
  */
@@ -99,24 +55,42 @@ export function toFormData(worker: CfWorkerInit): FormData {
   const {
     main,
     modules,
-    variables,
+    bindings,
     migrations,
     usage_model,
     compatibility_date,
     compatibility_flags,
   } = worker;
   const { name, type: mainType } = main;
 
-  const bindings = [];
-  for (const [name, variable] of Object.entries(variables ?? {})) {
-    const binding = toBinding(name, variable);
-    bindings.push(binding);
-  }
+  const metadataBindings: WorkerMetadata["bindings"] = [];
+
+  bindings.kv_namespaces?.forEach(({ id, binding }) => {
+    metadataBindings.push({
+      name: binding,
+      type: "kv_namespace",
+      namespace_id: id,
+    });
+  });
+
+  bindings.durable_objects?.bindings.forEach(
+    ({ name, class_name, script_name }) => {
+      metadataBindings.push({
+        name,
+        type: "durable_object_namespace",
+        class_name: class_name,
+        ...(script_name && { script_name }),
+      });
+    }
+  );
+
+  Object.entries(bindings.vars || {})?.forEach(([key, value]) => {
+    metadataBindings.push({ name: key, type: "plain_text", text: value });
+  });
 
-  // TODO: this object should be typed
-  const metadata = {
+  const metadata: WorkerMetadata = {
     ...(mainType !== "commonjs" ? { main_module: name } : { body_part: name }),
-    bindings,
+    bindings: metadataBindings,
     ...(compatibility_date && { compatibility_date }),
     ...(compatibility_flags && { compatibility_flags }),
     ...(usage_model && { usage_model }),

packages/wrangler/src/api/worker.ts

@@ -62,22 +62,31 @@ export interface CfModule {
   type?: CfModuleType;
 }
 
+/**
+ * A map of variable names to values.
+ */
+interface CfVars {
+  [key: string]: string;
+}
+
 /**
  * A KV namespace.
  */
-export interface CfKvNamespace {
-  /**
-   * The namespace ID.
-   */
-  namespaceId: string;
+interface CfKvNamespace {
+  binding: string;
+  id: string;
 }
 
-export interface CfDurableObject {
+/**
+ * A Durable Object.
+ */
+interface CfDurableObject {
+  name: string;
   class_name: string;
   script_name?: string;
 }
 
-interface CfDOMigrations {
+export interface CfDurableObjectMigrations {
   old_tag?: string;
   new_tag: string;
   steps: {
@@ -87,35 +96,6 @@ interface CfDOMigrations {
   }[];
 }
 
-/**
- * A `WebCrypto` key.
- *
- * @link https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/importKey
- */
-export interface CfCryptoKey {
-  /**
-   * The format.
-   */
-  format: string;
-  /**
-   * The algorithm.
-   */
-  algorithm: string;
-  /**
-   * The usages.
-   */
-  usages: string[];
-  /**
-   * The data.
-   */
-  data: BufferSource | JsonWebKey;
-}
-
-/**
- * A variable (aka. environment variable).
- */
-export type CfVariable = string | CfKvNamespace | CfCryptoKey | CfDurableObject;
-
 /**
  * Options for creating a `CfWorker`.
  */
@@ -133,10 +113,14 @@ export interface CfWorkerInit {
    */
   modules: void | CfModule[];
   /**
-   * The map of names to variables. (aka. environment variables)
+   * All the bindings
    */
-  variables?: { [name: string]: CfVariable };
-  migrations: void | CfDOMigrations;
+  bindings: {
+    kv_namespaces?: CfKvNamespace[];
+    durable_objects?: { bindings: CfDurableObject[] };
+    vars?: CfVars;
+  };
+  migrations: void | CfDurableObjectMigrations;
   compatibility_date: string | void;
   compatibility_flags: void | string[];
   usage_model: void | "bundled" | "unbound";

packages/wrangler/src/config.ts

@@ -1,7 +1,7 @@
 // we're going to manually write both the type definition AND
 // the validator for the config, so that we can give better error messages
 
-type DOMigration = {
+type DurableObjectMigration = {
   tag: string;
   new_classes?: string[];
   renamed_classes?: string[];
@@ -25,13 +25,13 @@ type Dev = {
   upstream_protocol?: string;
 };
 
-type Vars = { [key: string]: string };
+export type Vars = { [key: string]: string };
 
 type Cron = string; // TODO: we should be able to parse a cron pattern with ts
 
 type KVNamespace = {
-  binding?: string;
-  preview_id: string;
+  binding: string;
+  preview_id?: string;
   id: string;
 };
 
@@ -108,7 +108,7 @@ export type Config = {
   jsx_factory?: string; // inherited
   jsx_fragment?: string; // inherited
   vars?: Vars;
-  migrations?: DOMigration[];
+  migrations?: DurableObjectMigration[];
   durable_objects?: { bindings: DurableObject[] };
   kv_namespaces?: KVNamespace[];
   site?: Site; // inherited