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

🚀 Feature Request: Generate more general types for wrangler types #6303

Closed
stevezhu opened this issue Jul 21, 2024 · 2 comments
Closed

🚀 Feature Request: Generate more general types for wrangler types #6303

stevezhu opened this issue Jul 21, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@stevezhu
Copy link

stevezhu commented Jul 21, 2024

Describe the solution

Problem

Wrangler generates the exact type for environment variables defined. For example, the following wrangler.toml:

[vars]
AUTH_DOMAIN = "https://www.example.com"

Generates this type:

interface Env {
  AUTH_DOMAIN: "https://www.example.com";
}

It should be assumed that environment variables are able to change, otherwise they would just be defined as constants instead. This kind of type generation seems to make more sense:

 interface Env {
-  AUTH_DOMAIN: "https://www.example.com";
+  AUTH_DOMAIN: string;
 }

I don't think it's correct to assume that the values defined in wrangler.toml are the only possible values for the environment variables.

Secrets

Another issue is that secrets are only defined inside .dev.vars which isn't checked into version control. This creates issues for CI where the type generation isn't idemponent because .dev.vars doesn't exist in the CI environment. Changing the type generation to be more general will allow placeholder values to be defined in wrangler.toml for types.

eg.

[vars]
AUTH_DOMAIN = "https://www.example.com"
AUTH_SECRET = "<secret>"
interface Env {
  AUTH_DOMAIN: string;
  AUTH_SECRET: string;
}

Different environments

This would also solve this relevant issue: #5850

Suggestion

Reference:

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;`;
}

It makes more sense to me for the implementation of constructTypes to be something more like this

 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};`;
-  }
+  if (typeof value === "string" || typeof value === "number" || typeof value === "boolean") {
+    if (useRawVal) {
+      return `${typeKey}: ${value};`;
+    }
+    return `${typeKey}: ${typeof value};`;
+  }
   return `${typeKey}: unknown;`;
 }

Changing the implementation or adding an extra flag to wrangler types to enable this behavior would be greatly appreciated.

@stevezhu stevezhu added the enhancement New feature or request label Jul 21, 2024
@github-project-automation github-project-automation bot moved this to Untriaged in workers-sdk Jul 21, 2024
@stevezhu
Copy link
Author

Realized a related PR exists here: #5086
And a comment on the same suggestion as this issue: #5086 (comment)

@andyjessop
Copy link
Contributor

I'll close this as it's a duplicate of #5086, which is implementing the same suggestion.

@github-project-automation github-project-automation bot moved this from Untriaged to Done in workers-sdk Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

No branches or pull requests

2 participants