standard-engine icon indicating copy to clipboard operation
standard-engine copied to clipboard

feat: add option to configure standard via rc config file

Open TillaTheHun0 opened this issue 5 years ago • 12 comments

Thanks for Standard! ❤️

This adds the ability to configure the StandardJS engine outside of package.json, ie. in a file like .standardrc, .standardrc.js, etc. Config in package.json is still prioritized over those options though, so no current behavior is changed. This removes a dep on pkg-conf and adds a dep on cosmiconfig.

From cosmiconfig README:

By default, Cosmiconfig will start where you tell it to start and search up the directory tree for the following:

- a `package.json` property
- a JSON or YAML, extensionless "rc file"
- an "rc file" with the extensions `.json`, `.yaml`, `.yml`, or `.js`.
- a `.config.js` CommonJS module

For example, if your module's name is "soursocks", cosmiconfig will search up the directory tree for configuration in the following places:

- a `soursocks` property in `package.json`
- a `.soursocksrc` file in JSON or YAML format
- a `.soursocksrc.json` file
- a `.soursocksrc.yaml`, `.soursocksrc.yml`, or `.soursocksrc.js` file
- a `soursocks.config.js` file exporting a JS object

I personally prefer tooling configuration outside of package.json bc then package.json will change only when a dep or script is updated, basically; easier to spot in a PR. Also makes it easier to spot when tooling config changes in a PR as well.

Also added a test for it as well.

Let me know what else it would need and thanks again!

TillaTheHun0 avatar Mar 06 '19 03:03 TillaTheHun0

This is very useful as it means our package.json only changes when deps change instead of also configs. Any view on this PR?

samhatoum avatar May 15 '19 17:05 samhatoum

Any view on this? I see that standardx allows for a similar thing, via the eslintrc file, but I think this would be a nice thing to have built into vanilla standard.

TillaTheHun0 avatar Jul 20 '19 22:07 TillaTheHun0

I like this idea since I assume the primary goal is to allow a .standardrc within the project directory? Is there an implied feature that ~/.standardrc would be read as well? I'd be 👎 on that based on my (admittedly brief) skim through cosmiconfig's docs. It seems that cosmiconfig supports a searchPlaces option which would allow us to respect the XDG basedir spec. Without this, we would be removing users' ability to control how they want their configuration stored.

That said, if we can define searchPlaces in this PR to be XDG-compliant, then I'm on board!

jasonkarns avatar Mar 24 '20 19:03 jasonkarns

@jasonkarns correct, that was my idea. I configure effectively all of my other tooling using dedicated config files except standard bc it currently doesn't support it. I could technically just use eslint directly and use the standard plugins in an .eslintrc.*, but that seems to cut against what standard is trying to accomplish; something more out of the box would be nice.

You make an interesting point. Admittedly, I am not familiar with the XDG basedir spec, besides just skimming it, but definitely familiar with some of the problems that it's trying to address 😄. I see some tools I use, like gatsby and verdaccio, are adhering to the relevant portion of the spec.

ESLint currently uses ~/.eslintrc.* if no configuration file is found in the project. This will be deprecated starting in v7, and removed in v8.

Aligning with ESLint and how it scans for a config file seems more kosher for this project IMO. That being said, if a maintainer thinks otherwise, perhaps merging #214, i'll be happy to make the changes that you suggested.

TillaTheHun0 avatar Mar 25 '20 03:03 TillaTheHun0

It looks like #213 fixes those failing tests that are also failing on master. Perhaps, when that PR is merged, this and #214 can be rebased onto that. 👍

TillaTheHun0 avatar Mar 25 '20 03:03 TillaTheHun0

Good idea, unfortunately there are conflicts with master branch! Once conflicts are solved, we could review that PR and probably merge that to master! Thanks for your contribution! @TillaTheHun0

theoludwig avatar Dec 10 '20 13:12 theoludwig

Thanks for the followup @Divlo . I can rebase and fix the conflicts. Though I need to make some changes to compliment #214 w.r.t @jasonkarns comment. Looks like cosmiconfig doesn't respect XDG (https://github.com/davidtheclark/cosmiconfig/issues/152), so I am currently trying to put together a PR there that will hopefully be merged. Then we could use that here.

TillaTheHun0 avatar Dec 11 '20 16:12 TillaTheHun0

@Divlo I was able to figure out how to fallback to XDG config base dir without the PR to cosmiconfig. Though I probably will still try to PR there.

Basically what happens is standard-engine does the regular search for a config file, up to the user's home dir. If a config isn't found, then it will check XDG_CONFIG_HOME/${cmd} for a config* file. I also added a test for this.

I think this is ready to go. Let me know if it needs anything else!

TillaTheHun0 avatar Dec 12 '20 02:12 TillaTheHun0

I fixed the standard style, and added a section to the README. Let me know if any changes are needed.

TillaTheHun0 avatar Dec 13 '20 02:12 TillaTheHun0

Hmm, looks like the test for XDG is failing on CI. The tests work on my local, so this is odd. 🤔

TillaTheHun0 avatar Dec 13 '20 02:12 TillaTheHun0

Hmm, looks like the test for XDG is failing on CI. The tests work on my local, so this is odd.

It seems like the tests are also failing on my local machine. (I'm on Ubuntu 20.04 LTS) Capture2

theoludwig avatar Dec 13 '20 09:12 theoludwig

I wasn't clearing the require cache properly in the test, but it was using my own XDG_CONFIG_HOME so still passing 😅 . Should be fixed now!

TillaTheHun0 avatar Dec 13 '20 15:12 TillaTheHun0