Ava: Nested configuration files

[novemberborn]

See the proposed solution here: https://github.com/avajs/ava/issues/2185#issuecomment-529211615

---

As of v2.2.0 AVA supports a --config CLI flag that lets you load a different configuration file. Currently that file is required to be in the same directory as the package.json file. This is because the configuration may contain glob patterns, Babel options, and require paths that are relative. It's hard to know how to correctly resolve those patterns, options and paths.

We also don't have an official way of extending a configuration. The suggestion is to use object spread, like so:

import baseConfig from './ava.config.js'

export default {
  ...baseConfig,
  files: ['./integration-tests/**/*']
}

If the configuration files are in different directories we'll end up with a mix of relative patterns, options and paths.

If we want to allow nested configuration files we need to solve:

• [ ] How to rewrite glob patterns, relative to the directory the configuration file is in
• [ ] How to interpret the Babel options relative to the directory the configuration file is in
• [ ] How to interpret require paths relative to the directory the configuration file is in
• [ ] Is the configuration file allowed to be outside the directory the package.json file is in?
• [ ] How do we rewrite or anchor patterns, options and paths from any file being extended?

There may be other issues I'm overlooking at the moment.

If you're interested in having support for nested configuration files please give this questions a thought and share your insights.

[rauschma]

General suggestions:

• I wouldn’t treat package.json and config files as mutually exclusive.
• I would support multiple “inline” configurations in package.json (note: value of "ava" is an array, not an object):
  json "ava": [ { "name:": "mjs", "babel": false, "compileEnhancements": false, "extensions": [ "mjs" ] }, { "name:": "js", "babel": false, "compileEnhancements": false, "extensions": [ "js" ] }, ]
  Now you can do:
  
  
  
  • --config mjs
  
  
  • --config js
  
  
  • --config some-config-file.js
  
  
• The ability to “extend” other files would be very helpful for sharing data:
  
  
  
  • "extend": "mjs"
  
  
  • "extend": "js"
  
  
  • "extend": "some-config-file.js"
  
  

Handling paths:

• I would support three kinds of paths and globs (the latter could be tricky):
  
  
  
  • Relative to current file: ./subdir/foo.js, other-subdir/bar.js, ../parent-dir/baz.js
  
  
  • Absolute: /home/rauschma/tmp/qux.js
  
  
  • Relative to project root (=directory in which package.json is stored): $PROJ/dir/file.js or $ROOT/dir/file.js (or similar)
  
  
• Alternative: Treat all relative paths and globs as relative to the package.json directory.
• Internally, paths should maybe be normalized to absolute paths.

[novemberborn]

Hey @rauschma thanks for your feedback.

• I wouldn’t treat package.json and config files as mutually exclusive.

Since we don't merge configurations, we decided to make them mutually exclusive to avoid confusion as to which file was being used.

• I would support multiple “inline” configurations in package.json (note: value of "ava" is an array, not an object):

Oh that's quite interesting!

• Treat all relative paths and globs as relative to the package.json directory.

Yes that would make merging configurations easier, but at the risk of users trying to use paths relative to their configuration file regardless. Unless we mandate a $packageRoot prefix so at least it's made explicit?

[novemberborn]

https://github.com/avajs/ava/issues/2234 pertains a use case where the config file is above the package being tested, rather than nested within it.

I wonder if we could side-step all these issues by making the config file explicitly confirm the project directory:

export default ({ projectDir }) => ({ // projectDir being the directory holding the closest package.json file to where the CLI was invoked
  projectDir, // Confirm what project dir is used. Everything is resolved relative to this file, but you could change it too
})

[sovanyio]

Being able to override the directory in the config seems reasonable to me.

In my circumstance referred to in #2234, the path blobs in the root level config are explicitly generic to cover resolution from the root or the sub-projects.

[novemberborn]

Being able to override the directory in the config seems reasonable to me.

To be clear, what I'm suggesting is that by returning the projectDir, the config file "confirms" it knows it's located elsewhere, and is taking responsibility for the various relative paths and patterns it may contain.

It's a little artificial because you're just returning a value that's passed to factory function but we can deal with that in documentation. And it does allow you to change the project directory if need be, which is nice.

So we can loosen the config location check by allowing configurations that return a projectDir. We need to improve the error message to make this clear, and update the documentation. Finally, we need to use the projectDir returned by the configuration when we create the Api instance and probably for the ESLint plugin helper too.

[sovanyio]

Sure, but as a user that is attempting to handle the resolution outside of the expected root directory, I am already aware of the pitfalls of the resolution issues. It seems heavy handed to enforce an opinion about configuration and path resolution here.

I would expect that the config resolution would work like other node front-end tools and look up the directory tree for configuration files and use node's native resolution according to the context it was run. Confirming a non-default directory here is sufficient for the use case, if opinionated.

[novemberborn]

I would expect that the config resolution would work like other node front-end tools and look up the directory tree for configuration files and use node's native resolution according to the context it was run.

Well, yes, but the point of this issue is that "just make it work" might be rather difficult given the mix of relative patterns, Babel options and require paths. Not doing any corrections would not meet user expectations either.

Confirming a non-default directory here is sufficient for the use case, if opinionated.

I think it's the least bad solution. Rewriting the patterns, options and paths may just be unnecessarily difficult to implement.

[shellscape]

What strikes me is that there's already a reasonable default solution to this, written by Ava's author no less: https://github.com/sindresorhus/find-up

The decision tree is then pretty simple:

• Config specified via option/flag?
• Config in package.json?
• Config in local file (start at CWD and find-up)?

I would find it immensely useful if ava would simply traverse up looking for one-config-to-rule-them-all for our monorepo. We're using pnpm for monorepo management and triggering tests with the --config flag has been a pain.

In our monorepo setup, having no support for this means _every single package_ (there are around 30) is going to have to contain ava config, or an ava.config.js file in every package directory. If we don't do that, we have to write _extra tooling_ to copy and delete config files. The developer burden in this case makes for really poor DX.

[novemberborn]

• Config in local file (start at CWD and find-up)?

@shellscape here's the rub.

AVA uses find-up to find the closest package.json from where you invoke it. That's considered to be the "project directory" and all tests found in that directory are executed. (Regardless, I should add, if those tests are nested "projects".)

Configuration needs to apply to the project as a whole. We don't support per-file (or directory) config.

As laid out at the start of this issue, the challenge with having configuration files that are in different directories from the "project directory" is that any relative values are still resolved to that "project directory", not the location of the configuration file.

The proposal for solving that is to allow configuration files to specify the project directory they apply to. As it happens we're already providing that value when you export a factory function in your configuration file.

Specifically in https://github.com/avajs/ava/issues/2185#issuecomment-529211615 I propose a "confirmation" that the configuration applies to the project directory that AVA found. As a follow-up we could allow configuration files to override the project directory.

I would find it immensely useful if ava would simply traverse up looking for one-config-to-rule-them-all for our monorepo. We're using pnpm for monorepo management and triggering tests with the --config flag has been a pain.

In our monorepo setup, having no support for this means _every single package_ (there are around 30) is going to have to contain ava config, or an ava.config.js file in every package directory. If we don't do that, we have to write _extra tooling_ to copy and delete config files. The developer burden in this case makes for really poor DX.

Yea I hear ya. I suppose, besides determining the project directory, AVA could continue looking for ava.config.js files. But we'd need to know where to stop! Is there anything about (typical) monorepo setups so we could mandate the ava.config.js file can only exist in the monorepo root?

Alternatively https://github.com/avajs/ava/issues/2167 would let you specify the path in the package.json file, provided this issue is resolved, but that still requires setup within each package.

I'd love to explore this further, but perhaps in a dedicated issue. We'll still need to resolve this issue, as per the proposal in https://github.com/avajs/ava/issues/2185#issuecomment-529211615. PR welcome 😉

[shellscape]

Thanks for the very detailed reply. Much appreciated.

I tend to agree with @sovanyio here:

Sure, but as a user that is attempting to handle the resolution outside of the expected root directory, I am already aware of the pitfalls of the resolution issues. It seems heavy handed to enforce an opinion about configuration and path resolution here.

I'd personally think a warning here would be more apt. If the user attempted to use a config outside of the project directory, a warning would be displayed on the console. Something along the lines of "Ava is running tests for X with a config from Y".

Confirming a non-default directory here is sufficient for the use case, if opinionated.

Not only opinionated, but - and I say this without snark - a rather over-engineered solution. Multiple config file return types is more of a barrier to user adoption than is understanding where a file is placed. I speak from experience, having been a webpack core member for a stint - it still confuses webpack users to this day.

But we'd need to know where to stop! Is there anything about (typical) monorepo setups so we could mandate the ava.config.js file can only exist in the monorepo root?

find-up has never let me down and I've never had to tell it where to stop. Eventually, and quite quickly it'll hit the root. I don't think we need to mandate anything in this case. There's good practice that we can always warn about, but there's no need to not allow people to get crazy if they want to get crazy.

[novemberborn]

I tend to agree with @sovanyio here:

Sure, but as a user that is attempting to handle the resolution outside of the expected root directory, I am already aware of the pitfalls of the resolution issues. It seems heavy handed to enforce an opinion about configuration and path resolution here.

I'd personally think a warning here would be more apt. If the user attempted to use a config outside of the project directory, a warning would be displayed on the console. Something along the lines of "Ava is running tests for X with a config from Y".

Confirming a non-default directory here is sufficient for the use case, if opinionated.

Not only opinionated, but - and I say this without snark - a rather over-engineered solution. Multiple config file return types is more of a barrier to user adoption than is understanding where a file is placed. I speak from experience, having been a webpack core member for a stint - it still confuses webpack users to this day.

Fair enough, we could start with a warning and see how that goes. We now actually have warning output for when experimental features are enabled, so we could build on that.

But we'd need to know where to stop! Is there anything about (typical) monorepo setups so we could mandate the ava.config.js file can only exist in the monorepo root?

find-up has never let me down and I've never had to tell it where to stop. Eventually, and quite quickly it'll hit the root. I don't think we need to mandate anything in this case. There's good practice that we can always warn about, but there's no need to not allow people to get crazy if they want to get crazy.

IIRC this was one of the reasons babel.config.js stops at the package.json boundary. Too many cases where a higher directory contained some Babel config that was then applied.

[shellscape]

Excellent point about babel. I do recall the wild west days when phantom higher-level files were an issue.Would a --max-config-level flag make sense here? Default could be 0 to keep it as it is now, and this could still work well with allowing --config to be set to any location.

[novemberborn]

We're talking about different things here. One issues is that we've played it safe so far and required the config file to be next to the package.json. If we print a warning when it's not, then that's solved.

The other is that in monorepos you'd like to discover an ava.config.js higher in the file hierarchy. That's a separate feature, one I'm happy to support, provided we have a way of stopping the search for that file before it hits, say, your home directory. The question is how would we determine the root of a monorepo? One solution may be to allow for such files provided they're in the same directory as a .git directory. So it's not arbitrary.

[shellscape]

Looking for .git is an excellent call. That's got my support. I have some time today, depending on work, that I might be able to get some PRs in for this.

_Update: I've been dogged by a plague I got from my 8 mo old. Gonna be a week or few before I can get to this_

[novemberborn]

Hope you're feeling better @shellscape!

I've opened #2284 and #2285 to track the resolutions to our discussions here. Thanks all for participating!

[shellscape]

Oh man, I totally let this fall through the cracks, my apologies for that. I'll check the two new issues.