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

config/dotenv.js does not respect environment assumed from ember deploy prod #46

Open
lolmaus opened this issue Dec 24, 2017 · 5 comments

Comments

@lolmaus
Copy link

lolmaus commented Dec 24, 2017

Hi!

This is a follow-up to #30.

I use multiple dotenv files -- one per deploy target. My config/dotenv.js has logic to select a specific dotenv file provided in a DOTENV_FILE env var (see #25). If DOTENV_FILE is empty, then it picks a default one depending on environment.

The problem is that config/dotenv.js runs before the ember-cli-deploy pipeline. It selects the development dotenv file, then the ember deploy prod command makes ember-cli-deploy use the production environment. The build becomes faulty.

I have to run DOTENV_FILE=prod ember deploy prod every time, which is annoyingly redundant.

I ended up using this workaround:

function deployEnv () {
  if (
    process.argv[2] === 'deploy'
    && (process.argv[3] === 'prod' || process.argv[3] === 'production')
  ) {
    return 'production'
  }
}

let environment =
  process.env.EMBER_ENV
  || deployEnv()
  || 'development'

Maybe ember-cli-dotenv can do a similar thing to assume the environment. Maybe Ember CLI even offers a more robust way to access the command line params.

@SergeAstapov
Copy link
Collaborator

@lolmaus unfortunately with Ember CLI >= 2.16 we can not rely on environment detection made by Ember CLI: we have to initialize dotenv configs at init stage of addon in order to make environment variables available at config stage.
included hook is to late for that as discussed in #30.

I'm personally in favor of using DOTENV_FILE env var and make it supported by addon itself instead of hardcoding ember-cli-deploy commands.
DOTENV_FILE=.env.prod ember deploy prod seems to be completely fine approach as it should be setup on CI. In case of smaller projects without CI - it can be scripted, e.g. into package.json like npm run deploy-prod

@fivetanley @jasonmit thoughts?

@lolmaus
Copy link
Author

lolmaus commented Dec 27, 2017

What I want is:

  1. Though I strongly believe in decoupling build environment from deploy target, I want each environment to be associated with a specific deploy target by default, unless overridden by hand.
  2. I want to have one source of truth for selecting environment.

I don't want to end up doing

DOTENV_FILE=production EMBER_ENV=production ember deploy production

and then check for process.env.DEPLOY_TARGET (in ember-cli-build.js, config/environment.js or index.js of in-repo addons), as ember-cli-deploy recommends.

There is clearly a need for ember-cli-dotenv, ember-cli-deploy and ember-cli teams to sync up.

@lolmaus
Copy link
Author

lolmaus commented Dec 27, 2017

instead of hardcoding ember-cli-deploy commands

Well, we're already doing this:

    // Is it "ember test" or "ember t" command without explicit env specified?
    if (!env && (process.argv.indexOf('test') > -1 || process.argv.indexOf('t') > -1)) {
      env = 'test'
    }

https://github.com/fivetanley/ember-cli-dotenv/blob/2.0.0/index.js#L58-L61

@jasonmit
Copy link
Collaborator

jasonmit commented Dec 29, 2017

I want to have one source of truth for selecting environment. I don't want to end up doing DOTENV_FILE=production EMBER_ENV=production ember deploy production

_resolveEnvironment is needed because of an upstream bug where the build target is unknown at the time we need it. We parse all the internal commands if EMBER_ENV is unset. Which is why I'm adamant in not adding custom arg parsing logic that would couple us to ember deploy's deploy target arg. At the end of the day, _resolveEnvironment should only ever return what would expect from the EmberApp instances env fn and the goal is to remove it entirely when the upstream bug is resolved.

ec-dotenv's default config file lookup strategy is centered around the EmberEnv when looking for the builds associated dotenv file. For most, that's sufficient. For you, and maybe others, you need the ability to override - which is possible as you already know.

Short or writing our own plugin architecture (total overkill), you can write an ec-deploy plugin that sets process.env.DOTENV_FILE. As long as that happens before our init hook, you would achieve what it is you're after.

I'm personally in favor of using DOTENV_FILE env var and make it supported by addon itself instead of hardcoding ember-cli-deploy commands.

👍 @SergeAstapov

@jeremyhaile
Copy link

I just spent a lot of time trying to understand why env was development while I was running ember deploy production. The default behavior is quite unintuitive, especially given that I imagine a primary use of this gem is for production secrets that should be available during a deploy.

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

No branches or pull requests

4 participants