100% line coverage when 50% at most with ESM

[jdalton]

I'm filing at the request of @bcoe. The bug may actually be in v8-to-istanbul.

To repro npm i -g c8 esm nyc

Then create a file foo.js:

const { log } = console

function a() {
  if (true) {
    log("hi")
  } else {
    log("false")
  }
}

function b() {
  if (true) {
    log("hi")
  } else {
    log("false")
  }
}

a()

and run c8 node foo.js which will report:

hi
----------|----------|----------|----------|----------|-------------------|
File      |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
----------|----------|----------|----------|----------|-------------------|
All files |       55 |       75 |        0 |       55 |                   |
 foo.js   |       55 |       75 |        0 |       55 |... 13,14,15,16,17 |
----------|----------|----------|----------|----------|-------------------|

now change the line const { log } = console to import { log } from "console"
and run c8 node -r esm foo.js which will report:

hi
----------|----------|----------|----------|----------|-------------------|
File      |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
----------|----------|----------|----------|----------|-------------------|
All files |      100 |      100 |      100 |      100 |                   |
 foo.js   |      100 |      100 |      100 |      100 |                   |
----------|----------|----------|----------|----------|-------------------|

If I run nyc node -r esm foo.js instead it will report:

hi
----------|----------|----------|----------|----------|-------------------|
File      |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
----------|----------|----------|----------|----------|-------------------|
All files |    42.86 |       25 |       50 |    42.86 |                   |
 foo.js   |    42.86 |       25 |       50 |    42.86 |        7,12,13,15 |
----------|----------|----------|----------|----------|-------------------|

and with report --reporter=html is

[bcoe]

@danieldiekmeier you've hit the nail on the head, the problem is that we're not taking into account the wrapper that's injected before executing ESM code. A similar problem to the first issue pointed out here:

nodejs/node#22919

would love help digging into this; for starters we could allow he header length to be configured in c8 ... this feels like a fragile long-term solution though, and I was unable to find common ground in the Node.js internal modules (the prefix injected into the code seems to vary).

[danieldiekmeier]

Maybe sourcemaps could help? I'm not too familiar with how istanbul/nyc do it, but I think they can work backwards from sourcemaps to show the coverage of your original source code?

[jdalton]

If sourcemaps are the answer I can provide inline sourcemaps or if some other c8 specific metadata is needed I can do that too.

[matthewmueller]

Quick question: is this a bug in the V8 implementation or in userland?

[bcoe]

@matthewmueller is your issue also ESM related? in this case it's a little bit of both:

• source maps aren't yet well supported in V8 and Node.js.
• but, we should be able to work with @jdalton to add them to ESM potentially, in such a way that c8 can interpret them.

[matthewmueller]

Ah, I hadn't realized this issue was ESM-specific, but that's still a very interesting summary of the current state of affairs. Thanks!

[GMartigny]

I updated the minimal reproduction repo for this issue with the last version of c8 and esm, but still no luck.
@jdalton does it's still in the work ? Can we help on that ?

[bcoe]

@jdalton, @GMartigny, and company, with recent work in Node.js, related to source-maps, as of Node 13, we can now support source-maps that are added during in-memory transpilation, see:

#152

I would love to add a similar test for ESM, and close this ticket.

[j03m]

Grabbing this will try to tackle

[JohnHardy]

Did you have any luck with this?

[StreetStrider]

I'm using esm and I've tried switch to c8 because I got some issues with nyc + esm previously. Everythings seems to work OK, however, today I've noticed that in one of esm + c8 projects I got 100% coverage, even when I add brand new code which is cannot be covered from start.

[bcoe]

@StreetStrider @JohnHardy we would still need to do a bit of work to make ESM and c8 source map support work together appropriately I believe.

[chrisveness]

In Node.js v13.2.0+ ES-modules is available without the --experimental-modules flag, and applications will run without the esm package: I have found that c8 works well using the native Node.js loader if v13.2.0+ can be used.

[j03m]

Apologies, I have been working on other issues :(

[Real-John-Cheung]

hmm any updates on this?
I got very confusing result like this

And I think it might be caused by the same problem (In the above case I guess that the uncover branches are actually from the 'if's )...
I wonder if there is any short-term trick that I can do to solve this temporarily (guessing where the actual problem is is hard)...
On macOS Catalina with Node version 14.15.4, c8 version 7.7.1

[Swivelgames]

Any update on this?

I wouldn't press so hard, but this bug effectively renders c8 useless. Code coverage for ESM modules at this point is bit completely broken. The metrics are effectively useless given how off the numbers are that its reporting for ESM modules. More and more projects are moving to ESM, especially given that it's been available since Node 12, and Node 20 is expected to be released in a month.

Any traction on this at all would be superb! 🤞

[Swivelgames]

@danieldiekmeier Given that this is still an issue even with native ES Modules, has discussion, history, and watchers, and its still open, I don't see how it would be useful to create multiple issues when this one already exists.

However, if you're not experiencing issues with it, I would recommend you Unsubscribe from the topic so that you're not bothered by future notifications 🙂

[araujoarthur0]

Are there any updates to this?

[nwoolls]

From what I can tell both c8 and Nyc are broken if the source under test makes use of ESM. Is that correct? Is there any form of coverage collection that works when using ESM?

[StreetStrider]

@nwoolls I've been using nyc + ts-mocha and it reports proper numbers for my project.

[nwoolls]

@StreetStrider thanks very much for that info & direction. I pushed further down that path and got it working. The issue I had run into with that direction ended up being with Chai (chaijs/chai#1568).

[StreetStrider]

@nwoolls I had something similar. I think this is due to chai v5 ESM migration. I decided to stick with v4, maybe for an extended time. Chai CJS has been serving me for years without problems, so why should I push to the new version when v4 does the job. Besides, it is a testing infrastructure, so no risks or vulnerabilities for the user.

[nwoolls]

@StreetStrider that's the direction I took as well 👍🏻.