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(core): add .env support to Configuration #4835

Closed
wants to merge 7 commits into from

Conversation

jj811208
Copy link
Contributor

@jj811208 jj811208 commented Sep 8, 2022

What's the problem this PR addresses?

Closes #4718
related issue: #2724
related PR: #3938

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.

I agree with this point because my project has the same problem.

I'm not sure about other use cases for env in yarn, but I guess as long as yarnrc.yml supports .env, it should solve most of the use cases

  • This PR does not load .production.env, .development.env, .test.env..., only .env
  • When there are multiple yarnrc.yml in the project, each yarnrc.yml will load both the .env in the same directory and the .env at the root of the project.

How did you fix it?

  1. install the dotenv@16.0.2 package 5ed9434
  2. new Configuration private method - findPackageEnv 630a7c9
  3. let the parseXxx family of functions accept env parameters 1f961ba
  4. yarnrc.yml load the .env file 1374abb
  5. expose environment information in the Configuration ec7ed66
  6. new test about the feature d5870bb d9f7ca9

Checklist

  • 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.

@merceyz merceyz self-requested a review September 8, 2022 14:32
@jj811208 jj811208 force-pushed the feat/yarnrc-support-dot-env branch from db93568 to e164f61 Compare September 8, 2022 14:42
@jj811208

This comment was marked as off-topic.

@jj811208 jj811208 marked this pull request as ready for review September 9, 2022 07:33
@jj811208 jj811208 requested a review from arcanis as a code owner September 9, 2022 07:33
@jj811208 jj811208 changed the title feat(core): make yarnrc.yml support .env feat(core): make yarnrc.yml supports .env Sep 9, 2022
@merceyz merceyz changed the title feat(core): make yarnrc.yml supports .env feat(core): add .env support to Configuration Sep 15, 2022
@jj811208 jj811208 force-pushed the feat/yarnrc-support-dot-env branch 2 times, most recently from d791aad to a8a79a0 Compare September 15, 2022 16:13
@RDIL RDIL requested a review from arcanis September 17, 2022 00:27
@jj811208 jj811208 force-pushed the feat/yarnrc-support-dot-env branch 3 times, most recently from 3b789dd to b9a2887 Compare September 22, 2022 16:25
@jj811208 jj811208 force-pushed the feat/yarnrc-support-dot-env branch from b9a2887 to a96b68c Compare October 5, 2022 12:13
@jj811208
Copy link
Contributor Author

I think this will conflict with #4912, so I'll change its state back to draft first~

after #4912 is merged, I will come back to resolve the conflict and reopen 🙂.

@jj811208 jj811208 marked this pull request as draft October 10, 2022 12:37
@jj811208 jj811208 force-pushed the feat/yarnrc-support-dot-env branch 4 times, most recently from a9cc2c7 to 8cc11b1 Compare October 14, 2022 12:18
@jj811208
Copy link
Contributor Author

jj811208 commented Oct 14, 2022

#4912 is merged 🎉!!

So I came back to this PR.

It has been more than a month since this PR was created, and during this period, I have been following yarn issues and yarn discord,

but I still don't see any use cases that need multiple .yarnrc.yml 🧐,

I think such use cases are quite rare, not to mention multiple .yarnrc.yml and multiple .env use cases,

so I think that I did originally over-design 🙈,

for these rare use cases, I made the code more complex, making the review more difficult and increasing the chances of breaking change in the future,

I prefer to make it simpler now.


I made the following changes:

1. Rename findPackageEnv to findProjectEnv and put it in miscUtils

Although the names are similar, I personally think the package smells like a dependency, and I couldn't find the reason why I made it a configuration method before🙃

2. No longer reads the .env from each workspace

This use case is too rare

3. No longer put .env in configuration.values

I think we can read .env in this way miscUtils.findProjectEnv(cwd), although inconvenient, but it can prevent breaking change and we can decide on a better way to get it in the future.

4. environmentSettings can also read .env now

I missed this before

5. Make parseSingleValue pure, pass in env from outside

I think making it a pure function, putting process.env and projectEnv in the same place to make this code easier to read.

6. Not supported for the lockfileFilename property

1. To make `.env` work for `.yarnrc.yml`, we need to read the project's `.env` first.
2. To read the project's `.env`, we need to get the project's directory first.
3. To get the project directory, we need to get `lockfileFilename` first.
4. To get the `lockfileFilename` name, load `.yarnrc.yml` first.
Loop... so I finally use `DEFAULT_LOCK_FILENAME`

If there is a clearer need for .env in the future, I would be happy to work on it again 😃.

@jj811208 jj811208 marked this pull request as ready for review October 14, 2022 12:47
@jj811208 jj811208 force-pushed the feat/yarnrc-support-dot-env branch 3 times, most recently from 5517450 to 641f53b Compare October 22, 2022 11:15
@jj811208 jj811208 force-pushed the feat/yarnrc-support-dot-env branch from 641f53b to 38c1c7d Compare October 29, 2022 07:06
@arcanis arcanis mentioned this pull request Jun 25, 2023
3 tasks
arcanis added a commit that referenced this pull request Jun 26, 2023
**What's the problem this PR addresses?**

A common need is to provide environment values into the environment via
`.env` files. There's been a couple of issues and attempts at
implementation already, which all were decently upvoted. I myself could
have used it once or twice 😄

Props to @jj811208 for his implementation in #4835 - I wanted to make
the configuration a little more generic (allowing to have multiple
environment files, and to possibly disable it altogether), but it was a
appreciated start.

Fixes #4718 
Closes #4835 (Supercedes it)

**How did you fix it?**

A new setting, `injectEnvironmentFiles`, lets you define files that Yarn
will load and inject into all scripts. It only affects subprocesses -
Yarn itself still uses `process.env` for its checks, so you can't for
example set `YARN_*` values and expect them to be applied to the current
process (use the yarnrc file for that instead).

The `injectEnvironmentFiles` setting has a few properties:

- It defaults to `.env`
- Nothing will be injected if it's set to an empty array or null
- The paths inside may be suffixed by `?` - in that case, Yarn won't
throw if the file doesn't exist

The idea with this last property is to allow for simple user
configuration (imagine, with the example below, that the project also
has a gitignore with `.env.*`):

```
injectEnvironmentFiles:
  - .env
  - .env.${USER}?
```

**Checklist**
<!--- Don't worry if you miss something, chores are automatically
tested. -->
<!--- This checklist exists to help you remember doing the chores when
you submit a PR. -->
<!--- Put an `x` in all the boxes that apply. -->
- [x] I have read the [Contributing
Guide](https://yarnpkg.com/advanced/contributing).

<!-- See
https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released
for more details. -->
<!-- Check with `yarn version check` and fix with `yarn version check
-i` -->
- [x] I have set the packages that need to be released for my changes to
be effective.

<!-- The "Testing chores" workflow validates that your PR follows our
guidelines. -->
<!-- If it doesn't pass, click on it to see details as to what your PR
might be missing. -->
- [x] I will check that all automated PR checks pass before the PR gets
reviewed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for .env file
3 participants