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

NODE_ENV and stage collides to each other #150

Open
rostislav-simonik opened this issue Feb 28, 2021 · 3 comments
Open

NODE_ENV and stage collides to each other #150

rostislav-simonik opened this issue Feb 28, 2021 · 3 comments

Comments

@rostislav-simonik
Copy link

rostislav-simonik commented Feb 28, 2021

Is your feature request related to a problem? Please describe.
NODE_ENV and stage are two different things,

NODE_ENV usually controls let's say for the sake of simplicity just "optimizations"

  • NODE_ENV=development -> very verbose, non optimised
  • NODE_ENV=production -> silent, optimised

stage represents where the application should be deployed, it can specify different database, file storage etc.

  • stage=development - for development purposes
  • stage=qa - for acceptance testing
  • stage=staging - preproduction testing
  • stage=production - production environment

so you can have mix like

  • stage=development, NODE_ENV=development
  • stage=qa, NODE_ENV=production
  • stage=staging, NODE_ENV=production
  • stage=production, NODE_ENV=production

because of getEnvironment() function, it's impossible to differentiate by stage if NODE_ENV has been specified as well. and it must be specified due to other toolings that required that env variable.

  getEnvironment(options) {
    return (
      process.env.NODE_ENV || options.env || options.stage || 'development'
    );
  }

Describe the solution you'd like
Not sure what would be the best, but I'd suggest completely remove NODE_ENV from that setup, but because we don't want to break existing usage, I'd introduce some option

custom:
  dotenv:
    # default: false
    ignoreNodeEnv: true

and to adjust getEnvironment() like this

  getEnvironment(options) {
    return (
       !this.config.ignoreNodeEnv && process.env.NODE_ENV || options.env || options.stage || 'development'
    );
  }

Describe alternatives you've considered
Writing own parser donEnvParser

@neverendingqs
Copy link
Owner

I'm not convinced either way around the name of the environment variable NODE_ENV. I can see how this decision was made, as NODE_ENV is a de facto way of describing the environment the code is going to run in. I was unable to get more context in #24 or e63d52d when this was introduced.

I would argue however that command line arguments should probably override environment variables, as command line arguments are explicit while environment variables are implicit. If process.env.NODE_ENV was second-last instead of first in getEnvironment(), would that work? If so, I would like do this as part of v4 (https://github.com/neverendingqs/serverless-dotenv-plugin/projects/1), especially since v4 changes can be pulled in right away in v3 with the v4BreakingChanges option.

As a workaround, you should be able to set NODE_ENV inside your dotenv file. That way, you can either use command line arguments or set NODE_ENV to resolve which dotenv files to use, and then this plugin will set NODE_ENV for your Lambda functions.

I am interested in your use case as well, specifically why NODE_ENV is being set outside of dotenv files.

@neverendingqs neverendingqs added feature awaiting response Contributors are awaiting a response. bug labels Feb 28, 2021
@rostislav-simonik
Copy link
Author

rostislav-simonik commented Mar 1, 2021

@neverendingqs yes, if we increase precedence of command-line arguments over NODE_ENV in that case it's fine.

specifying NODE_ENV within .env files is problematic because if you are using such other tools as generate_artefacts, etc. not familiar with .env files then it must be declared outside

scripts: {
   "start":  "yarn start:development",
    "start:development": "export NODE_ENV=development DEPLOYMENT=development; yarn _start",
    "_start": "generate_artifacts && sls offline --stage $DEPLOYMENT",
}

I'm using github branch dependency so no need for immediate merge, but if it would be somehow introduced in 4.0, then it would be great.

@github-actions github-actions bot removed the awaiting response Contributors are awaiting a response. label Mar 1, 2021
@danilofuchs
Copy link
Contributor

I think we could add a new option stage and let users choose between ${env:NODE_ENV} or ${self:provider.stage}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants