From 6fbfa0155e4f664530107552613d03e012141045 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Wed, 1 Jan 2025 13:43:19 +0100 Subject: [PATCH 01/12] fix: widen multi-env `vars` types in `wrangler types` --- .changeset/proud-forks-build.md | 39 ++++++++ .../src/__tests__/type-generation.test.ts | 44 +++++++++ .../wrangler/src/type-generation/index.ts | 97 ++++++++++++++----- 3 files changed, 156 insertions(+), 24 deletions(-) create mode 100644 .changeset/proud-forks-build.md diff --git a/.changeset/proud-forks-build.md b/.changeset/proud-forks-build.md new file mode 100644 index 000000000000..20ba9b3492c0 --- /dev/null +++ b/.changeset/proud-forks-build.md @@ -0,0 +1,39 @@ +--- +"wrangler": patch +--- + +fix: widen multi-env `vars` types in `wrangler types` + +Currently types for variable generate string literal, those are appropriate when +a single environment has been specified in the config file but if multiple environments +are specified this however wrongly restricts the typing, the changes here fix such +incorrect behavior. + +For example, given a `wrangler.toml` containing the following: + +``` +[vars] +MY_VAR = "dev value" + +[env.production] +[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/packages/wrangler/src/__tests__/type-generation.test.ts b/packages/wrangler/src/__tests__/type-generation.test.ts index 435f178a15bc..39877ba9bece 100644 --- a/packages/wrangler/src/__tests__/type-generation.test.ts +++ b/packages/wrangler/src/__tests__/type-generation.test.ts @@ -634,6 +634,50 @@ describe("generateTypes()", () => { `); }); + it("should produce unions where appropriate for vars present in multiple environments", async () => { + 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" + ); + + 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)\\"}; + } + " + `); + }); + describe("customization", () => { describe("env", () => { it("should allow the user to customize the interface name", async () => { diff --git a/packages/wrangler/src/type-generation/index.ts b/packages/wrangler/src/type-generation/index.ts index b3ef27d136a7..dce1220345b5 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 { @@ -113,11 +113,9 @@ export async function typesHandler( true ) as Record; - const configBindingsWithSecrets: Partial & { - secrets: Record; - } = { + const configBindingsWithSecrets = { kv_namespaces: config.kv_namespaces ?? [], - vars: { ...config.vars }, + vars: getVarsInfo(args), wasm_modules: config.wasm_modules, text_blobs: { ...config.text_blobs, @@ -207,15 +205,29 @@ export function generateImportSpecifier(from: string, to: string) { */ export function constructType( key: string, - value: string | number | boolean, + value: string | number | boolean | string[], useRawVal = true ) { const typeKey = constructTypeKey(key); - if (typeof value === "string") { + + const stringValue = + typeof value === "string" + ? value + : Array.isArray(value) && value.length === 1 + ? value[0] + : null; + + if (stringValue) { + if (useRawVal) { + return `${typeKey}: ${stringValue};`; + } + return `${typeKey}: ${JSON.stringify(stringValue)};`; + } + if (Array.isArray(value)) { if (useRawVal) { - return `${typeKey}: ${value};`; + return `${typeKey}: ${value.join(" | ")};`; } - return `${typeKey}: ${JSON.stringify(value)};`; + return `${typeKey}: ${value.map((str) => JSON.stringify(str)).join("|")};`; } if (typeof value === "number" || typeof value === "boolean") { return `${typeKey}: ${value};`; @@ -225,8 +237,12 @@ export function constructType( type Secrets = Record; +type ConfigToDTS = Partial> & { vars: VarsInfo } & { + secrets: Secrets; +}; + async function generateTypes( - configToDTS: Partial & { secrets: Secrets }, + configToDTS: ConfigToDTS, config: Config, envInterface: string, outputPath: string @@ -275,19 +291,25 @@ 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, varInfo] of vars) { + const varValueTypes = new Set( + varInfo + .map(({ value }) => value) + .map((varValue) => { + if ( + typeof varValue === "string" || + typeof varValue === "number" || + typeof varValue === "boolean" + ) { + return `"${varValue}"`; + } + if (typeof varValue === "object" && varValue !== null) { + return `${JSON.stringify(varValue)}`; + } + }) + .filter(Boolean) + ) as Set; + envTypeStructure.push(constructType(varName, [...varValueTypes], true)); } } @@ -578,3 +600,30 @@ type TSConfig = { types: string[]; }; }; + +type VarValue = Config["vars"][string]; + +type VarInfoValue = { value: VarValue; env?: string }; + +type VarsInfo = Record; + +function getVarsInfo( + args: StrictYargsOptionsToInterface +): VarsInfo { + const varsInfo: VarsInfo = {}; + const { rawConfig } = experimental_readRawConfig(args); + + function collectVars(vars: RawEnvironment["vars"], envName?: string) { + Object.entries(vars ?? {}).forEach(([key, value]) => { + varsInfo[key] ??= []; + varsInfo[key].push({ value, env: envName }); + }); + } + + collectVars(rawConfig.vars); + Object.entries(rawConfig.env ?? {}).forEach(([envName, env]) => { + collectVars(env.vars, envName); + }); + + return varsInfo; +} From 10a879747e264793fef6bf52f641dcdb63f9b0a0 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Wed, 1 Jan 2025 15:07:49 +0100 Subject: [PATCH 02/12] add `loose-vars` option to `wrangler types` --- .changeset/unlucky-timers-sort.md | 46 +++++++ .../src/__tests__/type-generation.test.ts | 121 ++++++++++++------ .../wrangler/src/type-generation/index.ts | 24 +++- 3 files changed, 153 insertions(+), 38 deletions(-) create mode 100644 .changeset/unlucky-timers-sort.md diff --git a/.changeset/unlucky-timers-sort.md b/.changeset/unlucky-timers-sort.md new file mode 100644 index 000000000000..7f28bc596ec1 --- /dev/null +++ b/.changeset/unlucky-timers-sort.md @@ -0,0 +1,46 @@ +--- +"wrangler": minor +--- + +add `loose-vars` option to `wrangler types` + +add a new `--loose-vars` option to `wrangler types` that developers can use to get more +loose types for their variables instead of literal and union types + +these more loose types 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 --loose-vars` would instead generate: + +``` +interface Env { + MY_VARIABLE: string; + MY_NUMBERS: number[]; +} +``` + +(allowing the developer to easily change their toml variables without the +risk of braking typescript types) diff --git a/packages/wrangler/src/__tests__/type-generation.test.ts b/packages/wrangler/src/__tests__/type-generation.test.ts index 39877ba9bece..b426840bda92 100644 --- a/packages/wrangler/src/__tests__/type-generation.test.ts +++ b/packages/wrangler/src/__tests__/type-generation.test.ts @@ -602,6 +602,35 @@ describe("generateTypes()", () => { `); }); + it("should respect the loose-vars option", 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 --loose-vars"); + + 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,48 +663,68 @@ describe("generateTypes()", () => { `); }); - it("should produce unions where appropriate for vars present in multiple environments", async () => { - 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], - }, + 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"], }, - staging: { - vars: { - MY_VAR_A: "A (stag)", + 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" - ); + } as TOML.JsonMap), + "utf-8" + ); + }); - await runWrangler("types"); + it("should produce string and union types for variables (default)", async () => { + await runWrangler("types"); - expect(std.out).toMatchInlineSnapshot(` - "Generating project 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)\\"}; - } - " - `); + 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 loose types for variables (with --loose-vars)", async () => { + await runWrangler("types --loose-vars=true"); + + 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", () => { diff --git a/packages/wrangler/src/type-generation/index.ts b/packages/wrangler/src/type-generation/index.ts index dce1220345b5..2b821f959c8a 100644 --- a/packages/wrangler/src/type-generation/index.ts +++ b/packages/wrangler/src/type-generation/index.ts @@ -38,6 +38,12 @@ export function typesOptions(yargs: CommonYargsArgv) { type: "string", describe: "The path of the generated runtime types file", demandOption: false, + }) + .option("loose-vars", { + type: "boolean", + default: false, + describe: + 'Generate "loose"/generic types instead of literal and union types for variables', }); } @@ -147,7 +153,8 @@ export async function typesHandler( configBindingsWithSecrets, config, envInterface, - outputPath + outputPath, + args.looseVars ); } @@ -245,7 +252,8 @@ async function generateTypes( configToDTS: ConfigToDTS, config: Config, envInterface: string, - outputPath: string + outputPath: string, + looseVars: boolean ) { const configContainsEntrypoint = config.main !== undefined || !!config.site?.["entry-point"]; @@ -296,6 +304,18 @@ async function generateTypes( varInfo .map(({ value }) => value) .map((varValue) => { + if (looseVars) { + if (Array.isArray(varValue)) { + const typesInArray = [ + ...new Set(varValue.map((item) => typeof item)), + ].sort(); + if (typesInArray.length === 1) { + return `${typesInArray[0]}[]`; + } + return `(${typesInArray.join("|")})[]`; + } + return typeof varValue; + } if ( typeof varValue === "string" || typeof varValue === "number" || From ada4e7d42a271188c76fdd32398a69e36619c2e9 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Thu, 2 Jan 2025 14:22:44 +0100 Subject: [PATCH 03/12] fixup! add `loose-vars` option to `wrangler types` rename `loose-vars` to `strict-vars` --- .changeset/unlucky-timers-sort.md | 10 +++++----- .../wrangler/src/__tests__/type-generation.test.ts | 8 ++++---- packages/wrangler/src/type-generation/index.ts | 13 ++++++------- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/.changeset/unlucky-timers-sort.md b/.changeset/unlucky-timers-sort.md index 7f28bc596ec1..e6ed1a972431 100644 --- a/.changeset/unlucky-timers-sort.md +++ b/.changeset/unlucky-timers-sort.md @@ -2,12 +2,12 @@ "wrangler": minor --- -add `loose-vars` option to `wrangler types` +add `strict-vars` option to `wrangler types` -add a new `--loose-vars` option to `wrangler types` that developers can use to get more -loose types for their variables instead of literal and union types +add a new `--strict-vars` option to `wrangler types` that developers can use to enable/disable +more strict/literal types for their variables -these more loose types can be useful when developers change often their `vars` values, +opting out of strict variables can be useful when developers change often their `vars` values, even more so when multiple environments are involved ## Example @@ -33,7 +33,7 @@ interface Env { } ``` -while `wrangler types --loose-vars` would instead generate: +while `wrangler types --strict-vars=false` would instead generate: ``` interface Env { diff --git a/packages/wrangler/src/__tests__/type-generation.test.ts b/packages/wrangler/src/__tests__/type-generation.test.ts index b426840bda92..13fa14a777f9 100644 --- a/packages/wrangler/src/__tests__/type-generation.test.ts +++ b/packages/wrangler/src/__tests__/type-generation.test.ts @@ -602,7 +602,7 @@ describe("generateTypes()", () => { `); }); - it("should respect the loose-vars option", async () => { + it("should allow opting out of strict-vars", async () => { fs.writeFileSync( "./wrangler.toml", TOML.stringify({ @@ -616,7 +616,7 @@ describe("generateTypes()", () => { "utf-8" ); - await runWrangler("types --loose-vars"); + await runWrangler("types --strict-vars=false"); expect(std.out).toMatchInlineSnapshot(` "Generating project types... @@ -710,8 +710,8 @@ describe("generateTypes()", () => { `); }); - it("should produce loose types for variables (with --loose-vars)", async () => { - await runWrangler("types --loose-vars=true"); + 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... diff --git a/packages/wrangler/src/type-generation/index.ts b/packages/wrangler/src/type-generation/index.ts index 2b821f959c8a..c387664d8dfd 100644 --- a/packages/wrangler/src/type-generation/index.ts +++ b/packages/wrangler/src/type-generation/index.ts @@ -39,11 +39,10 @@ export function typesOptions(yargs: CommonYargsArgv) { describe: "The path of the generated runtime types file", demandOption: false, }) - .option("loose-vars", { + .option("strict-vars", { type: "boolean", - default: false, - describe: - 'Generate "loose"/generic types instead of literal and union types for variables', + default: true, + describe: "Generate literal and union types for variables", }); } @@ -154,7 +153,7 @@ export async function typesHandler( config, envInterface, outputPath, - args.looseVars + args.strictVars ); } @@ -253,7 +252,7 @@ async function generateTypes( config: Config, envInterface: string, outputPath: string, - looseVars: boolean + strictVars: boolean ) { const configContainsEntrypoint = config.main !== undefined || !!config.site?.["entry-point"]; @@ -304,7 +303,7 @@ async function generateTypes( varInfo .map(({ value }) => value) .map((varValue) => { - if (looseVars) { + if (!strictVars) { if (Array.isArray(varValue)) { const typesInArray = [ ...new Set(varValue.map((item) => typeof item)), From c262fc5d9150a9669158d5993bd5cccf5cdf5b2c Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Tue, 7 Jan 2025 15:27:58 +0100 Subject: [PATCH 04/12] Update .changeset/unlucky-timers-sort.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Somhairle MacLeòid --- .changeset/unlucky-timers-sort.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/unlucky-timers-sort.md b/.changeset/unlucky-timers-sort.md index e6ed1a972431..e21100c8aa6d 100644 --- a/.changeset/unlucky-timers-sort.md +++ b/.changeset/unlucky-timers-sort.md @@ -43,4 +43,4 @@ interface Env { ``` (allowing the developer to easily change their toml variables without the -risk of braking typescript types) +risk of breaking typescript types) From 2b1fd3e5fd2bf8b5378bdfc208b41efe8280f6aa Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Tue, 7 Jan 2025 15:30:06 +0100 Subject: [PATCH 05/12] remove unnecessary double `map` --- packages/wrangler/src/type-generation/index.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/wrangler/src/type-generation/index.ts b/packages/wrangler/src/type-generation/index.ts index c387664d8dfd..bf2c953bd5c5 100644 --- a/packages/wrangler/src/type-generation/index.ts +++ b/packages/wrangler/src/type-generation/index.ts @@ -301,8 +301,7 @@ async function generateTypes( for (const [varName, varInfo] of vars) { const varValueTypes = new Set( varInfo - .map(({ value }) => value) - .map((varValue) => { + .map(({ value: varValue }) => { if (!strictVars) { if (Array.isArray(varValue)) { const typesInArray = [ From 52648c67fffdac2fdb9fb272778a5fc0439b3295 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Tue, 7 Jan 2025 18:21:59 +0100 Subject: [PATCH 06/12] fix improve and simplify types generation logic --- .../src/__tests__/type-generation.test.ts | 80 +++++----- .../wrangler/src/type-generation/index.ts | 147 ++++++++---------- 2 files changed, 109 insertions(+), 118 deletions(-) diff --git a/packages/wrangler/src/__tests__/type-generation.test.ts b/packages/wrangler/src/__tests__/type-generation.test.ts index 13fa14a777f9..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( @@ -663,6 +625,46 @@ 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( @@ -861,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 bf2c953bd5c5..2abe0043b9a1 100644 --- a/packages/wrangler/src/type-generation/index.ts +++ b/packages/wrangler/src/type-generation/index.ts @@ -205,42 +205,6 @@ 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 | string[], - useRawVal = true -) { - const typeKey = constructTypeKey(key); - - const stringValue = - typeof value === "string" - ? value - : Array.isArray(value) && value.length === 1 - ? value[0] - : null; - - if (stringValue) { - if (useRawVal) { - return `${typeKey}: ${stringValue};`; - } - return `${typeKey}: ${JSON.stringify(stringValue)};`; - } - if (Array.isArray(value)) { - if (useRawVal) { - return `${typeKey}: ${value.join(" | ")};`; - } - return `${typeKey}: ${value.map((str) => JSON.stringify(str)).join("|")};`; - } - if (typeof value === "number" || typeof value === "boolean") { - return `${typeKey}: ${value};`; - } - return `${typeKey}: unknown;`; -} - type Secrets = Record; type ConfigToDTS = Partial> & { vars: VarsInfo } & { @@ -285,11 +249,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", + ]); } } @@ -314,25 +281,31 @@ async function generateTypes( } return typeof varValue; } - if ( - typeof varValue === "string" || - typeof varValue === "number" || - typeof varValue === "boolean" - ) { - return `"${varValue}"`; + if (typeof varValue === "number" || typeof varValue === "boolean") { + return `${varValue}`; } - if (typeof varValue === "object" && varValue !== null) { - return `${JSON.stringify(varValue)}`; + if (typeof varValue === "string" || typeof varValue === "object") { + return JSON.stringify(varValue); } + return "unknown"; }) .filter(Boolean) ) as Set; - envTypeStructure.push(constructType(varName, [...varValueTypes], true)); + + const typeKey = constructTypeKey(varName); + const constructedValues = [...varValueTypes]; + + envTypeStructure.push([ + typeKey, + constructedValues.length === 1 + ? constructedValues[0] + : constructedValues.join(" | "), + ]); } } for (const secretName in configToDTS.secrets) { - envTypeStructure.push(constructType(secretName, "string", true)); + envTypeStructure.push([constructTypeKey(secretName), "string"]); } if (configToDTS.durable_objects?.bindings) { @@ -355,66 +328,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"]); } } } @@ -422,32 +397,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", + ]); } } @@ -455,28 +439,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"]); } } @@ -517,7 +506,7 @@ function writeDTSFile({ envInterface, path, }: { - envTypeStructure: string[]; + envTypeStructure: [string, string][]; modulesTypeStructure: string[]; formatType: CfScriptFormat; envInterface: string; @@ -550,7 +539,7 @@ function writeDTSFile({ const { fileContent, consoleOutput } = generateTypeStrings( formatType, envInterface, - envTypeStructure, + envTypeStructure.map(([key, value]) => `${key}: ${value};`), modulesTypeStructure ); From 7e57589b9733fdd09396366a8884b8bd1feb4877 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Tue, 7 Jan 2025 18:29:26 +0100 Subject: [PATCH 07/12] Update .changeset/proud-forks-build.md Co-authored-by: Pete Bacon Darwin --- .changeset/proud-forks-build.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.changeset/proud-forks-build.md b/.changeset/proud-forks-build.md index 20ba9b3492c0..6f82d5862592 100644 --- a/.changeset/proud-forks-build.md +++ b/.changeset/proud-forks-build.md @@ -4,10 +4,8 @@ fix: widen multi-env `vars` types in `wrangler types` -Currently types for variable generate string literal, those are appropriate when -a single environment has been specified in the config file but if multiple environments -are specified this however wrongly restricts the typing, the changes here fix such -incorrect behavior. +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: From 1d8fb94b0fd1624a79ac6bb9373533b618c1a7cc Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Tue, 7 Jan 2025 18:31:09 +0100 Subject: [PATCH 08/12] Update .changeset/unlucky-timers-sort.md Co-authored-by: Pete Bacon Darwin --- .changeset/unlucky-timers-sort.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/unlucky-timers-sort.md b/.changeset/unlucky-timers-sort.md index e21100c8aa6d..ca3ac6032140 100644 --- a/.changeset/unlucky-timers-sort.md +++ b/.changeset/unlucky-timers-sort.md @@ -2,7 +2,7 @@ "wrangler": minor --- -add `strict-vars` option to `wrangler types` +add `--strict-vars` option to `wrangler types` add a new `--strict-vars` option to `wrangler types` that developers can use to enable/disable more strict/literal types for their variables From bf39a65055db2042b2b9f43e55993fb862cd5af8 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Tue, 7 Jan 2025 18:30:47 +0100 Subject: [PATCH 09/12] remove unnecessary line --- .changeset/proud-forks-build.md | 1 - 1 file changed, 1 deletion(-) diff --git a/.changeset/proud-forks-build.md b/.changeset/proud-forks-build.md index 6f82d5862592..7dc7ff27427f 100644 --- a/.changeset/proud-forks-build.md +++ b/.changeset/proud-forks-build.md @@ -13,7 +13,6 @@ For example, given a `wrangler.toml` containing the following: [vars] MY_VAR = "dev value" -[env.production] [env.production.vars] MY_VAR = "prod value" ``` From 8a6d0dd63e451728fc0eb6bc0d311306aaff20ed Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Tue, 7 Jan 2025 18:35:06 +0100 Subject: [PATCH 10/12] clarify `--strict-vars` changeset --- .changeset/unlucky-timers-sort.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/unlucky-timers-sort.md b/.changeset/unlucky-timers-sort.md index ca3ac6032140..8c89983ac378 100644 --- a/.changeset/unlucky-timers-sort.md +++ b/.changeset/unlucky-timers-sort.md @@ -4,8 +4,8 @@ add `--strict-vars` option to `wrangler types` -add a new `--strict-vars` option to `wrangler types` that developers can use to enable/disable -more strict/literal types for their variables +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 From 41e01a6934c66f0980666fd70dfdc7163af1b593 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Tue, 7 Jan 2025 19:23:22 +0100 Subject: [PATCH 11/12] simplify logic and add comments --- .../wrangler/src/type-generation/index.ts | 124 ++++++++++-------- 1 file changed, 68 insertions(+), 56 deletions(-) diff --git a/packages/wrangler/src/type-generation/index.ts b/packages/wrangler/src/type-generation/index.ts index 2abe0043b9a1..d9d631f6f1ab 100644 --- a/packages/wrangler/src/type-generation/index.ts +++ b/packages/wrangler/src/type-generation/index.ts @@ -120,7 +120,7 @@ export async function typesHandler( const configBindingsWithSecrets = { kv_namespaces: config.kv_namespaces ?? [], - vars: getVarsInfo(args), + vars: collectAllVars(args), wasm_modules: config.wasm_modules, text_blobs: { ...config.text_blobs, @@ -152,8 +152,7 @@ export async function typesHandler( configBindingsWithSecrets, config, envInterface, - outputPath, - args.strictVars + outputPath ); } @@ -207,7 +206,7 @@ export function generateImportSpecifier(from: string, to: string) { type Secrets = Record; -type ConfigToDTS = Partial> & { vars: VarsInfo } & { +type ConfigToDTS = Partial> & { vars: VarTypes } & { secrets: Secrets; }; @@ -215,8 +214,7 @@ async function generateTypes( configToDTS: ConfigToDTS, config: Config, envInterface: string, - outputPath: string, - strictVars: boolean + outputPath: string ) { const configContainsEntrypoint = config.main !== undefined || !!config.site?.["entry-point"]; @@ -265,41 +263,10 @@ async function generateTypes( const vars = Object.entries(configToDTS.vars).filter( ([key]) => !(key in configToDTS.secrets) ); - for (const [varName, varInfo] of vars) { - const varValueTypes = new Set( - varInfo - .map(({ value: varValue }) => { - if (!strictVars) { - if (Array.isArray(varValue)) { - const typesInArray = [ - ...new Set(varValue.map((item) => typeof item)), - ].sort(); - if (typesInArray.length === 1) { - return `${typesInArray[0]}[]`; - } - return `(${typesInArray.join("|")})[]`; - } - return typeof varValue; - } - if (typeof varValue === "number" || typeof varValue === "boolean") { - return `${varValue}`; - } - if (typeof varValue === "string" || typeof varValue === "object") { - return JSON.stringify(varValue); - } - return "unknown"; - }) - .filter(Boolean) - ) as Set; - - const typeKey = constructTypeKey(varName); - const constructedValues = [...varValueTypes]; - + for (const [varName, varValues] of vars) { envTypeStructure.push([ - typeKey, - constructedValues.length === 1 - ? constructedValues[0] - : constructedValues.join(" | "), + constructTypeKey(varName), + varValues.length === 1 ? varValues[0] : varValues.join(" | "), ]); } } @@ -608,29 +575,74 @@ type TSConfig = { }; }; -type VarValue = Config["vars"][string]; - -type VarInfoValue = { value: VarValue; env?: string }; - -type VarsInfo = Record; +type VarTypes = Record; -function getVarsInfo( +/** + * 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 -): VarsInfo { - const varsInfo: VarsInfo = {}; - const { rawConfig } = experimental_readRawConfig(args); +): Record { + const varsInfo: Record> = {}; - function collectVars(vars: RawEnvironment["vars"], envName?: string) { + // 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] ??= []; - varsInfo[key].push({ value, env: envName }); + 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"); }); } - collectVars(rawConfig.vars); - Object.entries(rawConfig.env ?? {}).forEach(([envName, env]) => { - collectVars(env.vars, envName); + const { rawConfig } = experimental_readRawConfig(args); + collectEnvironmentVars(rawConfig.vars); + Object.entries(rawConfig.env ?? {}).forEach(([_envName, env]) => { + collectEnvironmentVars(env.vars); }); - return varsInfo; + 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 the 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("|")})[]`; } From 5405cd8e60162e55c1169757aaab0e2e427d3d1e Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Tue, 7 Jan 2025 22:09:10 +0100 Subject: [PATCH 12/12] update `the` to `a` --- packages/wrangler/src/type-generation/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/wrangler/src/type-generation/index.ts b/packages/wrangler/src/type-generation/index.ts index d9d631f6f1ab..04b24ea6f972 100644 --- a/packages/wrangler/src/type-generation/index.ts +++ b/packages/wrangler/src/type-generation/index.ts @@ -635,7 +635,7 @@ function collectAllVars( * `['false', true]` returns `(string|boolean)[]`, * * @param array the target array - * @returns the string representing the types of such 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();