diff --git a/.changeset/proud-forks-build.md b/.changeset/proud-forks-build.md new file mode 100644 index 000000000000..7dc7ff27427f --- /dev/null +++ b/.changeset/proud-forks-build.md @@ -0,0 +1,36 @@ +--- +"wrangler": patch +--- + +fix: widen multi-env `vars` types in `wrangler types` + +Currently, the type generated for `vars` is a string literal consisting of the value of the variable in the top level environment. If multiple environments +are specified this wrongly restricts the type, since the variable could contain any of the values from each of the environments. + +For example, given a `wrangler.toml` containing the following: + +``` +[vars] +MY_VAR = "dev value" + +[env.production.vars] +MY_VAR = "prod value" +``` + +running `wrangler types` would generate: + +```ts +interface Env { + MY_VAR: "dev value"; +} +``` + +making typescript incorrectly assume that `MY_VAR` is always going to be `"dev value"` + +after these changes, the generated interface would instead be: + +```ts +interface Env { + MY_VAR: "dev value" | "prod value"; +} +``` diff --git a/.changeset/unlucky-timers-sort.md b/.changeset/unlucky-timers-sort.md new file mode 100644 index 000000000000..8c89983ac378 --- /dev/null +++ b/.changeset/unlucky-timers-sort.md @@ -0,0 +1,46 @@ +--- +"wrangler": minor +--- + +add `--strict-vars` option to `wrangler types` + +add a new `--strict-vars` option to `wrangler types` that developers can (by setting the +flag to `false`) use to disable the default strict/literal types generation for their variables + +opting out of strict variables can be useful when developers change often their `vars` values, +even more so when multiple environments are involved + +## Example + +With a toml containing: + +```toml +[vars] +MY_VARIABLE = "production_value" +MY_NUMBERS = [1, 2, 3] + +[env.staging.vars] +MY_VARIABLE = "staging_value" +MY_NUMBERS = [7, 8, 9] +``` + +the `wrangler types` command would generate the following interface: + +``` +interface Env { + MY_VARIABLE: "production_value" | "staging_value"; + MY_NUMBERS: [1,2,3] | [7,8,9]; +} +``` + +while `wrangler types --strict-vars=false` would instead generate: + +``` +interface Env { + MY_VARIABLE: string; + MY_NUMBERS: number[]; +} +``` + +(allowing the developer to easily change their toml variables without the +risk of breaking typescript types) diff --git a/packages/wrangler/src/__tests__/type-generation.test.ts b/packages/wrangler/src/__tests__/type-generation.test.ts index 435f178a15bc..45385df0c988 100644 --- a/packages/wrangler/src/__tests__/type-generation.test.ts +++ b/packages/wrangler/src/__tests__/type-generation.test.ts @@ -2,7 +2,6 @@ import * as fs from "fs"; import * as TOML from "@iarna/toml"; import { constructTSModuleGlob, - constructType, constructTypeKey, generateImportSpecifier, isValidIdentifier, @@ -65,43 +64,6 @@ describe("constructTSModuleGlob() should return a valid TS glob ", () => { }); }); -describe("constructType", () => { - it("should return a valid type", () => { - expect(constructType("valid", "string")).toBe("valid: string;"); - expect(constructType("valid123", "string")).toBe("valid123: string;"); - expect(constructType("valid_123", "string")).toBe("valid_123: string;"); - expect(constructType("valid_123_", "string")).toBe("valid_123_: string;"); - expect(constructType("_valid_123_", "string")).toBe("_valid_123_: string;"); - expect(constructType("_valid_123_", "string")).toBe("_valid_123_: string;"); - - expect(constructType("123invalid", "string")).toBe('"123invalid": string;'); - expect(constructType("invalid-123", "string")).toBe( - '"invalid-123": string;' - ); - expect(constructType("invalid 123", "string")).toBe( - '"invalid 123": string;' - ); - - expect(constructType("valid", 'a"', false)).toBe('valid: "a\\"";'); - expect(constructType("valid", "a\\", false)).toBe('valid: "a\\\\";'); - expect(constructType("valid", "a\\b", false)).toBe('valid: "a\\\\b";'); - expect(constructType("valid", 'a\\b"', false)).toBe('valid: "a\\\\b\\"";'); - - expect(constructType("valid", 1)).toBe("valid: 1;"); - expect(constructType("valid", 12345)).toBe("valid: 12345;"); - expect(constructType("valid", true)).toBe("valid: true;"); - expect(constructType("valid", false)).toBe("valid: false;"); - }); -}); - -describe("constructType with multiline strings", () => { - it("should correctly escape newlines in string values", () => { - const multilineString = "This is a\nmulti-line\nstring"; - const expected = `valid: "This is a\\nmulti-line\\nstring";`; - expect(constructType("valid", multilineString, false)).toBe(expected); - }); -}); - describe("generateImportSpecifier", () => { it("should generate a relative import specifier", () => { expect(generateImportSpecifier("/app/types.ts", "/app/index.ts")).toBe( @@ -602,6 +564,35 @@ describe("generateTypes()", () => { `); }); + it("should allow opting out of strict-vars", async () => { + fs.writeFileSync( + "./wrangler.toml", + TOML.stringify({ + vars: { + varStr: "A from wrangler toml", + varArrNum: [1, 2, 3], + varArrMix: [1, "two", 3, true], + varObj: { test: true }, + }, + } as TOML.JsonMap), + "utf-8" + ); + + await runWrangler("types --strict-vars=false"); + + expect(std.out).toMatchInlineSnapshot(` + "Generating project types... + + interface Env { + varStr: string; + varArrNum: number[]; + varArrMix: (boolean|number|string)[]; + varObj: object; + } + " + `); + }); + it("should override vars with secrets", async () => { fs.writeFileSync( "./wrangler.toml", @@ -634,6 +625,110 @@ describe("generateTypes()", () => { `); }); + it("various different types of vars", async () => { + fs.writeFileSync( + "./wrangler.toml", + TOML.stringify({ + vars: { + "var-a": '"a\\""', + "var-a-1": '"a\\\\"', + "var-a-b": '"a\\\\b"', + "var-a-b-": '"a\\\\b\\""', + 1: 1, + 12345: 12345, + true: true, + false: false, + "multi\nline\nvar": "this\nis\na\nmulti\nline\nvariable!", + }, + } as TOML.JsonMap), + "utf-8" + ); + await runWrangler("types"); + + expect(std.out).toMatchInlineSnapshot(` + "Generating project types... + + interface Env { + \\"1\\": 1; + \\"12345\\": 12345; + \\"var-a\\": \\"/\\"a///\\"/\\"\\"; + \\"var-a-1\\": \\"/\\"a/////\\"\\"; + \\"var-a-b\\": \\"/\\"a////b/\\"\\"; + \\"var-a-b-\\": \\"/\\"a////b///\\"/\\"\\"; + true: true; + false: false; + \\"multi + line + var\\": \\"this/nis/na/nmulti/nline/nvariable!\\"; + } + " + `); + }); + + describe("vars present in multiple environments", () => { + beforeEach(() => { + fs.writeFileSync( + "./wrangler.toml", + TOML.stringify({ + vars: { + MY_VAR: "a var", + MY_VAR_A: "A (dev)", + MY_VAR_B: { value: "B (dev)" }, + MY_VAR_C: ["a", "b", "c"], + }, + env: { + production: { + vars: { + MY_VAR: "a var", + MY_VAR_A: "A (prod)", + MY_VAR_B: { value: "B (prod)" }, + MY_VAR_C: [1, 2, 3], + }, + }, + staging: { + vars: { + MY_VAR_A: "A (stag)", + }, + }, + }, + } as TOML.JsonMap), + "utf-8" + ); + }); + + it("should produce string and union types for variables (default)", async () => { + await runWrangler("types"); + + expect(std.out).toMatchInlineSnapshot(` + "Generating project types... + + interface Env { + MY_VAR: \\"a var\\"; + MY_VAR_A: \\"A (dev)\\" | \\"A (prod)\\" | \\"A (stag)\\"; + MY_VAR_C: [\\"a\\",\\"b\\",\\"c\\"] | [1,2,3]; + MY_VAR_B: {\\"value\\":\\"B (dev)\\"} | {\\"value\\":\\"B (prod)\\"}; + } + " + `); + }); + + it("should produce non-strict types for variables (with --strict-vars=false)", async () => { + await runWrangler("types --strict-vars=false"); + + expect(std.out).toMatchInlineSnapshot(` + "Generating project types... + + interface Env { + MY_VAR: string; + MY_VAR_A: string; + MY_VAR_C: string[] | number[]; + MY_VAR_B: object; + } + " + `); + }); + }); + describe("customization", () => { describe("env", () => { it("should allow the user to customize the interface name", async () => { @@ -768,7 +863,7 @@ describe("generateTypes()", () => { }); }); - it("should allow multiple customization to be applied together", async () => { + it("should allow multiple customizations to be applied together", async () => { fs.writeFileSync( "./wrangler.toml", TOML.stringify({ diff --git a/packages/wrangler/src/type-generation/index.ts b/packages/wrangler/src/type-generation/index.ts index b3ef27d136a7..04b24ea6f972 100644 --- a/packages/wrangler/src/type-generation/index.ts +++ b/packages/wrangler/src/type-generation/index.ts @@ -2,7 +2,7 @@ import * as fs from "node:fs"; import { basename, dirname, extname, join, relative, resolve } from "node:path"; import { findUpSync } from "find-up"; import { getNodeCompat } from "miniflare"; -import { readConfig } from "../config"; +import { experimental_readRawConfig, readConfig } from "../config"; import { getEntry } from "../deployment-bundle/entry"; import { getVarsForDev } from "../dev/dev-vars"; import { CommandLineArgsError, UserError } from "../errors"; @@ -11,7 +11,7 @@ import { parseJSONC } from "../parse"; import { printWranglerBanner } from "../wrangler-banner"; import { generateRuntimeTypes } from "./runtime"; import { logRuntimeTypesMessage } from "./runtime/log-runtime-types-message"; -import type { Config } from "../config"; +import type { Config, RawEnvironment } from "../config"; import type { Entry } from "../deployment-bundle/entry"; import type { CfScriptFormat } from "../deployment-bundle/worker"; import type { @@ -38,6 +38,11 @@ export function typesOptions(yargs: CommonYargsArgv) { type: "string", describe: "The path of the generated runtime types file", demandOption: false, + }) + .option("strict-vars", { + type: "boolean", + default: true, + describe: "Generate literal and union types for variables", }); } @@ -113,11 +118,9 @@ export async function typesHandler( true ) as Record; - const configBindingsWithSecrets: Partial & { - secrets: Record; - } = { + const configBindingsWithSecrets = { kv_namespaces: config.kv_namespaces ?? [], - vars: { ...config.vars }, + vars: collectAllVars(args), wasm_modules: config.wasm_modules, text_blobs: { ...config.text_blobs, @@ -201,32 +204,14 @@ export function generateImportSpecifier(from: string, to: string) { } } -/** - * Construct a full type definition for a key-value pair - * If useRawVal is true, we'll use the value as the type, otherwise we'll format it for a string definition - */ -export function constructType( - key: string, - value: string | number | boolean, - useRawVal = true -) { - const typeKey = constructTypeKey(key); - if (typeof value === "string") { - if (useRawVal) { - return `${typeKey}: ${value};`; - } - return `${typeKey}: ${JSON.stringify(value)};`; - } - if (typeof value === "number" || typeof value === "boolean") { - return `${typeKey}: ${value};`; - } - return `${typeKey}: unknown;`; -} - type Secrets = Record; +type ConfigToDTS = Partial> & { vars: VarTypes } & { + secrets: Secrets; +}; + async function generateTypes( - configToDTS: Partial & { secrets: Secrets }, + configToDTS: ConfigToDTS, config: Config, envInterface: string, outputPath: string @@ -262,11 +247,14 @@ async function generateTypes( ); } - const envTypeStructure: string[] = []; + const envTypeStructure: [string, string][] = []; if (configToDTS.kv_namespaces) { for (const kvNamespace of configToDTS.kv_namespaces) { - envTypeStructure.push(constructType(kvNamespace.binding, "KVNamespace")); + envTypeStructure.push([ + constructTypeKey(kvNamespace.binding), + "KVNamespace", + ]); } } @@ -275,24 +263,16 @@ async function generateTypes( const vars = Object.entries(configToDTS.vars).filter( ([key]) => !(key in configToDTS.secrets) ); - for (const [varName, varValue] of vars) { - if ( - typeof varValue === "string" || - typeof varValue === "number" || - typeof varValue === "boolean" - ) { - envTypeStructure.push(constructType(varName, varValue, false)); - } - if (typeof varValue === "object" && varValue !== null) { - envTypeStructure.push( - constructType(varName, JSON.stringify(varValue), true) - ); - } + for (const [varName, varValues] of vars) { + envTypeStructure.push([ + constructTypeKey(varName), + varValues.length === 1 ? varValues[0] : varValues.join(" | "), + ]); } } for (const secretName in configToDTS.secrets) { - envTypeStructure.push(constructType(secretName, "string", true)); + envTypeStructure.push([constructTypeKey(secretName), "string"]); } if (configToDTS.durable_objects?.bindings) { @@ -315,66 +295,68 @@ async function generateTypes( typeName = `DurableObjectNamespace /* ${durableObject.class_name} */`; } - envTypeStructure.push(constructType(durableObject.name, typeName)); + envTypeStructure.push([constructTypeKey(durableObject.name), typeName]); } } if (configToDTS.r2_buckets) { for (const R2Bucket of configToDTS.r2_buckets) { - envTypeStructure.push(constructType(R2Bucket.binding, "R2Bucket")); + envTypeStructure.push([constructTypeKey(R2Bucket.binding), "R2Bucket"]); } } if (configToDTS.d1_databases) { for (const d1 of configToDTS.d1_databases) { - envTypeStructure.push(constructType(d1.binding, "D1Database")); + envTypeStructure.push([constructTypeKey(d1.binding), "D1Database"]); } } if (configToDTS.services) { for (const service of configToDTS.services) { - envTypeStructure.push(constructType(service.binding, "Fetcher")); + envTypeStructure.push([constructTypeKey(service.binding), "Fetcher"]); } } if (configToDTS.analytics_engine_datasets) { for (const analyticsEngine of configToDTS.analytics_engine_datasets) { - envTypeStructure.push( - constructType(analyticsEngine.binding, "AnalyticsEngineDataset") - ); + envTypeStructure.push([ + constructTypeKey(analyticsEngine.binding), + "AnalyticsEngineDataset", + ]); } } if (configToDTS.dispatch_namespaces) { for (const namespace of configToDTS.dispatch_namespaces) { - envTypeStructure.push( - constructType(namespace.binding, "DispatchNamespace") - ); + envTypeStructure.push([ + constructTypeKey(namespace.binding), + "DispatchNamespace", + ]); } } if (configToDTS.logfwdr?.bindings?.length) { - envTypeStructure.push(constructType("LOGFWDR_SCHEMA", "any")); + envTypeStructure.push([constructTypeKey("LOGFWDR_SCHEMA"), "any"]); } if (configToDTS.data_blobs) { for (const dataBlobs in configToDTS.data_blobs) { - envTypeStructure.push(constructType(dataBlobs, "ArrayBuffer")); + envTypeStructure.push([constructTypeKey(dataBlobs), "ArrayBuffer"]); } } if (configToDTS.text_blobs) { for (const textBlobs in configToDTS.text_blobs) { - envTypeStructure.push(constructType(textBlobs, "string")); + envTypeStructure.push([constructTypeKey(textBlobs), "string"]); } } if (configToDTS.unsafe?.bindings) { for (const unsafe of configToDTS.unsafe.bindings) { if (unsafe.type === "ratelimit") { - envTypeStructure.push(constructType(unsafe.name, "RateLimit")); + envTypeStructure.push([constructTypeKey(unsafe.name), "RateLimit"]); } else { - envTypeStructure.push(constructType(unsafe.name, "any")); + envTypeStructure.push([constructTypeKey(unsafe.name), "any"]); } } } @@ -382,32 +364,41 @@ async function generateTypes( if (configToDTS.queues) { if (configToDTS.queues.producers) { for (const queue of configToDTS.queues.producers) { - envTypeStructure.push(constructType(queue.binding, "Queue")); + envTypeStructure.push([constructTypeKey(queue.binding), "Queue"]); } } } if (configToDTS.send_email) { for (const sendEmail of configToDTS.send_email) { - envTypeStructure.push(constructType(sendEmail.name, "SendEmail")); + envTypeStructure.push([constructTypeKey(sendEmail.name), "SendEmail"]); } } if (configToDTS.vectorize) { for (const vectorize of configToDTS.vectorize) { - envTypeStructure.push(constructType(vectorize.binding, "VectorizeIndex")); + envTypeStructure.push([ + constructTypeKey(vectorize.binding), + "VectorizeIndex", + ]); } } if (configToDTS.hyperdrive) { for (const hyperdrive of configToDTS.hyperdrive) { - envTypeStructure.push(constructType(hyperdrive.binding, "Hyperdrive")); + envTypeStructure.push([ + constructTypeKey(hyperdrive.binding), + "Hyperdrive", + ]); } } if (configToDTS.mtls_certificates) { for (const mtlsCertificate of configToDTS.mtls_certificates) { - envTypeStructure.push(constructType(mtlsCertificate.binding, "Fetcher")); + envTypeStructure.push([ + constructTypeKey(mtlsCertificate.binding), + "Fetcher", + ]); } } @@ -415,28 +406,33 @@ async function generateTypes( // The BrowserWorker type in @cloudflare/puppeteer is of type // { fetch: typeof fetch }, but workers-types doesn't include it // and Fetcher is valid for the purposes of handing it to puppeteer - envTypeStructure.push( - constructType(configToDTS.browser.binding, "Fetcher") - ); + envTypeStructure.push([ + constructTypeKey(configToDTS.browser.binding), + "Fetcher", + ]); } if (configToDTS.ai) { - envTypeStructure.push(constructType(configToDTS.ai.binding, "Ai")); + envTypeStructure.push([constructTypeKey(configToDTS.ai.binding), "Ai"]); } if (configToDTS.version_metadata) { - envTypeStructure.push( - `${configToDTS.version_metadata.binding}: { id: string; tag: string };` - ); + envTypeStructure.push([ + configToDTS.version_metadata.binding, + "{ id: string; tag: string }", + ]); } if (configToDTS.assets?.binding) { - envTypeStructure.push(constructType(configToDTS.assets.binding, "Fetcher")); + envTypeStructure.push([ + constructTypeKey(configToDTS.assets.binding), + "Fetcher", + ]); } if (configToDTS.workflows) { for (const workflow of configToDTS.workflows) { - envTypeStructure.push(constructType(workflow.binding, "Workflow")); + envTypeStructure.push([constructTypeKey(workflow.binding), "Workflow"]); } } @@ -477,7 +473,7 @@ function writeDTSFile({ envInterface, path, }: { - envTypeStructure: string[]; + envTypeStructure: [string, string][]; modulesTypeStructure: string[]; formatType: CfScriptFormat; envInterface: string; @@ -510,7 +506,7 @@ function writeDTSFile({ const { fileContent, consoleOutput } = generateTypeStrings( formatType, envInterface, - envTypeStructure, + envTypeStructure.map(([key, value]) => `${key}: ${value};`), modulesTypeStructure ); @@ -578,3 +574,75 @@ type TSConfig = { types: string[]; }; }; + +type VarTypes = Record; + +/** + * Collects all the vars types across all the environments defined in the config file + * + * @param args all the CLI arguments passed to the `types` command + * @returns an object which keys are the variable names and values are arrays containing all the computed types for such variables + */ +function collectAllVars( + args: StrictYargsOptionsToInterface +): Record { + const varsInfo: Record> = {}; + + // Collects onto the `varsInfo` object the vars and values for a specific environment + function collectEnvironmentVars(vars: RawEnvironment["vars"]) { + Object.entries(vars ?? {}).forEach(([key, value]) => { + varsInfo[key] ??= new Set(); + + if (!args.strictVars) { + // when strict-vars is false we basically only want the plain "typeof" values + varsInfo[key].add( + Array.isArray(value) ? typeofArray(value) : typeof value + ); + return; + } + + if (typeof value === "number" || typeof value === "boolean") { + varsInfo[key].add(`${value}`); + return; + } + if (typeof value === "string" || typeof value === "object") { + varsInfo[key].add(JSON.stringify(value)); + return; + } + + // let's fallback to a safe `unknown` if we couldn't detect the type + varsInfo[key].add("unknown"); + }); + } + + const { rawConfig } = experimental_readRawConfig(args); + collectEnvironmentVars(rawConfig.vars); + Object.entries(rawConfig.env ?? {}).forEach(([_envName, env]) => { + collectEnvironmentVars(env.vars); + }); + + return Object.fromEntries( + Object.entries(varsInfo).map(([key, value]) => [key, [...value]]) + ); +} + +/** + * Given an array it returns a string representing the types present in such array + * + * e.g. + * `[1, 2, 3]` returns `number[]`, + * `[1, 2, 'three']` returns `(number|string)[]`, + * `['false', true]` returns `(string|boolean)[]`, + * + * @param array the target array + * @returns a string representing the types of such array + */ +function typeofArray(array: unknown[]): string { + const typesInArray = [...new Set(array.map((item) => typeof item))].sort(); + + if (typesInArray.length === 1) { + return `${typesInArray[0]}[]`; + } + + return `(${typesInArray.join("|")})[]`; +}