berry icon indicating copy to clipboard operation
berry copied to clipboard

feat(core): add `.env` support to `Configuration`

Open jj811208 opened this issue 2 years ago β€’ 1 comments

What's the problem this PR addresses?

Closes https://github.com/yarnpkg/berry/issues/4718 related issue: https://github.com/yarnpkg/berry/issues/2724 related PR: https://github.com/yarnpkg/berry/pull/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 [email protected] package 5ed943492
  2. new Configuration private method - findPackageEnv 630a7c9a0
  3. let the parseXxx family of functions accept env parameters 1f961ba31
  4. yarnrc.yml load the .env file 1374abb
  5. expose environment information in the Configuration ec7ed66dc
  6. new test about the feature d5870bbf2 d9f7ca993

Checklist

  • [x] I have set the packages that need to be released for my changes to be effective.
  • [x] I will check that all automated PR checks pass before the PR gets reviewed.

jj811208 avatar Sep 08 '22 14:09 jj811208

Hi Everyone,

I get an error in the windows integration test.

thrown: "Exceeded timeout of 45000 ms for a test.

I tried to force-push to trigger the ci again (Sorry. I don't have direct retry access.), then I get a similar error in a different stage.

https://github.com/yarnpkg/berry/runs/8252033060 https://github.com/yarnpkg/berry/runs/8251390598

I checked the Github Action log for this repo and found that this seems to be an existing error (happened by chance?), guess it's because our windows runner performance not enough.

Before asking for a review, should I repeat the trigger for ci until it passes?

jj811208 avatar Sep 08 '22 16:09 jj811208

I think this will conflict with https://github.com/yarnpkg/berry/pull/4912, so I'll change its state back to draft first~

after https://github.com/yarnpkg/berry/pull/4912 is merged, I will come back to resolve the conflict and reopen πŸ™‚.

jj811208 avatar Oct 10 '22 12:10 jj811208

https://github.com/yarnpkg/berry/pull/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 avatar Oct 14 '22 12:10 jj811208