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

todo: figure out how to instrument .mjs files #659

Open
bcoe opened this issue Sep 4, 2017 · 18 comments
Open

todo: figure out how to instrument .mjs files #659

bcoe opened this issue Sep 4, 2017 · 18 comments

Comments

@bcoe
Copy link
Member

bcoe commented Sep 4, 2017

@bmeck has some amazing work close to landing that introduces ESM modules to node:

nodejs/node#14369

Our strategy for hooking require statements (using require.extensions) will no longer work for .mjs files (which use the new ESM module syntax). @bmeck indicates that we might be able to pull off something similar using import()

Let's try to stay in the loop, and make sure we have an implementation ready to go for .mjs as soon as it's possible.

@bmeck
Copy link

bmeck commented Sep 5, 2017

In newer builds of v8, code coverage is built in. I would say looking at https://chromedevtools.github.io/devtools-protocol/tot/Profiler/#method-startPreciseCoverage first might be the quickest approach.

Web compatible hooks for ESM are still being looked at and don't have a spec yet.

@bmeck
Copy link

bmeck commented Sep 5, 2017

~workflow:

  1. node --inspect-brk=$DEBUG_PORT app.js
  2. connect over http://127.0.0.1:$DEBUG_PORT/json to grab URL to send messages to over ws:
  3. Profiler.enable
  4. Profiler.startPreciseCoverage
  5. Runtime.enable -> should generate a Runtime.executionContextCreated with id of 1, that is the main context.
  6. Runtime.runIfWaitingForDebugger
  7. wait for Runtime.executionContextDestroyed with id of 1.
  8. Profiler.takePreciseCoverage to get dump of coverage

@jkrems
Copy link

jkrems commented Dec 22, 2017

Btw, there's a POC for implementing it here: https://github.com/bcoe/c8

@ORESoftware
Copy link

link to ESM?

@ORESoftware
Copy link

oh I thought it was WebAssembly for Node, but I guess it's ES6 modules for Node?

@OmgImAlexis
Copy link

Relating to what @bmeck was talking about nodejs/node#22527 was just merged.

@stale stale bot added the wontfix label Jan 5, 2019
@OmgImAlexis
Copy link

Still an issue.

@stale stale bot removed wontfix labels Jan 6, 2019
@jrgleason
Copy link

I am also looking to use NYC with ESM here is a SO. Is there a way to manually import the files for coverage?

@bcoe
Copy link
Member Author

bcoe commented Mar 7, 2019

@coreyfarrell @jrgleason the ESM module implementation in Node.js completely overhauls how require works, and makes it so it's no longer possible to use some of nyc's tricks. we should be able to get --experimental-modules working in c8 however, would love help doing so (adding tests would be a good start).

@coreyfarrell
Copy link
Member

I just tested node -r @babel/register --experimental-modules ./bin.mjs, babel transformations did not happen. I'm sure they'll eventually get that working but until they do I don't think we should make any effort to get it working with nyc.

@mikeal
Copy link

mikeal commented Aug 24, 2019

Is there a resolution to this yet?

@bcoe I tried using c8 but it also doesn’t work yet :(

@Yaffle
Copy link

Yaffle commented Aug 24, 2019

c8 works perfectly for me with .js and type module

@bcoe
Copy link
Member Author

bcoe commented Aug 26, 2019

@mikeal c8 works for the subset of modules I've tested with, mind opening an issue on c8?

@mikeal
Copy link

mikeal commented Aug 26, 2019

Some quick details, I’ll come back to this later to debug a bit more.

  1. I’m not using .mjs because you really don’t need to. With —experimental-modules you can use .js for new style modules if you set { type: ‘module’ } in package.json and/or you import from filename.js rather than .js. If you’re loading these modules directly in the browser without a compiler this workflow is actually preferable.
  2. My tests are executed from a cli.js with the following lines at the top in order to always run with experimental modules.
#!/bin/sh
":" //# comment; exec /usr/bin/env node --experimental-modules "$0" "$@" && /* eslint-disable-line */

One or both of these things may be messing up c8.

@mikeal
Copy link

mikeal commented Aug 26, 2019

Ok, this seems to be working for me now. I don’t what was going on there but it just works entirely now 🤷🏽‍♀️

@jrgleason
Copy link

@mikeal You are supposed to use mjs to signify it is a moduled javascript file. Another example is cjs for commonJs file. Eventually this may be enforced the same way extensions are required now so better to do it from the start ;-)

@coreyfarrell
Copy link
Member

@jrgleason that's not true with "type": "module" in your package.json .js files are intepreted as ES modules.

That said the current status of node.js is that it doesn't support live transpilation of files loaded by the node.js native ES module loader (regardless of extension). There are a couple proposals to address this but for now using nyc with native ES modules requires that you test pre-instrumented files. As mentioned earlier the alternative is to use c8 instead of nyc.

@vassudanagunta
Copy link

Is this necessary anymore given the recommended shift to ESM now that Node's support for it is no longer experimental?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests