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: Load .env.*.local envs #55

Merged
merged 2 commits into from Jul 24, 2020

Conversation

danilofuchs
Copy link
Contributor

@danilofuchs danilofuchs commented Apr 19, 2020

Closes #54 (allows local env files)
Addresses #29 (allows config.path to be an array)

My approach automatically resolves env files in the following order (forked from create-react-app):

  • .env: Default.
  • .env.local: Local overrides. This file is loaded for all environments except test.
  • .env.development, .env.test, .env.production: Environment-specific settings.
  • .env.development.local, .env.test.local, .env.production.local: Local overrides of environment-specific settings.

Users are advised to commit .env* to their repos, except for .env.*.local.

This, however, introduces a breaking change, since .env file will always be loaded in any environment.

I can enable this feature behind a flag, if you think it's a safer option.

@kbsanders
Copy link

This would be super useful to have for local development with serverless-offline as that plugin no longer automatically sets an IS_OFFLINE=true env variable (That feature was removed, for whatever reason).

dherault/serverless-offline#959
dherault/serverless-offline#768

So being able to set it manually in a .env.local file that doesn't get committed to the repo would be perfect.

@colynb
Copy link
Collaborator

colynb commented Apr 27, 2020

hi @danilofuchs. I’m not that familiar with the CRA recommendations and it seems counter intuitive to me though I looked into it. I’m not wrapping my head around the benefits but it seems most of the purported benefits have to do with client side app bundling. For serverless apps I’m not seeing where there would be similar benefits but rather see that as highly insecure. I apologize for not understanding but can you clear this up more for me? Thanks.

@danilofuchs
Copy link
Contributor Author

User Story

As a developer working on a project with my team of 5 developers, I share common development envs, such as endpoint URLs, non-sensitive configuration data for a third party integration and default feature toggles in an .env.development file, which is automatically loaded on my dev environment. I want to be able to override this values to test code behaviour locally, such as using a different port on my dev server or enabling a feature toggle without risking commiting these specific configs and changing the behaviour for all my peers.


My own use case

I need to set some shared (commited) env vars in a .env or .env.development file, such as endpoint URLs, IS_OFFLINE, TZ, etc.

In this file, I do not commit any credentials. However, when working locally some things must be overwritten. For instance, I need to change the port of a local server endpoint as it conflicts with other projects only I use on my machine.

I store all production credentials on SSM. But, they are not available on a local environment. It is possible to default the credentials to a env value, but it is unwise to commit these credentials to .env.development, as every one would have access to it on the repo.

The current alternative is to not commit and .env file at all, but then I loose the ability to share common configurations which everyone will need.


it seems most of the purported benefits have to do with client side app bundling

This approach was actually first proposed and addopted in Rails, a monolithic server-side framework.

but rather see that as highly insecure

My use case states that credentials are not commited to the repo. This is exactly why .local files are interesting, as they allow users to commit .env.* files without fear, as all credentials are solely stored on their .env.*.local files


I agree this may not be the usual way of handling envs, and as such I think it is fair to put this feature behind a flag or enable it via the paths parameter. We can discuss how to better conciliate this behaviour.

I hope I made myself clear!

@colynb
Copy link
Collaborator

colynb commented Apr 27, 2020

I think I'm starting to get it knowing that I often struggle with the same thing. Project variables are split between secrets and general configuration, especially for local development. The standard convention has been to put both inside a single env file that does not get committed. There needs to be a way to commit env files for config/non-secret variables and exclude env files for secrets only. Basically, you need to parse two env files instead of one. The dotenv library does not really have a way to parse multiple env files but your code change maps over the available files then use dotenv to parse each of them one at a time. Would you mind setting up an example app in the PR under the examples folder that demonstrates what files will get committed and which ones will not?

@colynb
Copy link
Collaborator

colynb commented Apr 27, 2020

I think the thing that is tripping me up is the naming convention of the files. The .local is confusing to me. I always think of "local" as in local development server or localhost. At any rate, I'm thinking there is a way to not need a feature flag. It seems like the "resolveEnvFileNames" handles either a single env file or multiple files. I'm not sure how this could be a breaking change.

@danilofuchs
Copy link
Contributor Author

I believe this could be a breaking change since it always loads the .env file, regardless of stage. But this may be not of huge concern, since as it currently only loads the .env.STAGE file, users probably do not have a .env file anyway

@danilofuchs
Copy link
Contributor Author

I think the thing that is tripping me up is the naming convention of the files. The .local is confusing to me. I always think of "local" as in local development server or localhost.

I totally get what you mean.

I think that, as this is the standard approach on other widely used libs, I would rather keep it consistent.

What do you propose as an alternative?

@danilofuchs
Copy link
Contributor Author

Apparently, Symfony and Vue CLI also adopt the same approach

https://symfony.com/doc/current/configuration/dot-env-changes.html

https://cli.vuejs.org/guide/mode-and-env.html

@colynb
Copy link
Collaborator

colynb commented Apr 28, 2020

What do you propose as an alternative?

I wasn't suggesting an alternative. I definitely prefer to stick to community conventions.

@dekked
Copy link

dekked commented May 13, 2020

It looks like next.js has recently adopted the same convention, too.

@danilofuchs
Copy link
Contributor Author

@colynb can we move along with this PR? You mentioned I could create an example app, but honestly I don't know the best way to show the user which files must be commited if I need to commit the example to this repo.

I could write a Readme inside the example folder and commit the .local files with comments stating these files should be in gitignore...

@danilofuchs
Copy link
Contributor Author

Note: This will conflict with the changes in #48

@colynb
Copy link
Collaborator

colynb commented May 19, 2020

@colynb can we move along with this PR? You mentioned I could create an example app, but honestly I don't know the best way to show the user which files must be commited if I need to commit the example to this repo.

I could write a Readme inside the example folder and commit the .local files with comments stating these files should be in gitignore...

That's ok. I can take care of this release but probably not until this weekend. I do think it's an important release so know that I will not drop it. Also, I think it is more important than the changes in #48 which I will do after this one. Thanks for your patience.

@flybayer
Copy link

Ah yes, this will be super helpful!

@randytarampi
Copy link

Respectfully bumping this again

I'm going live with something next week and would love to get off @danilofuchs's branch and back onto the mainline before that happens – is there anything the community can help with? 🙏

@colynb
Copy link
Collaborator

colynb commented Jul 24, 2020

@danilofuchs would you have some time today to pair program with me to get this released? I could really use a 2nd pair of eyes.

https://calendly.com/infrontlabs/openhours

@danilofuchs
Copy link
Contributor Author

@danilofuchs would you have some time today to pair program with me to get this released? I could really use a 2nd pair of eyes.

Sure thing!

@colynb colynb merged commit b376518 into neverendingqs:master Jul 24, 2020
@colynb
Copy link
Collaborator

colynb commented Jul 24, 2020

@danilofuchs THANK YOU for your help, I'm really glad we did the call!

@danilofuchs danilofuchs deleted the danilofuchs/local-envs branch July 24, 2020 23:48
@danilofuchs
Copy link
Contributor Author

@danilofuchs THANK YOU for your help, I'm really glad we did the call!

Really happy to be able to contribute! Keep up with the great work, the open source community grows a lot when people take their time to make projects go forward. 🚀

@colynb
Copy link
Collaborator

colynb commented Jul 25, 2020 via email

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.

Support for local env values or multiple paths
6 participants