Esm: Issues with @babel/register and nyc

Created on 6 Dec 2017  路  14Comments  路  Source: standard-things/esm

I'm not sure exactly how to characterize the issue or its cause, but I made a repro repo so you can see what I mean.

https://github.com/tilgovi/esm-babel-nyc-test

Running the index.mjs file with node -r @std/esm -r @babel/register -r @babel/polyfill works just fine. Running the same under nyc using its --require / -i flag does not.

bug

Most helpful comment

@std/esm v0.19.0 is released :tada:

All 14 comments

Thanks @tilgovi!

Working on it.

Ok so there are a couple of interesting things here.

The first bit is using @babel/transform-runtime with the useESModules: true will generate code like:

var _wrapAsyncGenerator = require("@babel/runtime/helpers/builtin/es6/wrapAsyncGenerator");

Notice the builtin/es6 path. That file is ESM (it has export default in it).
So when we hit that we go, WOAH ESM being required needs the CJS compat vars option. But then we expect the result of the require to be { default: wrapAsyncGeneratorFunc } but Babel assumes the result of export default wrapAsyncGenerator is module.exports = wrapAsyncGenerator so it then fails with a type error since it wants a function and not an object.

Normally Babel handles this juggle and we can opt-in to supporting the Babel __esModule juggle with the interop option but the Babel runtime generated code does not use the Babel helper to juggle for __esModule. That should be a bug on Babel... I'll file it.

I looked into that, too, but found that when the file is .mjs, the runtime transform generates an import statement.

@babel/transform-runtime uses @babel/helper-module-imports which generates an import for ES modules: https://github.com/babel/babel/blob/master/packages/babel-helper-module-imports/src/is-module.js

Or, at least, it should.

With .mjs the builtin/es6/interopRequireDefault is

export default function _interopRequireDefault(obj) {
  return obj && obj.__esModule ? obj : {
    default: obj
  };
}

and its not used on itself:

var _interopRequireDefault = require("@babel/runtime/helpers/builtin/es6/interopRequireDefault");

var _wrapAsyncGenerator2 = _interopRequireDefault(require("@babel/runtime/helpers/builtin/es6/wrapAsyncGenerator"));

Update:

Filed a bug at https://github.com/babel/babel/issues/6983.
I'll work around as much as I can here though.

I don't think the runtime plugin should be generating a require() at all in this case, though.

I can dig, too, but I'm not sure where to look to get the babel transformed source before ESM gets to it. In the babel register cache?

Ah sorry, I was experimenting with the Babel option "modules": true. Still a valid Babel issue but not so much related to @std/esm. I'll keep digging.

Oh, I'm sorry. I understand now.

While it does generate it import statement, the files that it's importing from helpers/es6 are .js files, and the problem then occurs there where they import from one another.

Thanks, @jdalton. If you can't work around it, I understand completely and we can sit on this a few days to see if it's fixed upstream.

Also, I don't know what to make of the fact that things do work fine without nyc. So, maybe I don't understand and we just confused one another :-D.

There's still some things for me to fix on my end. Just sorting through what's @std/esm and what is Babel's issue. I have a fix on my end locally. The reason for one working and one not is because of how the hook is done depending on if its a preload node -r or a CLI load nyc -r. I'll smooth those out.

Update:

Okay! So, once I fix this bug you should be able to use just "esm": "js".

Update Update:

Patched https://github.com/standard-things/esm/commit/5249344b208763063db80f0637b70a89cb6c7655, https://github.com/standard-things/esm/commit/6827d64007d323a567d438945abe9e35ecd37360, and https://github.com/standard-things/esm/commit/d69bad3db3804b06ea171299cfdbda2712c622ad~~. Will follow-up with a scenario test.

Ok so I'm going to flip flop a bit from my previous update comment:

By Node ESM rules, ESM (so .mjs) must not use require.extensions. This means things like @babel/register should have no effect on it. In my previous comment I said you could just use "esm":"js", but that was because I was going to bypass the ESM rule for CLI use and treat it as "cjs": { "extensions": true }. Then the index.mjs would "just work". But thinking on it more, you should really just use "cjs": { "extensions": true } or "esm":"cjs".

What do you think @tilgovi?

Also do you have opinions on the option name?
If it's common I might make it "cjs": { "exts": true }.

You've been using extensions up until now. I don't see a strong reason to change it.

I agree that CLI use should not imply "extensions".

I'll test things out tomorrow and see if maybe there's a documentation contribution I want to make if you don't do it first.

Thanks so much for looking into this, and a special thank you for filing the babel bug. I'm following along over there, now. It looks like it will get resolved nicely.

Once that ships, I should have a workflow for authoring with @std/esm in .mjs and compiling a dual-use package with Babel transforms to support all my targets, outputting transformed .mjs and .js side-by-side.

I'm really grateful for all the work you're doing for the ecosystem right now. 馃檶

@std/esm v0.19.0 is released :tada:

Was this page helpful?
0 / 5 - 0 ratings