Added support for .env file

[tavoyne]

What's the problem this PR addresses?

Fixes #2724.

It was impossible to tell Yarn to load .env variables before running commands.

Typical use case is for NPM auth tokens that may differ from a project to an other, you want them to be stored locally in your project rather than in a .bashrc file for example.

Why not setting environment variables in a .bashrc/.zshrc file? This makes your project rely on external (invisible to you) files, and defeat the purpose of Yarn to make projects amazingly self-contained. Plus, it makes it impossible to have project-specific variables.

How did you fix it?

1. I added dotenv as a dependency of yarnpkg-cli.
2. I added the dotenvPath configuration key to yarnpkg-core/sources/Configuration.ts.
3. I implemented the following logic in yarnpkg-cli/sources/main.ts:

const dotenvPath = configuration.get(`dotenvPath`);
if (dotenvPath) {
  if (!xfs.existsSync(dotenvPath)) {
    process.stdout.write(cli.error(new Error(`The "dotenvPath" option has been set (in ${configuration.sources.get(`dotenvPath`)}), but the specified location doesn't exist (${dotenvPath}).`)));
    process.exitCode = 1;
  } else {
    dotenv.config({path: dotenvPath});
  }
}

4. I finally added the configuration key to the documentation (gatsby/static/configuration/yarnrc.json).

➡️ How to use this setting?

# ./.yarnrc.yml

# ...
dotenvPath: ./.env

// ./.env

NPM_AUTH_TOKEN=npm_123

And now, running yarn run or accessing process.env.NPM_AUTH_TOKEN in a Node module (that's ran by Yarn) does give us the expected npm_123 output.

If there's no file at dotenvPath, an exception is thrown (similar to what happen when yarnPath is unresolvable).

☀️ Note: if no dotenvPath is provided, nothing happens at all. So it's an opt-in, non-breaking feature. It won't bother current users who might have a .env file but don't expect it to be checked in.

Checklist

• I have read the Contributing Guide.

• I have set the packages that need to be released for my changes to be effective.

• I will check that all automated PR checks pass before the PR gets reviewed.

[paul-soporan]

While I agree that scoping environment variables per-project is better than using shell configuration files and I have no problem with supporting .env in core, I don't think this is the right approach.

• Injecting it in the CLI entry point makes users think Yarn supports .env while only the CLI does, anything using the API would not load it.
• I believe the right approach would be to use dotenv in:
  1. Configuration.find, but returning a new env that's passed down to the functions that expect an env rather than injecting it into process.env since Configuration.find shouldn't produce a side effect.
  2. The setupScriptEnvironment hook (which injects it into all of your scripts and commands that use an env) with the same mention, not mutating process.env.
• If we support it in core I don't see any value in not enabling it by default and having it be opt-in.

[tavoyne]

I agree with you @paul-soporan, what I did is a bit hacky and not a direct fix.

I also realised afterwards that it wasn't making .env variables available to use inside the .yarnrc.yml file, which is a shame since what we'd typically want to be able to do is to have e.g. NPM_TOKEN=npm_123 in .env and then consume it in .yarnrc.yml that way: npmAuthToken: ${NPM_TOKEN}.

What you suggest seems way more solid. I'd have to dig a bit into it as I'm not that familiar with the codebase, but I'd be happy to give it a try.

In terms of enabling this feature by default, we'd just have to set ./.env as the default dotenvPath value. The only possible problem that may arise is for users who have a root .env but don't expect Yarn and their Node program to be aware of it. They might upgrade without seeing this change (which might be a breaking one for them). I'm not sure exactly in what scenario this could be troublesome, but I prefer to mention it.

[chadmorrow]

I've been working on this myself off and on as a way of getting more familiar with the code base in general and have some questions.

If the env file is read in with Configuration.find, then the env file is updated, would the updates be reflected in yarn? How often, and when, is that find function called?

I was looking at implementing this as a plug-in but couldn't work out how to modify the configuration from the plugin without adding a command that would have to be manually invoked. Adding the hook for setupScriptEnviroment seemed straightforward though. Is a plug-in capable of offering this functionality or would it need to go into the core?

[alubbe]

We thought about putting this functionality into a plugin as well - but that would mean that the env variables would not be available inside of yarnrc, because that is where yarn learns about the plugins in the first place. So it probably has to go into core.

[tavoyne]

If the env file is read in with Configuration.find, then the env file is updated, would the updates be reflected in yarn? How often, and when, is that find function called?

The config is loaded every time you run a Yarn command. So a change in .env should be reflected immediately, i.e. in all subsequent commands, I guess.

I agree with you, @alubbe, it should go into core.

[chadmorrow]

1. Configuration.find, but returning a new env that's passed down to the functions that expect an env rather than injecting it into process.env since Configuration.find shouldn't produce a side effect.

Questions about this part. When you refer to "the functions that expect an env", would an example of one of those functions be something like Configuration.importSettings? That function takes an object with string keys and SettingsDefinition values. Would the right way to import from the dotenv file be to parse the file and turn the entries into SettingsDefinitions and then pass them to importSettings? What would the description be in that case for each one? A blank string? The type would probably need to be SettingsType.STRING then right?

I may be overthinking this but I wrote this and it looked a lot like the importSettings function so I'm betting it shouldn't be done exactly like this:

import {parse as dotenvParse} from 'dotenv';

...

  private importDotenvSettings() {
    const dotenvFile = this.values.get(`dotenvFilePath`);

    if (dotenvFile) {
      if (!xfs.existsSync(dotenvFile)) {
        throw new Error(`The "dotenvPath" option has been set (in ${this.sources.get(`dotenvFilePath`)}), but the specified location doesn't exist (${dotenvFile}).`);
      } else {
        const parsedDotenvFile = dotenvParse(xfs.readFileSync(dotenvFile));
        for (const [key, value] of Object.entries(parsedDotenvFile)) {
          if (this.settings.has(key))
            throw new Error(`Cannot redefine settings "${key}"`);

          const definition: SettingsDefinition = {description: ``, default: null, type: SettingsType.STRING} ;
          this.settings.set(key, definition);
          this.values.set(key, value);
        }
      }
    }
  }

[chadmorrow]

Another concern about implementing this like how I describe above is that the values loaded from the env file would then be able to be mutated by doing yarn config set x which probably shouldn't happen unless that change was also then written back to the .env file.

I don't know what the right path forward is on this.

[paul-soporan]

I was looking at implementing this as a plug-in but couldn't work out how to modify the configuration from the plugin without adding a command that would have to be manually invoked.

The configuration env can't be modified using a plugin currently. We could perhaps add some kind of setupConfigurationEnvironment hook to modify the configuration environment before loading it, but we should probably wait for a different use case than .env to arise since we'll implement .env support in core anyways.

We thought about putting this functionality into a plugin as well - but that would mean that the env variables would not be available inside of yarnrc, because that is where yarn learns about the plugins in the first place. So it probably has to go into core.

Yep, there's already the external https://github.com/jeysal/yarn-plugin-dotenv/ that makes .env environment variables available, but only inside scripts and it also mutates process.env.

Questions about this part. When you refer to "the functions that expect an env", would an example of one of those functions be something like Configuration.importSettings?

It wouldn't, importSettings is for importing setting definitions, not values. Definitions are what Yarn uses to check if a setting is valid and to decide how to parse a value for the corresponding setting.

.env is a file used to define environment variables, Yarn should import values from it, not definitions. When it encounters an env variable such as YARN_ENABLE_IMMUTABLE_INSTALLS=0, it should check whether an existing definition already exists, not create a new one.

The method that imports setting values is useWithSource:

berry/packages/yarnpkg-core/sources/Configuration.ts

Line 1126 in ba821ef

configuration.useWithSource(`<environment>`, excludeCoreFields(environmentSettings), startingCwd, {strict});

Another concern about implementing this like how I describe above is that the values loaded from the env file would then be able to be mutated by doing yarn config set x which probably shouldn't happen unless that change was also then written back to the .env file.

I'll move the rest of this on Discord.

[kherock]

The configuration env can't be modified using a plugin currently. We could perhaps add some kind of setupConfigurationEnvironment hook to modify the configuration environment before loading it, but we should probably wait for a different use case than .env to arise since we'll implement .env support in core anyways.

I just wanted to chime in here and mention that I'm most interested in supporting this at a plugin level where there's more freedom to be opinionated about how .env files are loaded. The repos I'm working on lately use .env.* files (like .env.local, .env.production, etc.) to easily switch between different sets of environment variables depending on the current value of NODE_ENV. Exposing a hook to allow customizing the configuration environment is exactly what I'm looking for - I'm happy to make a PR for a new hook with the same signature as setupScriptEnvironment if someone is interested in reviewing it.

[chadmorrow]

I agree with @kherock after thinking about this more. A hook that provides configuration customization would probably be more flexible and less specific to just this use case.

[arcanis]

If you see three posts marked as spam because they just ask for update in a thread where updates would be publicly visible if there was any, posting a fourth one won't be very useful.

I'm going to close this PR since the implementation isn't quite in line with what would be optimal. If someone's willing to implement in a new PR what Paul described here, we'll take another look.