JavaScript config files #114
Conversation
|
❤️ Love this, create contribution @elliotdickison. |
lib/extend.js
Outdated
| if (_.isString(opts)) { | ||
| options = fs.readJsonSync(opts); | ||
| options = require(path.resolve(process.cwd(), opts)); |
There was a problem hiding this comment.
Just a conversation starter, the recent addition to do something similar for style dictionary files (#89) used the resolve-cwd module. Open question to you and @jreichenberg: should we use resolve-cwd here as well? Should we remove the resolve-cwd module and use this way in combineJSON.js? Does it matter?
There was a problem hiding this comment.
I missed that dependency. Not sure why it's necessary, but I don't really care one way or the other happy to add it in for consistency. @jreichenberg what's the value add of that dependency?
There was a problem hiding this comment.
@elliotdickison See the discussion in 89: #89 (comment)
Both the native node require() statement and readJsonSync will parse any JSON you throw at it. The difference is that native require() will resolve packages in the process (yay!) but needs some help resolving a path relative to the cwd (boo!). So kind of depends on the desired capability. I know that doesn't help much...
test/configs/test.js
Outdated
| @@ -0,0 +1,87 @@ | |||
| module.exports = { | |||
There was a problem hiding this comment.
Would it be silly to just require the test.json file here so we don't have to keep the 2 files in sync? I mean, we probably won't be changing these much as the tests would break if we did. I dunno just thinking out loud.
There was a problem hiding this comment.
Ha that is awesome. Will do.
elliotdickison
force-pushed
the
js-config
branch
from
78db6b8 to
dcf8c3d
Compare
This change enables the user to use a JavaScript module for their config file. The goal of this change is to enable more flexible configuration similar to Webpack or Babel. For example filter functions, which previously could only be provided using the Node API, can now be included directly in a JS config file. JavaScript config files could also eventually remove the need for the register APIs because you could simply pass formatters, transformers, etc. as functions directly in the config instead of registering them with an alias and then referencing that alias in the config. This could potentially provide a simpler API very similar to the way webpack handles plugins: https://webpack.js.org/configuration/plugins/. Just a thought though. For now the default config file `./config.json` and the ability to continue to use JSON config files has not changed.
elliotdickison
force-pushed
the
js-config
branch
from
dcf8c3d to
633cf15
Compare
|
Switched to using |
LGTM
This change enables the user to use a JavaScript module for their
config file. The goal of this change is to enable more flexible
configuration similar to Webpack or Babel. For example filter
functions, which previously could only be provided using the Node
API, can now be included directly in a JS config file.
JavaScript config files could also eventually remove the need for
the register APIs because you could simply pass formatters,
transformers, etc. as functions directly in the config instead of
registering them with an alias and then referencing that alias in
the config. This could potentially provide a simpler API very
similar to the way webpack handles plugins:
https://webpack.js.org/configuration/plugins/. Just a thought
though. For now the default config file
./config.jsonand theability to continue to use JSON config files has not changed.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.