search

kabisa / maji

Maji is a framework to build great hybrid mobile apps.
https://www.majimobile.com/
MIT License
18 stars 10 forks source link

Let maji build command read NODE_ENV from environment #188

Closed leonderijke closed 6 years ago

leonderijke commented 6 years ago

The bin/maji build command wasn't able to read NODE_ENV from the environment, only from the passed in options. Because the build command is called from the bin/maji run command, but without passing in options, we should read from the environment.

If you set the environment of the run command by using the options: bin/maji run --env test, it will still build production. If you set it via the NODE_ENV environment variable it will build the right environment: NODE_ENV=test bin/maji run.

Therefore, this is a partial solution to #187.

pascalw commented 6 years ago

Hey @leonderijke I was just looking into this and this looks good to me! :+1:

I was thinking this might have to do with legacy APP_ENV support, but I was mistaken since we in the end decided to remove APP_ENV completely in favor of the more common NODE_ENV.

However I'm a little confused by your last remark that this is a partial solution to #187. Partial in what way?

leonderijke commented 6 years ago

Parameters aren't passed through when maji build is called from maji run. This PR only changes that NODE_ENV is read by maji build.

So, these two commands should have the same output, but they don't:

  1. bin/maji run --env test: will build production
  2. NODE_ENV=test bin/maji run: will build test

A complete solution to #187 will have both commands produce the same output (test environment).

pascalw commented 6 years ago

@leonderijke I would like to merge this as is, even if it's not a complete solution. I wan't to get rid of all bash scripts soon anyway, so there's not much point in fixing this now.

leonderijke commented 6 years ago

No problem! :shipit: