From 376a46be5feb7394bcd5b1b32f5c0efca28daf22 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Wed, 1 Jan 2025 13:43:19 +0100 Subject: [PATCH] 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 79268ed7dbc9..901d93eb7f1f 100644 --- a/packages/wrangler/src/__tests__/type-generation.test.ts +++ b/packages/wrangler/src/__tests__/type-generation.test.ts @@ -635,6 +635,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 347b55f8cec7..9da51f66e58a 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 { resolveWranglerConfigPath } from "../config/config-helpers"; import { getEntry } from "../deployment-bundle/entry"; import { getVarsForDev } from "../dev/dev-vars"; @@ -12,7 +12,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; +}