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

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

Open
ghost opened this issue Mar 8, 2020 · 30 comments
Open

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

ghost opened this issue Mar 8, 2020 · 30 comments

Comments

@ghost
Copy link

ghost commented Mar 8, 2020

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

Properko commented Mar 10, 2020

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

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

ghost commented Mar 10, 2020

Thank you @coreyfarrell . Looking forward.

@ghost ghost changed the title Nyc + ava on ESM Node.js 13 (no babel) Nyc on ESM Node.js 13 (no babel) Mar 10, 2020
@coreyfarrell
Copy link
Member

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

@robertdumitrescu
Copy link

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

@coreyfarrell
Copy link
Member

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

@AndyOGo

This comment has been minimized.

@coreyfarrell
Copy link
Member

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

AndyOGo commented Apr 26, 2020

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

@pandres95
Copy link

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

@coreyfarrell
Copy link
Member

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

Okay. Will wait until that happens, then.

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

@coreyfarrell
Copy link
Member

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.

Silic0nS0ldier added a commit to userfrosting/gulp-uf-bundle-assets that referenced this issue May 28, 2020
@Nexucis
Copy link

Nexucis commented Oct 10, 2020

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

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

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

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

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

okay, makes sense. Thanks for the insight.

@soletan
Copy link

soletan commented Apr 16, 2021

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

@richardsimko
Copy link

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

@jefftham
Copy link

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

brettz9 commented Apr 30, 2021

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

@coreyfarrell
Copy link
Member

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

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

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

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

@Bessonov
Copy link

@crystalfp I hadn't luck with both of them. See also bcoe/c8#244 .

@rickosborne
Copy link

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:

  • When I run without coverage, all tests run. (Whether via npm run test or the Mocha task.)
  • When I run with coverage, all tests run and I get good coverage numbers. (Whether via test:coverage or via IntelliJ's "Run with Coverage".)
  • When I run with coverage via IntelliJ, I also get the nice red/green line indicators.

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

@AndrewScottWCU
Copy link

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.

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