Extended properties cannot be override

molant commented 4 years ago

Link to bug demonstration repository

We are trying to update to v15 in https://github.com/webhintio/hint, the PR is https://github.com/webhintio/hint/pull/3513

And the relevant output of CI is here

Expected Behavior

webhint uses a monorepo approach where we have a common .nycrc file in the root of the project. In that file we set the minimum threshold to 80 for branches. Some packages might override the default configuration. packages/hint is one of those where the package.json has an nyc property that sets "branches": 75.

Unfortunately, it seems like with the newest nyc version, properties in the extends file cannot be override.

Observed Behavior

I've tried:

removing 80 from extends and having 75 in the package: test pass (expected as the coverage is 75.66%)
removing 80 from extends and having 80 in the package: test fail (expected)
changing 80 by 90 in the extends. Test fail saying the minimum threshold of 90 is not meet

This does not happen with previous nyc versions.

The documentation says:

You can then add the specific configuration options you want that aren't in that particular shared config, e.g.

(emphasis is mine) which maybe believe this might be intentional, but seems like a huge step back (ie.: having to have to set the coverage in each project).

Troubleshooting steps

[x] still occurring when I put cache: false in my nyc config

Environment Information

  System:
    OS: Linux 4.19 Ubuntu 18.04.4 LTS (Bionic Beaver)
    CPU: (8) x64 Intel(R) Core(TM) i7-1065G7 CPU @ 1.30GHz
    Memory: 8.63 GB / 12.17 GB
  Binaries:
    Node: 12.16.0 - ~/.nvm/versions/node/v12.16.0/bin/node
    Yarn: 1.21.1 - /usr/bin/yarn
    npm: 6.13.4 - ~/.nvm/versions/node/v12.16.0/bin/npm

coreyfarrell commented 4 years ago

I believe the issue is that .nycrc has a higher priority than package.json. So hint/package.json is being loaded, the extends is processed, but then .nycrc is found and overwrites your branches value again. Can you please try creating hint/.nycrc containing {}? This would block the root .nycrc from being found from within the hint directory. Another option is to just move your nyc options from hint/package.json to hint/.nycrc. I believe this should resolve the conflict.

Regarding what you see in the README I think the wording is not perfect. You can add options that overwrite the configuration being extended but they will overwrite (rather than merge). This is of particular interest if you are dealing with say @istanbuljs/nyc-config-babel which sets require: ['@babel/register']. Extending the babel config then setting require: ['ts-node/register'] would replace @babel/register.

molant commented 4 years ago

Thanks @coreyfarrell for the suggestions, but unfortunately none seem to work or I don't know what I'm doing wrong 😞

packages/hint/.nycrc with { } and current content in packages/hint/package.json fails
packages/hint/.nycrc with the following content and removing the nyc property from packages/hint/package.json fails as well:

{
  "branches": 75,
  "extends": "../../.nycrc"
}

During theses tests I've seen that nyc will go up the tree until it finds a .nycrc file, which is pretty cool. Will remove a bunch of properties from other packages 😊

alasdairhurst commented 4 years ago

I'm seeing this problem too - again coming from the monorepo approach and trying to solve this from a few different angles.

The behaviour of extends seems to be broken since it takes the values coming from "extends" over any values defined in the config file.

package.json

"nyc": {
  "extends": "path/to/.nycrc",
  "all": true,
  "per-file": true,
  "lines": 80
}

path/to/.nycrc

{
  "lines": 100,
  "per-file": false
}

The resulting config that nyc loads looks like this

{
  "lines": 100,
  "per-file": false,
  "all": true
}

This needs to be fixed in https://github.com/istanbuljs/load-nyc-config and would probably be counted as another breaking change.

jackmellis commented 4 years ago

Having this problem too, just to confirm as a non-monorepo-usecase. We have multiple separate packages that use a shared nycrc.yaml that we add as a node_module:

extends: "@internal/nyc-config"
branches: 0

I can manually edit the extended config and nyc is definitely picking it up, but I can't override any of its options...

jongear commented 3 years ago

I was able to work around this by creating an nyc.config.js file at the root of my project and applying my overrides there.

nyc.config.js:

const settings = require('path/to/nyc-config');

settings.exclude = settings.exclude.concat([
  'nyc.config.js',
  'tools/**'
]);

settings.lines = 80;

module.exports = settings;

ThangHuuVu commented 3 years ago

I am also experiencing this issue in "15.1.0". The common config file that we are reusing across projects is .nycrc, which makes it inconvenient to refactor (I would have to change in all other projects)

gstamac commented 3 years ago

I believe the issue is in here. It overwrites the configuration with extended configuration but it should be the other way around.

antross commented 3 years ago

For anyone else trying to work around this for a monorepo scenario, I was able to do so by adding my extends clause in a local package.json and putting the overrides in a local .nycrc. Both get picked up and the local .nycrc is applied last so the overrides still take effect.

See https://github.com/webhintio/hint/pull/4734/commits/cca423d228b57e89a59da4ab1d62c8d15a5b5214

istanbuljs / nyc