widen multi-env vars types in wrangler types and add --strict-vars option to the command (#5086)

* fix: widen multi-env `vars` types in `wrangler types`

* add `strict-vars` option to `wrangler types`

---------

Co-authored-by: Somhairle MacLeòid <[email protected]>
Co-authored-by: Pete Bacon Darwin <[email protected]>

Author: 3 people

.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";
+}
+```

.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)

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({