Esm: nyc/istanbul support

[dschnare]

Hey guys, might not be in your domain of things to support, but it would awesome sauce if @std/esm could be used with istanbul. Currently when using with nyc I get this error:

/sandbox/esm/node_modules/@std/esm/esm.js.gz:1
(function (exports, require, module, __filename, __dirname) { "use strict";const t=module,e=__filename;module.exports=(funct
ion(t){function e(i){if(s[i])return s[i].exports;var n=s[i]={i:i,l:!1,exports:{}};return t[i].call(n.exports,n,n.exports,e),
n.l=!0,n.exports}var s={};return e.m=t,e.c=s,e.d=function(t,s,i){e.o(t,s)||Object.defineProperty(t,s,{configurable:!1,enumer
able:!0,get:i})},e.n=function(t){var s=t&&t.__esModule?function(){return t.default}:function(){return t};return e.d(s,"a",s)
,s},e.o=function(t,e){return Object.prototype.hasOwnProperty.call(t,e)},e.p="",e(e.s=8)})([(function(t,e){t.exports=require(
"path")}),(function(t,e){t.exports=require("fs")}),(function(t,e){t.exports=require("module")}),(function(t,e,s){function i(
t,e){if(t instanceof n)return t;if("string"!=typeof t)return null;if(t.length>I)return null;if(!(e?T[X]:T[$]).test(t))return
 null;try{return new n(t,e)}catch(t){return null}}function n(t,e){if(t in

TypeError: Cannot set property 'entry' of undefined
    at Function.enable (/sandbox/esm/node_modules/@std/esm/esm.js.gz:1:123817)
    at Object.ce (/sandbox/esm/node_modules/@std/esm/esm.js.gz:1:39893)
    at Object.oe (/sandbox/esm/node_modules/@std/esm/esm.js.gz:1:39158)
    at Object.re (/sandbox/esm/node_modules/@std/esm/esm.js.gz:1:38400)
    at Object.n [as .js] (/sandbox/esm/node_modules/@std/esm/esm.js.gz:1:119562)
    at NYC._readTranspiledSource (/sandbox/esm/node_modules/nyc/index.js:160:26)
    at NYC.addFile (/sandbox/esm/node_modules/nyc/index.js:144:21)
    at /sandbox/esm/node_modules/nyc/index.js:176:11
    at /sandbox/esm/node_modules/nyc/index.js:244:5
    at Array.forEach (native)
npm ERR! Test failed.  See above for more details.

My .nycrc file looks like this:

{
  "include": [
    "src/**/*.js"
  ],
  "exclude": [
    "src/**/*.test.js"
  ],
  "require": [
    "@std/esm"
  ],
  "cache": true,
  "all": true
}

[dschnare]

If it also helps, my @std/esm settings are:

"@std/esm": {
  "esm": "all"
}

[jdalton]

Hi @dschnare!

Could you set up simple repro repo? That would help.

[dschnare]

I actually found a solution. Just set the cjs option to true did the trick. I'll set up that repo for you though for sure.

"@std/esm": {
  "esm": "all",
  "cjs": true
}

[dschnare]

@jdalton Here's a simple repo you can try out.

https://github.com/dschnare/std-esm-sandbox

[jdalton]

Thank you @dschnare!

I'm just making the issue invalid since you were able to resolve it with the supported options.
I'll let you know if I find something funky in your repro.

[dschnare]

@jdalton after messing around a little bit, I've discovered that every second (or third sometimes) run of npm test that uses nyc for code coverage reporting results with the following error:

Cannot read property 'decl' of undefined
TypeError: Cannot read property 'decl' of undefined
    at /std-esm-sandbox/esm/node_modules/nyc/node_modules/istanbul-reports/lib/html/annotator.js:83:28
    at Array.forEach (native)
    at annotateFunctions (/std-esm-sandbox/esm/node_modules/nyc/node_modules/istanbul-reports/lib/html/annotator.js:79:26)
    at Object.annotateSourceCode (/std-esm-sandbox/esm/node_modules/nyc/node_modules/istanbul-reports/lib/html/annotator.js:192:9)
    at HtmlReport.onDetail (/std-esm-sandbox/esm/node_modules/nyc/node_modules/istanbul-reports/lib/html/index.js:217:39)
    at Visitor.(anonymous function) [as onDetail] (/std-esm-sandbox/esm/node_modules/nyc/node_modules/istanbul-lib-report/lib/tree.js:34:30)
    at ReportNode.Node.visit (/std-esm-sandbox/esm/node_modules/nyc/node_modules/istanbul-lib-report/lib/tree.js:123:17)
    at /std-esm-sandbox/esm/node_modules/nyc/node_modules/istanbul-lib-report/lib/tree.js:116:23
    at Array.forEach (native)
    at visitChildren (/std-esm-sandbox/esm/node_modules/nyc/node_modules/istanbul-lib-report/lib/tree.js:115:32)

Eventually if you run npm test again it'll go away, but comes right back the next time you run npm test. I tested this by adding html to the reporter array in .nycrc and then after a failed run (i.e. with a lower test coverage than a successful run) I opened the coverage/index.html file and navigated to the file get-message.js. There the error will be listed as the source code.

[jdalton]

The decl looks familiar. You can get an unmasked stack by using the @std/esm option of "debug":true.

[dschnare]

I've narrowed it down to only occurring when you have the all nyc setting set to true. When all is set to false all is good.

However when all is set to true occasionally the coverage files written to .nyc_output have extra statement data that ultimately doesn't match up with instrumented function data.

Code in istanbul-reports/lib/html/annotator.js that has the error:

var fnStats = fileCoverage.f,
        fnMeta = fileCoverage.fnMap;
    if (!fnStats) {
        return;
    }
    Object.keys(fnStats).forEach(function (fName) {
        var count = fnStats[fName],
            meta = fnMeta[fName],
            type = count > 0 ? 'yes' : 'no',
            startCol = meta.decl.start.column,

The fileCoverage object is a merging between two JSON coverage files it seems.

Anyway, I really believe that when all is true all files are read and coverage information is generated for them and then during the test the files that are loaded via import have coverage files generated for them. Later when working with the two coverage files they don't match because one source file is pure ESM and the other is dynamically transformed. This dynamic transformation yields more functions that get instrumented.

[jdalton]

Later when working with the two coverage files they don't match because one source file is pure ESM and the other is dynamically transformed. This dynamic transformation yields more functions that get instrumented.

I wonder if nyc taps into the Node module load system as well to swap in-and-out instrumented calls. As we stabilize it'd great to get tools like that working together.

[jdalton]

Good news! Using v0.3.0 your std-esm-sandbox example appears to work 🎉

[laggingreflex]

Not working with later versions 0.6.0 - 0.7.1. It still works with 0.3.0 - 0.5.1
Git bisect tells me https://github.com/standard-things/esm/commit/d33593e57cd441b6cddf34f527084ba5ae0a7f99 is the first bad commit

[jdalton]

@laggingreflex
With the std-esm-sandbox repro using npm start works if the in-file syntax is updated to 0.6.0+:

require = require('@std/esm')(module)

The npm test works if you change the "test" script to include --require @std/esm:

"test": "NODE_ENV=testing nyc mocha --require @std/esm --opts .mocha.opts"

Mocha expands the .mocha.opts into process.argv arguments before importing files so we'll normally detect it. However, when nyc which has an .nycrc including

"require": [
  "@std/esm",
  "./script/require-all"
]

invokes the command, before the suggested change, it invokes mocha --require --opts .mocha.opts so the @std/esm required by nyc won't detect itself in the arguments. Adding --require @std/esm enables the @std/esm required by nyc to detect itself.

I've opened an issue on nyc to see if it is open to expanding its config to process.argv args like Mocha. https://github.com/istanbuljs/nyc/issues/660

[bcoe]

@jdalton thanks for taking this on \o/ I now know your motivation for passing along arguments.