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

feat(dotenv): add the expectVars option for typing of return value of load() #4447

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pasha-bolokhov
Copy link

@pasha-bolokhov pasha-bolokhov commented Mar 6, 2024

The requested feature that enables you to type the return value of load()

const { SERVER, PORT } = await load({
  envPath: ".env", 
  defaultsPath: ".env.defaults", 
  expectVars: ["SERVER", "PORT"] as const,
});

If set, expectVars does two things:

  • performs the actual check that the variables are there
  • sets the return type to Record<"SERVER" | "PORT", string>

The check is performed regardless of whether the example file has been provided or not in the options — this is for simplicity of the algorithm, and among other things, it also confirms that the example file is up to date with the code.

@CLAassistant
Copy link

CLAassistant commented Mar 6, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added the dotenv label Mar 6, 2024
@kt3k
Copy link
Member

kt3k commented Mar 7, 2024

I'm not sure these checks need to be done as part of load. The checks can be done after loading env vars like the below example, which feels more explicit and flexible to me.

const vars = await load({
  envPath: ".env", 
  defaultsPath: ".env.defaults", 
});

assertExists(vars["SERVER"]);
assertExists(vars["PORT"]);

@pasha-bolokhov
Copy link
Author

This is the feature that I would definitely use, to be honest.

This is the reference to one of the original requests, before dotenv was merged into deno_std:
pietvanzoen/deno-dotenv#58

I do think that this goes along with idea of automatizing of unpacking of .env variables. Yes, you could explicitly check that certain variables exist, but nobody does

The original idea was to get strong typing, and the only way to get strong typing is by listing the variables that you expect to have. And then if you do get the list, why not check that there are indeed there?

While you can use strong typing (via expectVars), you don't have to, if you omit expectVars option, in which case you get the standard load() behaviour

@iuioiua
Copy link
Collaborator

iuioiua commented Mar 10, 2024

This overlaps with the purpose of LoadOptions.examplePath too much. I agree with Yoshiya.

@pasha-bolokhov
Copy link
Author

This overlaps with the purpose of LoadOptions.examplePath too much. I agree with Yoshiya.

Well, the main purpose is the strong typing. The additional check of the existence of variables may be considered as an overlap (although, again the presence of the "example" file does not guarrantee the existence if the "example" file itself is out of date). If you do think that these checks are indeed duplicating the purpose of the "example" file, I can remove them

But again — the original and main purpose is to provide the typing of the return value, which should help avoid typos in variable names. It does seem natural to me that load() knows the return value it returns

Let me know…

@kt3k
Copy link
Member

kt3k commented Apr 17, 2024

This kind of type narrowing can be done in a more generic way like zod or valibot.

For example, the below example gives a nice typing to options (which is inferred as { HOST: string; PORT: number }:

import z from "npm:zod";
import { load } from "jsr:@std/dotenv@^0";

const Options = z.object({
  HOST: z.string(),
  PORT: z.coerce.number(),
});

const options = Options.parse(await load());

I think we should rather explore these kinds of solution (or just recommend zod, valibot, etc because these tools tend to have to have an opinionated design)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants