atticoos / gulp-ng-config

:wrench: Create AngularJS constants from a JSON config file
MIT License
173 stars 35 forks source link

Nested configuration with both environmental and non environmental variables #59

Closed RepoCorp closed 7 years ago

RepoCorp commented 7 years ago

I am trying to implement a configuration basically just like the Nested configuration example, with some environmental variables and some general ones.

So, with this:

  {
    "version": "0.1.0",
    "env": {
      "local": {
        "EnvironmentConfig": {
          "api": "http://localhost/"
        }
      },
      "production": {
        "EnvironmentConfig": {
          "api": "https://api.production.com/"
        }
      }
    }
  }

  gulpNgConfig('myApp.config', {
    environment: 'env.production'
  })

I was hoping I could get this:

angular.module('myApp.config', [])
.constant('EnvironmentConfig', {"api": "https://api.production.com/"})
.constant("version", "v0.1.0");

But I only get the environmental variable. So what is the point of having the nested environment or how can I get the non environmental variables to be added too?

atticoos commented 7 years ago

Great question @RepoCorp, when you specify a value for environment, you're specifying where to find the data needed to generate the Angular constants.

In your example the environment data lives in env.production, so the plugin will read env.production and use that to generate the data - but nothing more.

It sounds like you're suggesting that the plugin should "read everything inside env.production, along with any data in the JSON file that is outside env.production". Unfortunately that is an assumption the plugin would have to make that could differ from project to project.

For example, what if you didn't have a deeply nested configuration and it looked like:

{
  "production": {
    "foo": "bar",
  },
  "local": {
    "foo": "baz"
  }
}

With the current proposal we would end up with both configuration blocks being loaded when specifying environment: 'production'. In your example env.environmentA vs env.environmentB both share a common ancestor; that might not always be the case. Even if it did, how do we know how far up the tree to traverse? What if it was a.b.environmentA - do we look for other nodes in b, do we look in a, or do we continue to traverse upwards and outwards?

These are all assumptions the plugin would have to make -- often more risky than it might seem.


However I would like to support a scenario where there's environment specific data, and global data: What if you could specify multiple selection paths?

Imagine:

{
  "env": {
    "production": {...},
    "local": {...}
  },
  "global": {
    "version": "1.x.x",
    "something": "else"
  }
}
gulpNgConfig('myApp.config', {
  environment: ['env.production', 'global']
})

And that would:

☝️ I think this could be a solution that would make both of us happy. What do you think?

RepoCorp commented 7 years ago

That sounds perfect. Gives a lot of flexibility and control. 👍

atticoos commented 7 years ago

I like this idea. Let's do it.

Do you have interest in adding this new functionality? Could be fun

RepoCorp commented 7 years ago

I'll try 😄 Thank you!!

atticoos commented 7 years ago

Great. Support for "nested environments" was added in #41. There may be some helpful code diffs in there that can point you in the right direction. It's here that we look at the environment setting to load the subset of JSON data: https://github.com/ajwhite/gulp-ng-config/blob/5a0d878741326eb8dadc2ddf798921499107f251/gulp-ng-config.js#L74-L76

RepoCorp commented 7 years ago

Ok. I opened PR #61 with the changes. I haven't been working with Javascript for long and you have the honor 😉 to review my first contribution to an open source project ever! So feel free to change or discard anything, I will appreciate all the feedback.

Andrew-Max commented 7 years ago

:+1: on this pr! I would like to use this

atticoos commented 7 years ago

Support added in #61