istanbuljs / nyc

the Istanbul command line interface
https://istanbul.js.org/
ISC License
5.6k stars 359 forks source link

Nyc on ESM Node.js 13 (no babel) #1287

Open ghost opened 4 years ago

ghost commented 4 years ago

Does nyc work with ESM and Node.js 13 without babel?

I followed the steps from https://github.com/avajs/ava/blob/master/docs/recipes/code-coverage.md

My config from package.json is

"scripts": {
    "test": "ava",
    "coverage": "nyc ava"
},
"ava": {
    "files": [
      "src/test/**/*.spec.js"
    ],
    "require": [
      "./src/test/_setup-browser-env.js"
    ],
    "verbose": true
  }

When I run the coverage script I get this:

image

I don't use babel, and imports are done with import not require. I'm on the latest versions of node.js, ava, nyc

Properko commented 4 years ago

I'm seeing the same problem with mocha. nyc reports 0 coverage for native esm. (node 12 with --experimental-modules --experimental-json-modules flags)

coreyfarrell commented 4 years ago

nyc does not yet support coverage of node.js native ESM. It will take some time as node.js has just created API's to make this possible though they are still experimental and subject to breakage. I'm working on creating an @istanbuljs/esm-loader-hook module that could be used as NODE_OPTIONS='--experimental-loader @istanbuljs/esm-loader-hook' nyc mocha, I'll post here when I have updates for that. nyc won't handle this transparently until the loader hook API's are less experimental.

FWIW it looks like the currently available ESM transform hook is available in node.js ^12.16.0 || >=13.7.0. The minimum version could increase if additional breaking changes occur.

ghost commented 4 years ago

Thank you @coreyfarrell . Looking forward.

coreyfarrell commented 4 years ago

For anyone interested please see https://github.com/istanbuljs/esm-loader-hook#readme. To be very clear this is experimental.

robertdumitrescu commented 4 years ago

Any update for when this will be added to NYC without an additional loader?

coreyfarrell commented 4 years ago

This will not be considered until after the node.js loader feature is no longer experimental.

AndyOGo commented 4 years ago

nyc -r esm tape test doesn't collect coverage for me. If I use babel-regsiter it collects coverage correctly 😕 nyc -r babel-regsiter tape test works.

Related PR: https://github.com/AndyOGo/stylelint-declaration-strict-value/pull/64

Alright downgrading esm to 3.2.20 works on my local machine, but not in Travis CI...

coreyfarrell commented 4 years ago

This issue is about node.js native ES modules, if you are using the user-space esm module or babel then you are not experiencing the same issue.

AndyOGo commented 4 years ago

@coreyfarrell Thanks for your answer. You are right, I think the relevant issue is here https://github.com/standard-things/esm/issues/852

pandres95 commented 4 years ago

@coreyfarrell thanks a lot for your answer! If there's a way to help you push that module faster, glad to be there.

coreyfarrell commented 4 years ago

@pandres95 it's a complex set of issues being worked in node.js itself, there is nothing to be done on this end. It's not possible to accelerate progress . I fully expect it will be months before I'm able to consider including @istanbuljs/esm-loader-hook by default.

pandres95 commented 4 years ago

Okay. Will wait until that happens, then.

I tried using c8. However seems to be failing a bit.

coreyfarrell commented 4 years ago

I tried using c8. However seems to be failing a bit.

I'm sure @bcoe would appreciate a report on c8 if you can provide a reproduction.

Nexucis commented 4 years ago

Hi,

I'm trying to use Nyc with Typescript on ESM, and it doesn't seem it works as well. As I'm coding in Typescript, I have to use the loader ts-node/esm instead of the loader @istanbuljs/esm-loader-hook.

( I anyway tried the loader @istanbuljs/esm-loader-hook, and it doesn't work better).

Here is my current package.json.

And with the usage of nyc:

{
  "scripts": {
    "test": "node --experimental-specifier-resolution=node --experimental-modules --loader ts-node/esm node_modules/mocha/lib/cli/cli.js src/**/*.test.ts",
    "test:code-coverage": "nyc npm run test",
  },
"devDependencies": {
    [...]
    "nyc": "^15.1.0",
 }
}

It produces this result:

  135 passing (235ms)

----------|---------|----------|---------|---------|-------------------
File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
----------|---------|----------|---------|---------|-------------------
All files |       0 |        0 |       0 |       0 |
----------|---------|----------|---------|---------|-------------------

At this point I just don't know what to do or what to try :(.

As I'm not sure the problem is the same, I can create another issue if you prefer.

robertdumitrescu commented 3 years ago

Hey it was mentioned that this would not be implemented natively till the point where ESM is stable in Node. That time has come. It is now stable. What is the ETA for this to be supported officially by Istanbul/NYC?

coreyfarrell commented 3 years ago

To my knowledge the situation has not changed, the specific issue is that ESM loader hooks need to be stable. This is a separate feature on top of ESM itself.

robertdumitrescu commented 3 years ago

Okay but realistically speaking... Do we actually expect any change within those? Why is Nyc/Istanbul not aligning now and then just refine if changes are happening within ESM? More and more projects are using the ESM syntax out of the box and nobody likes to fiddle with configs and extra packages for coverage. I don't want to sound passive-aggressive but this would make a huge difference in every ones life.

coreyfarrell commented 3 years ago

Yes, we expect the --loader feature to have breaking changes. There are active discussions in the nodejs repository about completely breaking the existing loader (this is allowed as they are experimental).

istanbul provides an experimental plugin which you can use if you want (I use it in many side projects). nyc absolutely will not be integrating this plugin until after the feature is stable.

robertdumitrescu commented 3 years ago

okay, makes sense. Thanks for the insight.

soletan commented 3 years ago

Node 15 is current and esm support is stable there ... or am I missing something?! Are their any updates?

richardsimko commented 3 years ago

Yeah I have the same question, ESM is stable and is being used in production. How do we get nyc working with it?

jefftham commented 3 years ago

I am using node native esm (use import instead of require). I have "type": "module"in package.json, the nyc does not show the code coverage. after i remote "type": "module" from package.json, it works expectedly. Keeping this setting is what i expect by using node 14.

brettz9 commented 3 years ago

c8 is working for me with "type": "module".

coreyfarrell commented 3 years ago

You can use c8 instead of nyc or follow the instructions at @istanbuljs/esm-loader-hook to get coverage from nyc on projects with "type": "module".

To repeat myself, the --loader feature is separate from basic support of ESM. This loader feature required for nyc to support ESM is experimental and expected to have breaking changes. No further action will be taken until that support is stable.

Val-istar-Guo commented 3 years ago
----------|---------|----------|---------|---------|-------------------
File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
----------|---------|----------|---------|---------|-------------------
All files |       0 |        0 |       0 |       0 |                   
 index.ts |       0 |        0 |       0 |       0 | 11-66             
----------|---------|----------|---------|---------|-------------------

I get the same issue with ts-node/esm, Is there a way to fix it?

// ava.config.js
export default {
  files: ['tests/**/*.ts'],
  extensions: { ts: 'module' },
  nonSemVerExperiments: {
    configurableModuleFormat: true,
  },
  nodeArguments: [
    '--loader=ts-node/esm',
  ],
}
nyc ava && nyc report --reporter=text-lcov

I am not sure if it is caused by this problem:

Then in your AVA test files you must import it as if it has the .js extension it like so:

import {myFunction} from './index.js';

The problem can be reproduced here

crystalfp commented 2 years ago

Hi! Could we have an update on this issue?

I'm running coverage on a server driven from outside through its API. I'm running node 18.4.0 and nyc 15.1.0. The project is typescript already compiled to js + js.map with "target": "esnext", "module": "esnext", and "type": "module", in package.json. Running coverage this way: npx nyc npm start always give 0% coverage, but (running with lcov report) list all touched files with the correct number of lines. In the sources all lines are marked statement not covered. The nyc parameters inside package.json (found from previous nyc issues) are:

  "nyc": {
    "all": true,
    "exclude-after-remap": false,
    "reporter": [
      "html",
      "text",
      "json-summary"
    ],
    "include": [
      "common/*",
      "server/*"
    ],
    "es-modules": true
  },

Thanks for your help!

crystalfp commented 2 years ago

Well, unfortunately solved by moving to c8. this way: c8 --reporter=html --temp-directory v8coverage node server.js Hope it helps.

Bessonov commented 2 years ago

@crystalfp I hadn't luck with both of them. See also https://github.com/bcoe/c8/issues/244 .

rickosborne commented 1 year ago

In case this helps some future traveler, I did get this working with the following config:

package.json (just the relevant parts):

{
  "devDependencies": {
    "@types/mocha": "10.0.1",
    "@istanbuljs/nyc-config-typescript": "1.0.2",
    "@istanbuljs/esm-loader-hook": "0.2.0",
    "mocha": "10.2.0",
    "nyc": "15.1.0",
    "ts-node": "10.9.1",
    "typescript": ">=5.0.4"
  },
  "scripts": {
    "test": "ts-node-esm ./node_modules/.bin/mocha '**/*.test.ts'",
    "test:coverage": "ts-node-esm ./node_modules/.bin/nyc mocha **/*.test.ts"
  },
  "type": "module",
  "nyc": {
    "extends": "@istanbuljs/nyc-config-typescript",
    "check-coverage": true,
    "all": false,
    "include": [
      "src/**/!(*.test.*).[tj]s?(x)"
    ],
    "exclude": [
      "src/_tests_/**/*.*"
    ],
    "reporter": [
      "html",
      "lcov",
      "text",
      "text-summary"
    ],
    "report-dir": "coverage"
  }
}

.mocharc.json:

{
    "node-option": [
        "no-warnings",
        "experimental-specifier-resolution=node",
        "loader=@istanbuljs/esm-loader-hook",
        "loader=ts-node/esm"
    ]
}

I've got mine set up in run as a Mocha task in IntelliJ with no special config, beyond just setting it to "File Patterns" and **/*.test.ts but YMMV.

I can confirm:

Also, I have nyc set to all: false but it works just as fine with all: true.

AndrewScottWCU commented 6 months ago

Now that the Chai assertion library at version 5.0 (December 2023) and above uses ES syntax, supporting this needs to be NYC's top priority even if the fix is a cludge for now until NodeJS stabilizes the relevant parts. At this point the can cannot be kicked down the road any more as we are at the point that NYC may just cease to be relevant.