atticoos / gulp-ng-config

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

New configuration properties #51

Closed snk7891 closed 3 years ago

snk7891 commented 8 years ago

I was thinking about adding a couple of useful properties to this awesome package.

I usually share configuration between backend and frontend, for example API endpoints, client tokens and so on. What I would find useful is a feature that let me specify some "private properties" that are not brought to the frontend angular configuration file. I ususally specify those properties with a heading underscore, so I added these two properties to gulp-ng-config:

Secondly, I usually attach all properties to a single object, for example:

angular.module("app.config", [])
.constant("Config", {
  "auth": {
    "clientToken": {
      "name": "x-client-token",
      "value": "abcde"
    }
  },
  "restApiRoot": "/api/v1"
});

This way, you can avoid some namespace pollution issues in your controllers.

If you think they could be useful, I would like to add some tests and maybe push everything to the main repo.

Regards,

Roberto

atticoos commented 8 years ago

This is an interesting idea, thanks for contributing!

This sounds like a scenario that is general enough for the package to cover, but I think in its current form it's specific to your scenario.

Correct me if I'm mistaken, it sounds like the idea is to omit certain properties based on a rule? Currently this is expressed by stripPrivateProperties and stripHeadingCharacter -- so a boolean to indicate whether or not to perform the action, and a string to define the rule for the action (the character pattern to omit).

If I'm accurate here, I could see this moving more in a direction of a "blacklist", which could contain a pattern or collection of patterns. For example:

gulpNgConfig('app.config', {
  blacklist: /^_.*/ // any properties starting with `_`
})

or as a collection of rules

gulpNgConfig('app.config', {
  blacklist: [
    /^_.*/, // any properties starting with `_`
    /^-.*/  // any properties starting with `-`
    // etc..
  ]
})

And purely based on the existence of this blacklist would it attempt to omit any matches. Would this support your scenario?

snk7891 commented 8 years ago

Definitely! It supports my scenario and it is a far better idea. I think that a single regexp should be enough, since you can also specify alternatives with '|'. I will prepare a new commit with this in mind... If the blacklist property is specified, it will strip matching properties, otherwise it won't do anything.

Do yout think that the other property embedInObject could be useful? Otherwise I'll get rid of it.

Thanks!