Add support for `ava.config.js` #192

HenriBeck · 2018-05-31T20:58:52Z

Adds support for checking the ava.config.js when there is no ava config inside package.json.
This uses the esm module for parsing the config file.

Fixes #191

novemberborn

Thanks for the PR @HenriBeck!

Config loading is a little more involved, I've left some inline comments.

Would be good to have tests as well.

novemberborn · 2018-06-04T13:39:53Z

util.js

@@ -33,7 +34,15 @@ exports.getAvaConfig = filepath => {

 	try {
 		const packageContent = JSON.parse(fs.readFileSync(filepath, 'utf8'));
-		return (packageContent && packageContent.ava) || defaultResult;
+
+		if (packageContent && packageContent.ava) {


Strictly speaking AVA ignores "ava": {} in favor of an ava.config.js file. We should do the same here.

From the docs:

To use an ava.config.js file: It must be in the same directory as your package.json Your package.json must not contain an ava property (or, if it does, it must be an empty object)

I thought it would prefer the ava object from package.json

No it throws an exception if it finds config in package.json (that is, the "ava" key contains a non-empty object) as well as an ava.config.js file. Therefore if you find an empty object here, you need to check for ava.config.js as well. But it may not exist, and that should be OK.

I'm not sure what we should do here when both configurations are provided. If we throw an exception how does ESLint surface that?

When you throw an error in ESLint, it just ends the process. So I think throwing an error isn't an option here.

novemberborn · 2018-06-04T13:42:03Z

util.js

+		const fileOptions = path.parse(filepath);
+		const avaConfig = esmRequire(path.join(fileOptions.dir, 'ava.config.js'));
+
+		return avaConfig.default;


The config file may export a factory method that's invoked with an options argument. For here, that should be {projectDir: fileOptions.dir}.

We'll need to ignore MODULE_NOT_FOUND errors and return defaultResult instead.

Signed-off-by: Henri Beck <[email protected]>

HenriBeck · 2018-06-06T17:56:54Z

@novemberborn I added support for the function.
About the first change request, either I miss understood the ava docs or they are wrong.

novemberborn · 2018-06-11T17:12:55Z

util.js

@@ -33,7 +34,15 @@ exports.getAvaConfig = filepath => {

 	try {
 		const packageContent = JSON.parse(fs.readFileSync(filepath, 'utf8'));
-		return (packageContent && packageContent.ava) || defaultResult;
+
+		if (packageContent && packageContent.ava) {


No it throws an exception if it finds config in package.json (that is, the "ava" key contains a non-empty object) as well as an ava.config.js file. Therefore if you find an empty object here, you need to check for ava.config.js as well. But it may not exist, and that should be OK.

I'm not sure what we should do here when both configurations are provided. If we throw an exception how does ESLint surface that?

novemberborn · 2018-06-11T17:13:59Z

util.js

@@ -2,6 +2,7 @@
 const fs = require('fs');
 const path = require('path');
 const pkg = require('./package');
+const esmRequire = require('esm')(module);


To be in sync with AVA itself, you need to set {cjs: false, mode: 'all'} options.

novemberborn · 2018-07-13T15:08:06Z

I've fixed the merge conflicts and brought the logic in line with AVA itself. I've also added some tests and updated the relevant rule documentation.

@HenriBeck are you happy with these changes?

@sindresorhus any thoughts? Would be good to merge and ship so we're compatible with both the latest AVA and ESLint.

HenriBeck · 2018-07-13T17:45:55Z

Yeah, I think it's the right thing to return the default config when there is no config/invalid config because throwing an error would completely abort eslint.

A rule could be added which would validate the config maybe.

novemberborn · 2018-07-14T12:23:41Z

Yeah, I think it's the right thing to return the default config when there is no config/invalid config because throwing an error would completely abort eslint.

Thanks. I'm waiting for a new esm release that adds an option we need, so will merge and release when that's done.

A rule could be added which would validate the config maybe.

Yea, see #199.

sindresorhus · 2018-07-14T18:23:41Z

Looks great :)

Co-authored-by: Mark Wubben <[email protected]> Adds support for checking the `ava.config.js` when there is no `ava` config inside `package.json`. This uses the `esm` module for parsing the config file. Fixes #191

Adds support for checking the `ava.config.js` when there is no `ava` config inside `package.json`. This uses the `esm` module for parsing the config file. Fixes #191 Co-authored-by: Mark Wubben <[email protected]>

Must be similar to avajs#192

Add support for the ava.config.js

6896247

novemberborn requested changes Jun 4, 2018

View reviewed changes

Added support for exporting function from the ava config

2988c6f

Signed-off-by: Henri Beck <[email protected]>

novemberborn reviewed Jun 11, 2018

View reviewed changes

HenriBeck mentioned this pull request Jun 16, 2018

Add no-import-test-file rule #193

Merged

sindresorhus changed the title ~~Add support for the ava.config.js~~ Add support for ava.config.js Jun 24, 2018

sindresorhus and others added 9 commits June 24, 2018 15:19

Update util.js

68e0352

Merge branch 'master' into pr/192

83bb7a0

Update esm

f1a7f08

Configure esm the same as AVA itself

d0a3f1f

Clarify argument to getAvaConfig()

27fc3d2

Assign projectDir variable

091c29f

Match AVA's config loading behavior, minus the exceptions

62bf63e

Test new config resolving code

161a1c2

Update docs

899adb7

novemberborn approved these changes Jul 13, 2018

View reviewed changes

novemberborn mentioned this pull request Jul 14, 2018

Detect config file mishaps #199

Open

sindresorhus merged commit 415b7ba into avajs:master Jul 14, 2018

forresst added a commit to forresst/eslint-plugin-ava that referenced this pull request Jul 18, 2018

Add support for ava.config.js in no-import-test-files.md

2c87d90

Must be similar to avajs#192

forresst mentioned this pull request Jul 18, 2018

Add support for ava.config.js in no-import-test-files.md #204

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for `ava.config.js` #192

Add support for `ava.config.js` #192

HenriBeck commented May 31, 2018 •

edited by sindresorhus

Loading

novemberborn left a comment

novemberborn Jun 4, 2018

HenriBeck Jun 5, 2018

novemberborn Jun 11, 2018

HenriBeck Jun 16, 2018

novemberborn Jun 4, 2018

HenriBeck commented Jun 6, 2018

novemberborn Jun 11, 2018

novemberborn Jun 11, 2018

novemberborn commented Jul 13, 2018

HenriBeck commented Jul 13, 2018

novemberborn commented Jul 14, 2018

sindresorhus commented Jul 14, 2018

Add support for ava.config.js #192

Add support for ava.config.js #192

Conversation

HenriBeck commented May 31, 2018 • edited by sindresorhus Loading

novemberborn left a comment

Choose a reason for hiding this comment

novemberborn Jun 4, 2018

Choose a reason for hiding this comment

HenriBeck Jun 5, 2018

Choose a reason for hiding this comment

novemberborn Jun 11, 2018

Choose a reason for hiding this comment

HenriBeck Jun 16, 2018

Choose a reason for hiding this comment

novemberborn Jun 4, 2018

Choose a reason for hiding this comment

HenriBeck commented Jun 6, 2018

novemberborn Jun 11, 2018

Choose a reason for hiding this comment

novemberborn Jun 11, 2018

Choose a reason for hiding this comment

novemberborn commented Jul 13, 2018

HenriBeck commented Jul 13, 2018

novemberborn commented Jul 14, 2018

sindresorhus commented Jul 14, 2018

Add support for `ava.config.js` #192

Add support for `ava.config.js` #192

HenriBeck commented May 31, 2018 •

edited by sindresorhus

Loading