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

Files get ignored from coverage when esnext bigint is used. #947

Closed
evert opened this issue Dec 7, 2018 · 16 comments
Closed

Files get ignored from coverage when esnext bigint is used. #947

evert opened this issue Dec 7, 2018 · 16 comments

Comments

@evert
Copy link

evert commented Dec 7, 2018

Steps to reproduce.

  1. Have working nyc setup.
  2. Edit a file that already appeared in coverage info, and add the following function:
function foo() {
  return 5n;
}

Result is that the file disappears.

@bartekleon
Copy link

bartekleon commented Dec 16, 2018

+1 for that. I have even combined 2 projects, and file with any bigint is removed from coverage while others are checked. Please fix it :c

@bartekleon
Copy link

@evert it seems that when you replace all n notations to BigInt() it works again.
image

@coreyfarrell
Copy link
Member

#948 adds support for configuring the babel parser plugins to be used. This will be included in the next release and will allow you to add the bigInt parser. You can follow #972 for a notification of the next release.

https://babeljs.io/docs/en/babel-parser#plugins

@novemberborn
Copy link
Contributor

I've installed 13.2.0, installed @babel/plugin-syntax and added the following in my .nycrc:

  "plugins": [
    "@babel/plugin-syntax-bigint"
  ],

However the files with 0n are still excluded from the report. Perhaps I configured it wrong but I can't find any documentation for this either.

@coreyfarrell
Copy link
Member

You need to point to a babel parser plugin, not a babel plugin. So for this you need bigInt. Note that configuring the plugins option overwrites the existing plugins, so you may need to also add other babel parser plugins. See https://babeljs.io/docs/en/babel-parser#plugins for a list of valid plugins to be used here.

https://github.com/wzalazar/istanbuljs/blob/d23d023d43c7b030e708518e6791b67f39c4c9ad/packages/istanbul-lib-instrument/src/instrumenter.js#L86-L93

@novemberborn
Copy link
Contributor

Ah I see, thanks! Sorry it was the end of my day 👍

I think I got that to work. Did have to delete my nyc cache, perhaps plugins are not included in the cache key algorithm?

Calling this top-level configuration plugins is awfully generic though.

@coreyfarrell
Copy link
Member

Ah I see, thanks! Sorry it was the end of my day +1

I think I got that to work. Did have to delete my nyc cache, perhaps plugins are not included in the cache key algorithm?

Glad it's working for you. .nycrc definitely is not included. I thought that your package.json would be included but looking at the code for caching-transform I'm not confident. Command-line arguments definitely are not included.

Calling this top-level configuration plugins is awfully generic though.

Agreed in hindsight, parser-plugins would have been better. @bcoe @JaKXz can we do anything about this or is it too late? Maybe we can create an alias for now, delete the plugins option from nyc@14?

@JaKXz
Copy link
Member

JaKXz commented Feb 7, 2019

@coreyfarrell that's a good way to go - we could even just change the docs to only show parser-plugins.

@bartekleon
Copy link

bartekleon commented Feb 13, 2019

Ok, so I'm struggling with this for almost 2 hours. Can you tell me, how I am supposed to add this plugin? In package.json i have

"nyc": {
    "check-coverage": true,
    "lines": 100,
    "statements": 100,
    "functions": 100,
    "branches": 100,
    "include": [
      "src/**/*.js"
    ],
    "reporter": [
      "html"
    ],
    "require": [
      "@babel/register"
    ],
    "report-dir": "./coverage"
  },

in .babelrc

{
  "plugins": ["@babel/plugin-syntax-bigint"]
}

and I get nice error:
FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory

I have literally no idea. I can't find any doc talking about it, lurking in source code also doesn't provide any support.

@JaKXz
Copy link
Member

JaKXz commented Feb 13, 2019

I believe you need the plugins option in the nyc config as well @kmdrGroch

@bartekleon
Copy link

bartekleon commented Feb 13, 2019

@JaKXz if you mean

"nyc": {
    "check-coverage": true,
    "lines": 100,
    "statements": 100,
    "functions": 100,
    "branches": 100,
    "include": [
      "src/**/*.js"
    ],
    "reporter": [
      "html"
    ],
    "require": [
      "@babel/register"
    ],
    "plugins": [
      "@babel/plugin-syntax-bigint"
    ],
    "report-dir": "./coverage"
  },

and
.babelrc

{
  "parserOpts": {
    "plugins": ["@babel/plugin-syntax-bigint"] // tries to parse but error on first `10n` is thrown
  }
}

or

{
    "plugins": ["@babel/plugin-syntax-bigint"] // Javascript out of heap
}

dont work

@coreyfarrell
Copy link
Member

You do not need nyc to "require": "@babel/register" for this, I'd recommend against it unless you are using babel for other reasons. What you need is to add "plugins": ["bigInt"] to your nyc options. This option expects babel parser plugins, not babel plugins.

If you are still having trouble please post a new ticket with a link to a repository demonstrating the problem.

@coreyfarrell
Copy link
Member

CC @kmdrGroch @novemberborn @evert @wzalazar
FYI in [email protected] the option plugins is being renamed to parser-plugins, see #1031. This will be listed under breaking changes in the CHANGELOG.md.

@bcoe
Copy link
Member

bcoe commented Mar 19, 2019

@coreyfarrell as BigInts are in Node.js now, we should probably turn this on by default right?

@coreyfarrell
Copy link
Member

@kmdrGroch @novemberborn @evert @wzalazar starting with [email protected] the bigInt parser is enabled by default. Moving forward we intend to add parser plugins for any stage-3 feature which is supported by any released version of node.js (shortly after the new releases). That means [email protected] also supports class properties (public and private).

This new version of nyc version is currently released to npm under next tag for testing.

@bartekleon
Copy link

thanks, @coreyfarrell for info. Holding thumbs for you, good luck <3

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

No branches or pull requests

6 participants