Load environment file in development #695

ayrton · 2016-09-21T20:24:36Z

Adds support to load environment variables via a .env file for development using the very stable and battle tested ⚔ npm package dotenv. For more context see #687.

I've tested this out by doing the following:

npm run create-react-app my-app
cd my-app
echo "NODE_PATH=." >> .env
npm start

In my-app/src/index.js I changed import App from './App'; to import App from 'src/App'; and verified all still works (including tests)

ghost · 2016-09-21T20:24:44Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

ghost · 2016-09-21T20:49:31Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

gaearon · 2016-09-22T12:54:47Z

packages/react-scripts/template/README.md

@@ -434,7 +434,7 @@ REACT_APP_SECRET_CODE=abcdef npm start
 ```

 > Note: Defining environment variables in this manner is temporary for the life of the shell session. Setting
-permanent environment variables is outside the scope of these docs.
+permanent environment variables in development can be done in a .env file in the root of your project.


This should link to dotenv documentation.
Also please use backticks for .env so it’s highlighted.

gaearon · 2016-09-22T12:55:12Z

packages/react-scripts/scripts/start.js

+// Load environment variables from .env file. Surpress warnings using silent
+// if this file is missing. dotenv will never modify any environment variables
+// that have already been set.
+require('dotenv').config({silent: true})


Minor nit: let’s put a semicolon here (in all files).

gaearon · 2016-09-22T12:55:38Z

packages/react-scripts/package.json

@@ -40,6 +40,7 @@
    "cross-spawn": "4.0.0",
    "css-loader": "0.24.0",
    "detect-port": "1.0.0",
+    "dotenv": "^2.0.0",


Please keep the version pinned (2.0.0). We’re a bit more on the conservative side about updates.

gaearon · 2016-09-22T12:56:31Z

Overall this looks good to me and I’m OK with merging this after nits are addressed.
Thanks!

ayrton · 2016-09-22T13:14:21Z

@gaearon awesome. I've rebased and addressed your feedback

jhorneman · 2016-09-22T13:34:41Z

(As mentioned on Twitter, writing it here at @ayrton's request.)

Heroku uses a local .env file for its toolchain, to allow local testing before pushing to their servers. See https://devcenter.heroku.com/articles/heroku-local#copy-heroku-config-vars-to-your-local-env-file. Would this use of .env clash with that? They theoretically have the same purpose: in practice perhaps they represent different use cases.

(I can't comment or defend this beyond this link, because I don't use JavaScript with Heroku. I'm just bringing it up in case people didn't know Heroku's toolchain did this.)

Update: I just realized that if I were to use React in the JavaScript part of my mainly-Python app, and I were use to create-react-app, I'd be upset if:

my JS code were to behave oddly due to the .env file I set up so I can locally test my Python app on Heroku. (I'm assuming here create-react-app would never write to the .env file: that'd be catastrophic since it's not under revision control.)
I couldn't have create-react-app AND local Heroku because I'd need two different .env files.

ayrton · 2016-09-22T13:45:51Z

@jhorneman good question, this use case would go as follows:

npm run build
cd build/
heroku config:get CONFIG-VAR-NAME -s  >> .env
foreman start

The copied environment variables from heroku will live inside the build/ file (not inside the root of the project). The environment variables defined in the root .env are the ones used to build the actual project. So I don't believe it's an issue.

gaearon · 2016-09-22T13:48:39Z

I would like to get feedback from @mars before we merge this.

jhorneman · 2016-09-22T13:50:49Z

@ayrton: Thanks for replying. I honestly can't say if my case is an edge case, so I apologize if I sound pedantic, but I have an existing Python-and-some-JS Heroku app with a .env in the root of the project, which is for the Python part. The solution you propose wouldn't work for me (without rearranging the entire project), but perhaps I could (or should?) run create-react-app in a subfolder?

I guess if there are no catastrophic consequences from people doing this "wrong", an FAQ entry would suffice.

ayrton · 2016-09-22T13:52:42Z

@gaearon I think that's a good idea, I've been using CRA together with the create-react-app-buildpack to deploy on heroku. What's imo key to dotenv is the following:

We will never modify any environment variables that have already been set. In particular, if there is a variable in your .env file which collides with one that already exists in your environment, then that variable will be skipped. This behavior allows you to override all .env configurations with a machine-specific environment, although it is not recommended.

Meaning if somehow the .env was to land on the server/CI to create a production build that it will never collide with previously set environment variables.

gaearon · 2016-09-22T13:57:13Z

Can you write a more comprehensive Usage Guide entry that explains env variables more holistically? Like, what you’d normally do in development, and what you’d normally do in production.

gaearon · 2016-09-22T13:57:50Z

packages/react-scripts/scripts/test.js

@@ -9,6 +9,11 @@

 process.env.NODE_ENV = 'test';

+// Load environment variables from .env file. Surpress warnings using silent
+// if this file is missing. dotenv will never modify any environment variables
+// that have already been set.


It’s a good idea to link to dotenv docs from this comment.

ayrton · 2016-09-22T14:05:27Z

I'll try to write some more complete documentation tonight

mars · 2016-09-22T14:47:59Z

As a Heroku dev who's synced up local runtime with deployments many times, this dotenv feature is implemented perfectly ✅

That said, please allow me to call attention to an existing issue with CRA env vars that impact how the app works on Heroku: mars/create-react-app-buildpack#7

TL;DR these vars are compiled into the bundle, which means the build is not necessarily stateless. This can result in multi-stage deployments (pipelines) having stale values for env vars.

Please be careful/aware of this snag and voice you opinions/concerns in that Github issue.

fson · 2016-09-22T15:14:42Z

packages/react-scripts/scripts/start.js

+// Load environment variables from .env file. Surpress warnings using silent
+// if this file is missing. dotenv will never modify any environment variables
+// that have already been set.
+require('dotenv').config({silent: true});


Can this be moved to packages/react-scripts/config/env.js? That's the only place where these environment variables would be used anyway and then we don't need this in each script separately?

I liked this idea, but I can't get it to work (using NODE_PATH). I think somehow that config/env.js is not loaded or not loaded early enough:

diff --git a/packages/react-scripts/config/env.js b/packages/react-scripts/config/env.js index 66acf11..08faa82 100644 --- a/packages/react-scripts/config/env.js +++ b/packages/react-scripts/config/env.js @@ -14,2 +14,8 @@ +// Load environment variables from .env file. Surpress warnings using silent +// if this file is missing. dotenv will never modify any environment variables +// that have already been set. +// https://github.com/motdotla/dotenv +require('dotenv').config({silent: true}); + var REACT_APP = /^REACT_APP_/i; diff --git a/packages/react-scripts/config/webpack.config.dev.js b/packages/react-scripts/config/webpack.config.dev.js index 8d1de1c..5596de6 100644 --- a/packages/react-scripts/config/webpack.config.dev.js +++ b/packages/react-scripts/config/webpack.config.dev.js @@ -11,2 +11,3 @@ +var env = require('./env'); var path = require('path'); @@ -18,3 +19,2 @@ var WatchMissingNodeModulesPlugin = require('../scripts/utils/WatchMissingNodeMo var paths = require('./paths'); -var env = require('./env'); diff --git a/packages/react-scripts/config/webpack.config.prod.js b/packages/react-scripts/config/webpack.config.prod.js index 5790e8e..5c793c8 100644 --- a/packages/react-scripts/config/webpack.config.prod.js +++ b/packages/react-scripts/config/webpack.config.prod.js @@ -11,2 +11,3 @@ +var env = require('./env'); var path = require('path'); @@ -18,3 +19,2 @@ var url = require('url'); var paths = require('./paths'); -var env = require('./env'); diff --git a/packages/react-scripts/scripts/build.js b/packages/react-scripts/scripts/build.js index 7d87acc..71dcc79 100644 --- a/packages/react-scripts/scripts/build.js +++ b/packages/react-scripts/scripts/build.js @@ -14,8 +14,2 @@ process.env.NODE_ENV = 'production'; -// Load environment variables from .env file. Surpress warnings using silent -// if this file is missing. dotenv will never modify any environment variables -// that have already been set. -// https://github.com/motdotla/dotenv -require('dotenv').config({silent: true}); - var chalk = require('chalk'); diff --git a/packages/react-scripts/config/webpack.config.prod.js b/packages/react-scripts/config/webpack.config.prod.js index 5790e8e..5c793c8 100644 --- a/packages/react-scripts/config/webpack.config.prod.js +++ b/packages/react-scripts/config/webpack.config.prod.js @@ -11,2 +11,3 @@ +var env = require('./env'); var path = require('path'); @@ -18,3 +19,2 @@ var url = require('url'); var paths = require('./paths'); -var env = require('./env'); diff --git a/packages/react-scripts/scripts/build.js b/packages/react-scripts/scripts/build.js index 7d87acc..71dcc79 100644 --- a/packages/react-scripts/scripts/build.js +++ b/packages/react-scripts/scripts/build.js @@ -14,8 +14,2 @@ process.env.NODE_ENV = 'production'; -// Load environment variables from .env file. Surpress warnings using silent -// if this file is missing. dotenv will never modify any environment variables -// that have already been set. -// https://github.com/motdotla/dotenv -require('dotenv').config({silent: true}); - var chalk = require('chalk'); diff --git a/packages/react-scripts/scripts/start.js b/packages/react-scripts/scripts/start.js index a82e68d..b0290bf 100644 --- a/packages/react-scripts/scripts/start.js +++ b/packages/react-scripts/scripts/start.js @@ -13,8 +13,2 @@ process.env.NODE_ENV = 'development'; -// Load environment variables from .env file. Surpress warnings using silent -// if this file is missing. dotenv will never modify any environment variables -// that have already been set. -// https://github.com/motdotla/dotenv -require('dotenv').config({silent: true}); - var path = require('path'); diff --git a/packages/react-scripts/scripts/test.js b/packages/react-scripts/scripts/test.js index 0c123ec..d13aff3 100644 --- a/packages/react-scripts/scripts/test.js +++ b/packages/react-scripts/scripts/test.js @@ -11,8 +11,2 @@ process.env.NODE_ENV = 'test'; -// Load environment variables from .env file. Surpress warnings using silent -// if this file is missing. dotenv will never modify any environment variables -// that have already been set. -// https://github.com/motdotla/dotenv -require('dotenv').config({silent: true}); - const createJestConfig = require('./utils/createJestConfig');

but I can't get it to work (using NODE_PATH)

I thought this would only be used to specify the REACT_APP_* environment variables that we officially support? But there's some other use case for this that I'm missing? Can you clarify what you mean by "using NODE_PATH"?

I think there are use cases for this PR outside the REACT_APP_* usecases eg. absolute imports.

By setting NODE_PATH=. I can start using absolute imports in my codebase:

Relative:

import Button from '../../shared/button';

vs.

import Button from 'src/shared/button';

Note that absolute imports aren't officially supported in create react app as of now, so even if this environment variable might work, there is no guarantees of it continuing to be supported in the future. Therefore I wouldn't encourage using it until we add official support for absolute imports.

With this in mind, I think it's fine if the environment only works for REACT_APP_* variables – nothing else is officially supported right now.

Actually NODE_PATH is already supported since 0.4.0 (#476), sorry I was wrong about this 😊

juanpaco · 2016-09-22T16:31:12Z

I ❤️ dotenv. I was on a CRA project that recently ejected so that we could use it, so this is most welcome news!

This might be less of a concern for a UI that is completely separated from its server side the way a CRA app is, but on a development machine, I generally have to have a .env.development and a .env.test. If it's all in one file, then running tests with, say, a different API endpoint could be tricky.

I had a discussion with the dotenv folks about this here: motdotla/dotenv#150

so having a separate test and development file is legitimate in their eyes. How to make that easy for CRA users, I don't know right now.

I think that having the .env at all is a step up from not having it, but I would further ❤️ being able to have different values for both test and development.

gaearon · 2016-09-22T20:58:55Z

I just merged a PR that rename env.js and moves it to scripts/utils/getClientEnvironment so this would need to be rebased. Thanks!

benpickles · 2016-09-22T21:13:32Z

I use https://github.com/direnv/direnv which keeps environment variables in the environment and removes the need for an app dependency (in any language).

ayrton · 2016-09-22T21:20:38Z

@gaearon added more documentation + rebased

gaearon · 2016-09-22T21:29:39Z

@ayrton Can you please address the previous comment and explain the choice of dotenv over direnv?

ayrton · 2016-09-22T21:42:07Z

@benpickles I like the idea direnv but I don't think it's a perfect fit for CRA. You can't expect every user that wants to leverage environment variables to install an external dependency.

Afaik the goal of CRA is to provide an easy-to-use experience to get started with React with sensible defaults. Accepting this PR's approach means there is zero added friction for the end user to start using environment variables (if they wish) in their application (and following best practices as specified in the Twelve-Factor App methodology.

ayrton · 2016-09-22T21:51:17Z

@juanpaco the docs in dotenv do not recommend this:

We strongly recommend against having a "main" .env file and an "environment" .env file like .env.test. Your config should vary between deploys, and you should not be sharing values between environments.

juanpaco · 2016-09-22T23:27:35Z

@ayrton If you check out the thread I linked to, I asked them about it. Their concern was about having a "main" .env file that then gets a few things overridden with with env-specific values. Having complete sets is okay though. I found that the way the FAQ was written didn't really convey that point, so I asked them for clarification in that issue I linked to.

mars · 2016-09-23T02:39:17Z

@ayrton CRA's use of env vars does not honor 12-factor, because the vars are compiled into the bundle. To honor 12-f, the env vars must be read at runtime. See: mars/create-react-app-buildpack#7

ayrton · 2016-09-23T07:07:51Z

@mars I never experienced it any other way where environment variables are not compiled into the bundle for JS apps. I totally applaud the effort for experimentation in mars/create-react-app-buildpack#7 but I don't see how this specific PR could address this right now

ayrton · 2016-09-23T07:17:20Z

@gaearon what do we need to do to proceed here or did we hit a roadblock? I can see there are still open questions (for edge cases) which this PR doesn't address, but this was honestly never intended to fix those yet. I think this PR gets 99% of the people up and running to use environment variables on development (while not being an annoyance to the people who don't use them).

If this is going nowhere I'm happy to close for now.

gaearon · 2016-09-23T08:12:09Z

No roadblocks, this looks good to me. Can you please remove unrelated spacing changes in PR? Space is significant in Markdown and is used to break likes in those places.

ayrton · 2016-09-23T08:23:54Z

Wasn't aware of that 😄 (TIL) - Done!

gaearon · 2016-09-23T10:59:40Z

Thanks!

gaearon · 2016-09-23T21:50:40Z

0.5.0 has been released with this feature.

* Load environment file via dotenv if .env file is present * Document loading environment variables in .env file * Minor doc tweaks

ghost added the CLA Signed label Sep 21, 2016

ayrton mentioned this pull request Sep 22, 2016

Loading environment variables #687

Closed

gaearon suggested changes Sep 22, 2016

View reviewed changes

gaearon added this to the 0.5.0 milestone Sep 22, 2016