allan2 / dotenvy

A well-maintained fork of the dotenv crate
MIT License
713 stars 45 forks source link

`EnvLoader::new().load()?` should not fail even if `.env` file does not present. #134

Open simnalamburt opened 1 week ago

simnalamburt commented 1 week ago

It should proceed and silently defaults to std::env::vars, so that users can use dotenvy without .env file. Almost all dotenv implementations in various programming languages function normally even without a .env file.

allan2 commented 1 week ago

This has always been the default behaviour of dotenv/dotenvy.

v0.15 or earlier

fn main() {
    dotenvy::dotenv().unwrap();
}
thread 'main' panicked at src/main.rs:2:23:
called `Result::unwrap()` on an `Err` value: Io(Custom { kind: NotFound, error: "path not found" })

The new EnvLoader can be used without an env file by configuring EnvSequence.

simnalamburt commented 1 week ago

I wrote the code below, but there was no option to make the .env file optional.

use dotenvy::{EnvLoader, EnvSequence};

fn main() {
    EnvLoader::new().sequence(EnvSequence::EnvOnly).load().unwrap(); // Not panics
    EnvLoader::new().sequence(EnvSequence::EnvThenInput).load().unwrap(); // Panics
    EnvLoader::new().sequence(EnvSequence::InputThenEnv).load().unwrap(); // Panics
    EnvLoader::new().sequence(EnvSequence::InputOnly).load().unwrap(); // Panics
    println!("Hello, world!");
}

This seems unintuitive from the user’s perspective. The dotenvy-macros crate has an option called "required" that makes the .env file optional. What do you think about adding a "required" option to dotenvy as well?

// sample usage
let env = EnvLoader::new().required(false).load()?;
References
allan2 commented 1 week ago

EnvSequence expresses more than just required. It is more like:

loader.required(false).existing_env(true).override(false)

Having the configuration as a type is nice. It allows for this --> dev/prod example

The config syntax of [#load] being different is an issue though. I am thinking to have [#load] take a singular sequence parameter, aligning it with EnvSequence, and adding more documentation.

Thoughts?

simnalamburt commented 1 week ago
loader.required(false).existing_env(true).override(false)

Having the configuration as a type is nice.

Cool. It seems like this would allow for all possible cases to be expressed, and it would greatly increase user flexibility as well.

I am thinking to have [#load] take a singular sequence parameter, aligning it with EnvSequence, and adding more documentation.

It sounds good, but the types of values that can be passed as parameters within an attribute are extremely limited, so I’m not sure if it’s feasible to implement. If it’s possible, it would indeed be great.