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

Support variable expansion in the middle of a string. #97

Closed
wants to merge 1 commit into from

Conversation

blazzy
Copy link

@blazzy blazzy commented Sep 30, 2015

Allow basic variable expansion in the middle of a string.

An example use case:

$DB_USER=my_user
$DB_HOST=localhost
$DB_NAME=my_db

$DATABASE_URL="${DB_USER}@${DB_HOST}/${DB_NAME}"

Allow basic variable expansion in the middle of a string.

Here is an example use case:

```
$DB_USER=my_user
$DB_HOST=localhost
$DB_NAME=my_db

$DATABASE_URL="${DB_USER}@${DB_HOST}/${DB_NAME}"
```
@blazzy
Copy link
Author

blazzy commented Sep 30, 2015

Eh. I found #42 right after submitting this :-/. I guess this is a no go here.

@blazzy
Copy link
Author

blazzy commented Sep 30, 2015

For what it's worth, I think this implementation is much simpler to follow. It doesn't bother trying to tackle recursive variables, and it takes over handling existing expansion feature.

@maxbeatty
Copy link
Contributor

@blazzy totally open to a solution that would account for the randomness of API keys and the like that could include patterns such as ${*}. I've opened #98 to make this more explicit and up front

@blazzy
Copy link
Author

blazzy commented Oct 1, 2015

@maxbeatty This PR accounts for it by not expanding variables within single quotes matching the behavior of most shells. $ can also be escaped.

I guess that should be made explicit in the README. And this change should come with a major version bump.

+      it('does not expands inline variables in single quoted strings', function (done) {
+        parsed.SINGLE_QUOTED_INLINE_EXPAND.should.equal('inline${BASIC}ally')
+        done()
+      })

@maxbeatty
Copy link
Contributor

that's a good start but that would mean people need to opt out of variable expansion instead of opting in which is much more preferable because people can leave their files as-is until they're ready to make changes. they aren't blocked from upgrading versions. that's important to me.

@@ -17,3 +17,8 @@ EQUAL_SIGNS=equals==
RETAIN_INNER_QUOTES={"foo": "bar"}
RETAIN_INNER_QUOTES_AS_STRING='{"foo": "bar"}'
INCLUDE_SPACE=some spaced out string
BRACED_EXPAND=${BASIC}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't look to be used in the new tests

@maxbeatty
Copy link
Contributor

Please include documentation updates in this PR so it's super clear how to use and any pitfalls.

After looking at this a little closer, it looks like dotenv would support shell-styled variables ($BASIC) and ES6 template string syntax (${BASIC}) which brings me back to my original point of taking on too much responsibility by trying to do both Bash's job and JavaScript's job.

I'd also like to see more examples of why this is beneficial beyond string concatenation. The classic example feels like more ops work to me and would be better handled by simple application code. Every environment needs to define $DATABASE_URL in their .env instead of the app constructing it uniformly without effort.

$DATABASE_URL="${DB_USER}@${DB_HOST}/${DB_NAME}"
process.env.DATABASE_URL = process.env.DB_USER + '@' + process.env.DB_HOST + '/' + process.env.DB_NAME

@blazzy
Copy link
Author

blazzy commented Oct 20, 2015

In our case environment variables are not consistent in different environments. In production/hosted environments we get a full DATABASE_URL fed to our app. In dev we maintain each of the parts of the DATABASE_URL separately in order to support dev environment setup scripts that create the user with the appropriate password along with the database.

Sure we could still solve this at the app level by parsing the url in those dev setup scripts. At the time this patch felt like the cleaner approach.

I'm not sure where the line should be drawn in supporting shell features. I see that the ruby version of dotenv goes so far to support running shell commands. That sounds like a step too far.

But maybe what I really want is something that sources a shell file. Something like the module shell-source.

@maxbeatty
Copy link
Contributor

If I'm understanding your use case correctly, your development environment uses different environment variables than your production environment. You need variable expansion to make your development environment work like your production environment. If I have all of that straight, this isn't a compelling case. Your "dev environment setup scripts" should more closely mirror production or do this concatenation for you.

In fact after rereading Twelve-Factor again, I'm not sure any type of variable expansion should happen if we follow it strictly.

In a twelve-factor app, env vars are granular controls, each fully orthogonal to other env vars.

I'm backpedaling on being open to a solution to this problem given the lack of use cases and getting away from guiding principles 🚶

@JamesKyburz
Copy link

I have a use case where this functionality would be of use!!

If you use --link when running docker containers you get environment variables prefixed with the docker link name.

Being able to do

SERVICE_URL=http://linkname:${LINKNAME_PORT}

would be nice :)

Having to concatenate in javascript is less than ideal, because then you need to solutions a non docker one and a docker one.

@maxbeatty
Copy link
Contributor

@JamesKyburz can you expand on "you need to solutions a non docker one and a docker one"? I thought one of the main selling points of docker was to run your software the same in any environment. dotenv may not have a place in a docker setup and that's great because docker has made environment variables a first class citizen

@blazzy
Copy link
Author

blazzy commented Oct 29, 2015

There rarely is a 100% docker setup. His DB in dev might be a linked dockerized DB server. There he would need link specific environment variables. DBs in his deployed environments might be provided by a third party solution that gives him a full database URL through its API. It doesn't feel great introducing environment specific logic in the applications to handle both of these cases.

@JamesKyburz
Copy link

I think having the expansion functionality would be nice + it's using bash syntax which is nice!

Come to think of it my docker example wasn't the best

Basically I thought it would be nice to have

awesome_service=http://${awesome_host}:${awesome_port}

@nicekiwi
Copy link

nicekiwi commented Nov 7, 2015

this would be great for mongoDB

MONGOLAB_DATABASE=heroku_db
MONGOLAB_USER=username
MONGOLAB_PASSWORD=password
MONGOLAB_DOMAIN=abcd1234.mongolab.com
MONGOLAB_PORT=12345
MONGOLAB_URI=mongodb://${MONGOLAB_USER}:${MONGOLAB_PASSWORD}@${MONGOLAB_DOMAIN}:${MONGOLAB_PORT}/${MONGOLAB_DATABASE}

ah.. beyond concatenation.. :P well no.. I cant think of such a usecase beyond that. Just that it would be jolly useful.

@AndersDJohnson
Copy link

+1 for the feature

@maxbeatty
Copy link
Contributor

Sorry for the delay on this. We've now made it easier to work with variables after they are read from file. motdotla/dotenv-expand or a new project would be a good place to continue this discussion

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.

5 participants