Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extended properties cannot be override #1286

Open
1 task done
molant opened this issue Mar 4, 2020 · 8 comments · May be fixed by istanbuljs/load-nyc-config#19
Open
1 task done

Extended properties cannot be override #1286

molant opened this issue Mar 4, 2020 · 8 comments · May be fixed by istanbuljs/load-nyc-config#19

Comments

@molant
Copy link

molant commented Mar 4, 2020

Link to bug demonstration repository

We are trying to update to v15 in https://github.com/webhintio/hint, the PR is webhintio/hint#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

  • 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
Copy link
Member

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
Copy link
Author

molant commented Mar 8, 2020

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
Copy link

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
Copy link

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
Copy link

jongear commented Oct 27, 2020

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
Copy link

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
Copy link

gstamac commented Aug 2, 2021

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

antross added a commit to webhintio/hint that referenced this issue Sep 23, 2021
Switches to extend in package.json but override in local .nycrc
to work around istanbuljs/nyc#1286
@antross
Copy link

antross commented Sep 23, 2021

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 webhintio/hint@cca423d

antross added a commit to webhintio/hint that referenced this issue Sep 23, 2021
This change removes webhint's various telemetry prompts, packages, and hooks.

As part of migrating hosting for webhint.io we're shutting down the telemetry service.
Going forward we'll rely on telemetry from the various integrations and extension
stores rather than a separate, dedicated service.

Also works around a bug in nyc: istanbuljs/nyc#1286

- - - - - - - - - - - - - - - - - - - -

Close #4734
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants