alarner / perk

A well documented set of tools for building node web applications.
http://perkframework.com
MIT License
181 stars 31 forks source link

Add the ability for `npm run config` to load in previous configuration values living in the local.js file. #7

Closed alarner closed 7 years ago

alarner commented 8 years ago

Right now it will simply override the old values but there is no way to edit. This may require updating / adding some functionality to https://github.com/alarner/config-template

aurium commented 7 years ago

As i see it need more work on https://github.com/alarner/config-template, as you pointed.

I liked config-template! I believe it is a really nice tool, doing its first steps. :-)

The config-template's main function at module.exports must accept some options. I believe that must get the new signature function(configTemplate, options), where options should be an object, allowing more future configuration. For now options will get values, like configTemplate(tmpl, {values:currentConfigValues}).

The main function must add options.values in the lines data structure, allowing the editor to start with the values without more deep changes.

In this project, i see perk/build/config.js is where configTemplate() is called and local.js is written. I believe it simple need to require('.../local.js') and use it as options.values to configTemplate().

What you think @alarner? Should i code this?

alarner commented 7 years ago

@atrium I think you are 100% correct. It would be amazing if you could make those updates!

One thing to think about is that configTemplate and options.values might not completely match. options.values might not have all of the properties that are in the configTemplate, but it might also have more properties. For example, configTemplate could have the properties sessionSecret and dbPassword while options.values has dbPassword and awsPrivateKey. Since the library provides some basic input validation it would be great for it to be able to guess the validation type for the new property (in this case awsPrivateKey). It should be possible to guess whether the new property should be a boolean, number or string.

If you feel like adding this extra feature that would be wonderful, but not required. The way you described your solution would be a huge improvement and I'd very much appreciate the help!

aurium commented 7 years ago

Nice! I can do this. :smiley:

If i understand you correctly, you mean to add options.values extra data to lines. I was thinking the template should not to be that flexible, so I was thinking options.values extra data should be ignored.

Well, booth use-cases are possible, so I may add options.appendExtraData and defaults to true.

typeof is enough to guess the validation type? I think yes, and the line.type='json' can be set when typeofreturns object.

Is it right?

aurium commented 7 years ago

@alarner, Could you add a related issue to https://github.com/alarner/config-template ? :wink:

alarner commented 7 years ago

@aurium just added this issue: https://github.com/alarner/config-template/issues/4

Yes! You understand correctly. The reason I think that's important for Perk is that developers can manually adjust the local.js file and we wouldn't want to overwrite those changes if they add a new property, but fail to also add it to the local.template.js file. Again, I'm very appreciative of the help, so do as much or as little of those extra features as you want.

typeof should be good enough, but lodash is already included as a dependency so you may want to use that for type checking if you run into any weird JavaScript inconsistencies using typeof.