Add option to read build options from file #37
Conversation
Well this is ironic. No idea how to test this either.
Without the `run`, we don't get the special vars `status` and `output` set by bats, which makes testing a pain. This doesn't seem to break any tests.
Inspired by this [comment][1] on #27, this change adds the ability for ez-build to read options from files, and placing their contents precisely in the order the user intended. This is essentially the same as just printing the contents of the files verbatim, taking care to allow newlines by splitting on *all* whitespace. This allows us to re-use ez-build configuration across scripts, in a very predictable and explicit manner, as opposed to having magical lookup rules that can be hard to reason about. [1]: #27 (comment)
Also change the docs to go with the `build:dev` script convention rather than just `dev`. I don't have any good reasons for this, other than it looks neater.
|
Bumped minor version because of the feature change. |
Current coverage is 85.21% (diff: 100%)@@ master #37 diff @@
==========================================
Files 15 15
Lines 244 257 +13
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 198 219 +21
+ Misses 46 38 -8
Partials 0 0
|
mstade
changed the title
|
@mamapitufo – a penny for your thoughts! |
|
@DimitarChristoff, @Poetro – would love to hear your thoughts on this as well! |
|
I like it! Good decision on going explicit, and the implementation is very simple. |
|
@mamapitufo I miss working with you on a daily basis man, thanks for the feedback! :o) |
| ``` | ||
|
|
||
| ## Using ez-build in npm scripts | ||
|
|
||
| A typical scripts section of a package.json when using ez-build looks something like this: | ||
|
|
||
| ```json | ||
| { "dev": "ez-build --interactive" | ||
| , "build": "ez-build --production" | ||
| { "build": "ez-build --production" |
There was a problem hiding this comment.
i would personally prefer to see env === 'production' or env === 'development' due to the nature of many other bits (like react) being aware of the env setting and changing output.
then the script diffs can become cross-env env=production ez-build with a sensible default.
There was a problem hiding this comment.
Interesting, I didn't know that. I didn't find much by way of documentation on this, but what I could find, and from the issues in the React repo, it seems there are significant sections of code affected by the __DEV__ global, presumably set to whatever NODE_ENV is set to at build time (although I couldn't find this.) This whole thing also seem somewhat contentious, with changes possibly coming down the line.
I don't think moving to an unconventional environment variable API (with the cross platform issues it entails no less, as you point out) instead of your typical CLI flags makes sense. This would mean there are at least three ways to configure ez-build, and that's at least one too many I'd say. That said, I definitely see the value of setting NODE_ENV (I'm assuming that's what you meant to say, not env, but please do educate me otherwise!) during build time – not least if there are tools we use that provide significant benefits based on the switching of this variable. I created #38, where it can be discussed further – thanks for bringing it up!
|
|
||
| ```json | ||
| { | ||
| "build": "ez-build --production --include \"js:**/*.js,js:**/*.jsx\" --flags es2017", |
There was a problem hiding this comment.
if this supports glob patterns, you could write it as js:**/*.{js,jsx} - but it may also potentially benefit from not coupling to a structure that has a js folder - it could well be scripts or src or whatever.
There was a problem hiding this comment.
Good point re: globs! Unfortunately, due to how ez-build parses this flag, it probably won't work even though it does support globs. I'd consider this a bug, and we should fix it because you're right – this should be supported.
Regarding your second statement about a js folder – the js: part doesn't actually refer to any kind of directory structure. It's just a label for the JS pipeline, so --include "js:**/*.{js,jsx}" just means "for the JS pipeline, include these files in the build process." By default, ez-build assumes sources are found under src/, but this can be overridden by the -i|--src flag or by setting the directories key in package.json. The readme has more info on this as well.
Description
Apologies for the somewhat ironic PR title. This change allows users to specify options in a file, which then gets read whenever ez-build is invoked and pointed to that file. There is no magic to how these files are found and loaded – it's entirely explicit. Whenever ez-build encounters a CLI option that starts with
@, it will assume the rest of the string is a path to a file (relative to the current working dir) and load it under the assumption that the data within are more options. Files can include@references themselves, to nest the loading of options. This means there can be any number of options files, making this a pretty flexible feature.Motivation and Context
It came about when thinking about #27, and specifically when I wrote this comment. We've had a mounting problem of having to be increasingly wordy in our npm scripts, and as we invoke ez-build in different contexts with slightly different options, it is clear we need some way of re-using this configuration. I believe this is a good solution for that.
How Was This Tested?
These tests were added in opts-file.bats.
Types of changes
Checklist: