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

Type proccess.env based off the available secrets in a notebook #285

Open
versecafe opened this issue Sep 13, 2024 · 8 comments
Open

Type proccess.env based off the available secrets in a notebook #285

versecafe opened this issue Sep 13, 2024 · 8 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers hacktoberfest

Comments

@versecafe
Copy link
Contributor

image Based off secrets set in a notebook, add them to process.env type, this way proper warnings are given if a secret is not set but is used in the code
@benjreinhart
Copy link
Contributor

Adding some more clarity here in case it's helpful:

Secrets now need to be explicitly enabled per Srcbook. We can use this information to instruct typescript what to expect in the global process.env. This adds some nice typing support and errors for env variables, e.g.:

You can imagine a user importing a srcbook that's expecting secrets. Once they import and dependencies are installed, they'll see type errors in the code cells that inform them env variables are missing.

@Sanjoy-droid
Copy link
Contributor

@benjreinhart could you assign me on this issue I'd love to work on this

@versecafe
Copy link
Contributor Author

@Sanjoy-droid Feel free to take a go at it, if you need help reach out to @benjreinhart @nichochar or me, here's a reference of how to generate the types nicely to some capacity, you can take other approaches as well since this is for a stricter setup where there are constraints around types, lengths, prefixes, etc but shows how to type override process.env.*

import { z } from "zod";

export const envSchema = z.object({
  NODE_ENV: z
    .enum(["development", "production", "test"])
    .default("development"),
  VERCEL_ENV: z
    .enum(["production", "preview", "development"])
    .default("development"),
  CRON_SECRET: z.string().length(32),
  DATABASE_URL: z.string(),
  DATABASE_HOST: z.string(),
  DATABASE_USER: z.string(),
  DATABASE_PASSWORD: z.string(),
  DATABASE_NAME: z.string(),
  SITE_URL: z.string().default("http://localhost:3000"),
  SITE_LOCATION: z.string().default("localhost"),
  GITHUB_CLIENT_ID: z.string(),
  GITHUB_CLIENT_SECRET: z.string(),
  UPSTASH_REDIS_REST_URL: z.string(),
  UPSTASH_REDIS_REST_TOKEN: z.string(),
});

export type Env = z.infer<typeof envSchema>;

declare global {
  // eslint-disable-next-line @typescript-eslint/no-namespace -- intentional global override of process.env.
  namespace NodeJS {
    interface ProcessEnv extends Env {}
  }
}

@versecafe versecafe added enhancement New feature or request good first issue Good for newcomers labels Sep 21, 2024
@Sanjoy-droid
Copy link
Contributor

@versecafe when users imports srcbook that expects secrets but they are not set at global, I was able to get error/warn in console but not as type errors as @benjreinhart mentioned above.

Anyhow I am checking it out

@Sanjoy-droid Sanjoy-droid removed their assignment Sep 27, 2024
@Sanjoy-droid
Copy link
Contributor

Hi @versecafe

I wanted to let you know that I’ll be unassigning myself from this issue. I tried to work on it, but I wasn’t able to make progress. Thank you for giving me an opportunity to work on this , and I hope to contribute in the future!

@Lokendra-sinh
Copy link

Lokendra-sinh commented Oct 2, 2024

@nichochar could you please assign this to me...after @Sanjoy-droid dropping it I took a stab at it and here are my findings.

@benjreinhart FYI

We can extend the ProcessEnv interface but the issue is...the index signature defined in the "ProcessEnv" interface.

interface ProcessEnv {
  [key: string]: string | undefined;
}

This property allows access to any random env/secret even if we extend it, not allowing typescript to catch type errors. Surely, we can override it...but then we're not sure how this will affect third -party libraries we might be using in our code.

I was able to get the type errors by overriding the index signature.

declare global {
  namespace NodeJS {
    // Completely redefine ProcessEnv without the index signature to make sure typescript throws type errors
    interface ProcessEnv {
      NODE_ENV: 'development' | 'production' | 'test';

      API_KEY: string;
      DB_PASSWORD: string;

    }
  }
}

export {};

index.ts:

image

@benjreinhart @versecafe would love to know your thoughts on this?

@Lokendra-sinh
Copy link

Lokendra-sinh commented Oct 2, 2024

Hey @nichochar @versecafe. So I started working on the issue and here's my progress. I would really like to know the "smart way" and be proven wrong!

  1. I created a static env.ts file with some dummy env secrets.
  2. Inserted it under src so it's available during the compile time
image
  1. Static processEnv.ts file just to test if it's working or not
image

Based on the secrets in the srcbook, we have to dynamically generate the processEnv.ts file to make sure typescript's language service has access to all the latest environment variables.

@Sanjoy-droid
Copy link
Contributor

is this issue fixed? then shouldn't it be closed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers hacktoberfest
Projects
None yet
Development

No branches or pull requests

5 participants