Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 12 commits into from
Jan 15, 2025
Merged
36 changes: 36 additions & 0 deletions .changeset/proud-forks-build.md
Original file line number Diff line number Diff line change
@@ -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";
}
```
46 changes: 46 additions & 0 deletions .changeset/unlucky-timers-sort.md
Original file line number Diff line number Diff line change
@@ -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)
173 changes: 134 additions & 39 deletions packages/wrangler/src/__tests__/type-generation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import * as fs from "fs";
import * as TOML from "@iarna/toml";
import {
constructTSModuleGlob,
constructType,
constructTypeKey,
generateImportSpecifier,
isValidIdentifier,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -634,6 +625,110 @@ describe("generateTypes()", () => {
`);
});

it("various different types of vars", async () => {
dario-piotrowicz marked this conversation as resolved.
Show resolved Hide resolved
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 () => {
dario-piotrowicz marked this conversation as resolved.
Show resolved Hide resolved
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 () => {
Expand Down Expand Up @@ -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({
Expand Down
Loading
Loading