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

Issue upgrading to 3.0.57 - "Must use import to load ES module" #498

Closed
chadxz opened this issue Jul 9, 2018 · 8 comments
Closed

Issue upgrading to 3.0.57 - "Must use import to load ES module" #498

chadxz opened this issue Jul 9, 2018 · 8 comments
Labels

Comments

@chadxz
Copy link

chadxz commented Jul 9, 2018

Hello! My issue kind of walks the line between bug report and support request. Sorry if it is the latter, I can take to Stackoverflow if so.

I have been using esm with chadxz/awry and it's been working great for me. I use nyc and mocha for my tests, and have --require esm in my "mocha.opts" file. This configure has been working for me up to now.

After upgrading to esm@3.0.57, I am now getting the following message when I try to run my tests.

/Users/cmcelligott/src/awry/node_modules/mocha/lib/mocha.js:1
Error [ERR_REQUIRE_ESM]: Must use import to load ES module: file:///Users/cmcelligott/src/awry/test/api/ApplicationsAPI.spec.mjs

If i downgrade to esm@3.0.56 it works fine. If I upgrade to anything past 3.0.56 it breaks.

Steps to reproduce:

  1. $ git clone https://github.com/chadxz/awry
  2. $ npm i
  3. $ mocha
@jdalton
Copy link
Member

jdalton commented Jul 9, 2018

Hi @chadxz!

This is intended. Earlier versions of esm had a bug that let .mjs files be sideloaded by mocha, nyc, and others. By Node rules .mjs files cannot be loaded with the builtin require function and should throw an [ERR_REQUIRE_ESM] error. Since esm locks down .mjs files and doesn't extend them any esm options they cannot be sidedloaded through today's tools.

The easy fix would be to switch from .mjs to .js. The .js extension has all esm defaults and support enabled. The other way would be to have your test entry script be a .js file that then loads the test .mjs file with a dynamic import().

I added a note to the tips section (it is a little light though).

@jdalton jdalton closed this as completed Jul 9, 2018
@chadxz
Copy link
Author

chadxz commented Jul 9, 2018

Thank you! That got things going again. I opted to rename the files back to .js so I could continue to leverage mocha's test discovery mechanism.

@jdalton
Copy link
Member

jdalton commented Jul 9, 2018

Okay!

Ya, sorry about that 😅
The hardest thing for esm so far is keeping .mjs locked down (so not "just work"ing).

@demurgos
Copy link

demurgos commented Jul 9, 2018

Thanks for the issue. I had the same issue today with Mocha and esm stopping to work with my .mjs files after updating to the latest version.

@jdalton
Copy link
Member

jdalton commented Jul 9, 2018

My hunch is that most using esm do so for the interop enhancements with .js leaving .mjs use more limited. I was alerted to this gap by a happy user expressing their satisfaction of .mjs "just working" which was a red flag that I dropped detection somewhere.

@demurgos

I think my doc note is a little light. If you or @chadxz have suggestions for a better warning I'm up for it.
The .mjs file extension should not be the thing developers reach for if they want interop or ease of use. It's available since it's in --experimental-modules but since it's not fully baked I can't commit to any enhancements to it.

@demurgos
Copy link

demurgos commented Jul 10, 2018

I am authoring code in Typescript and emitting it to both CommonJS (.js) and ESM (.mjs) (dual build). The CommonJS output is there for interop, the ESM output is there to experiment. I do not use the esm package once my lib is published. I prefer to transpile at build time rather than runtime.

My use case for esm is for development, to be able to use nyc and mocha (same use case as described in the OP). I want to check that my code is ready for ESM once it's no longer experimental.
This currently implies either patching mocha or adding support for require("./foo.mjs"). For now I used the second solution with esm. I feel that this a valid use case and I see that I was not the only one to use esm this way.

Now, I see that require("./foo.mjs") is not yet fully specified: should it return the namespace synchronously or return a Promise? I understand that it's safer to just disallow it by default until it's sorted out.

  • Would it be possible to add an option to enable requiring mjs? Maybe something like requireMjs: "sync" | "async".
  • Would it be preferable to reach out to Mocha? They are already looking at supporting ESM and mjs but given that it's experimental and esm allowed a workaround, it was not a priority. If esm drops support for .mjs, it may be worth continuing the discussion with Mocha.

Regarding the error message, the most confusing thing was that it caused a breaking change in a patch update. At first I thought that I had a configuration issue with esm and it was not picking my files. Going forward, I think that keeping the error message similar to Node is a good thing. If the error stays the same, a possible enhancement may be to print a console.warn explaining that requiring .mjs is not supported by esm? (Or some other mechanism with a way to better control what is printed to stderr)

Maybe something like at the bottom of the README: "esm: The builtin require function cannot load .mjs files." (printed at most once)

@jdalton
Copy link
Member

jdalton commented Jul 10, 2018

@demurgos

Now, I see that require("./foo.mjs") is not yet fully specified.

esm locks .mjs to the behavior defined in the current --experimental-modules implementation. That implementation does not allow .mjs files to be loaded with require.

There's several ways to address this. As mentioned above you can always load a test.js entry file with Mocha to proxy load the test.mjs entry file through dynamic import(). You could get ESM syntax support for a test.js entry file using esm. Or you could use your fallback favorite transpiler/runtime to load the test file. It is just the test file entry after all.

Update:

Just to be clear the restriction is on .mjs side loaded with require. This means, while your test entry file may be .js (to support ESM interop by way of esm), your subsequent test files and package files can still be .mjs and imported within the test.js entry file using ESM syntax.

You can see this in action by looking at how esm uses Mocha to test its files here.

@demurgos
Copy link

demurgos commented Jul 10, 2018

Thanks for the reply.
Having an entry point importing the .mjs files is a good idea (thanks for the example).
The downside is that I can't use Mocha's test file detection mechanism so I need to change the way I organize my tests but it's doable. It would also ensure that I use Node's native ESM which is a good thing.

I'll still try to recontact Mocha to land their PR adding support for .mjs, but at least I have your approach provides an alternative.

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

No branches or pull requests

3 participants