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

Importing a ".js" using esm does not show error if imported file has an "export" decl #16474

Closed
giltayar opened this issue Oct 25, 2017 · 11 comments
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. stalled Issues and PRs that are stalled.

Comments

@giltayar
Copy link
Contributor

  • Version: v8.8.0
  • Platform: macOS
  • Subsystem: ES Modules

Assuming the following files:

// main.mjs
import {x} from './not-a-module.js'

// not-a-module.js
export const x;

One would expect that node --experimental-modules main.mjs would throw an error that not-a-module is not an ES module and thus does not support export. Rather, it throws this error:

SyntaxError: The requested module does not provide an export named 'x'
    at checkComplete (internal/loader/ModuleJob.js:86:27)
    at moduleJob.linked.then (internal/loader/ModuleJob.js:69:11)
    at <anonymous>

Note that running not-a-module.js as the main module does throw the expected error, i.e. node --experimental-modules not-a-module.js throws an err: SyntaxError: Unexpected token export.

Also note that node -r @std/esm does the right thing. Just sayin' :-).

@giltayar
Copy link
Contributor Author

BTW, I found this out when using @jdalton's lodash-es module, which exposes the esm modules as .js files, and couldn't figure out why importing a specific function from lodash-es wasn't working.

Hopefully, we'll get an mjs-ed version of lodash in the near future. :-)

@giltayar giltayar changed the title Importing a ".js" using es m does not show error if imported file has an "export" decl Importing a ".js" using esm does not show error if imported file has an "export" decl Oct 25, 2017
@jdalton
Copy link
Member

jdalton commented Oct 25, 2017

@giltayar

Hopefully, we'll get an mjs-ed version of lodash in the near future. :-)

Lodash will be keeping .js and giving the @std/esm route a go 😋

@mscdex mscdex added the esm Issues and PRs related to the ECMAScript Modules implementation. label Oct 25, 2017
@giltayar
Copy link
Contributor Author

@jdalton - I'm assuming you mean @std/esm and the non-default options of using js files?

I feel a great disturbance in the force. As if millions of modules cried out in terror. :-)

@bnoordhuis
Copy link
Member

One would expect that node --experimental-modules main.mjs would throw an error that not-a-module is not an ES module and thus does not support export.

Why, though? The main script is deliberately importing it as a module. I wouldn't expect Node.js to second-guess me here. In fact, I'd be rather annoyed if it did.

@giltayar
Copy link
Contributor Author

@bnoordhuis - I understand, but those are not the rules of the game as have been defined by the TSC in regards to ESM in Node. According to the rules - a file is an es module if and only if extension is mjs.

There have been lots of discussions around that, which I would not want to bring into this bug - this bug assumes the rules are as already defined.

@targos
Copy link
Member

targos commented Oct 25, 2017

It seems like a more general bug. I get this error even when the imported module has the .mjs extension.

Edit: no, sorry my test was wrong.

@guybedford
Copy link
Contributor

The way this happens is that the "commonJS shell" for "not-a-module.js" is instantiated fine to { default } which happens before the CommonJS module not-a-module.js has even been read, or evaluated. This is because CommonJS is not read or executed until the later execute phase.

The main.mjs module is then instantiated next, and sees that it doesn't have the exports it needs, which throws at that stage of the pipeline. The execute phase is then never reached.

If we were to have CommmonJS execute during the instantiation phase then we could show the right error, but that would mess with the execution timings that import 'a'; import 'b'; import 'c'; will execute in a deterministic tree execution order (for both CJS and ESM).

@targos
Copy link
Member

targos commented Oct 25, 2017

Maybe we can throw a better error from the CJS loader in that case? Like CJS modules only have a default export

@giltayar
Copy link
Contributor Author

giltayar commented Oct 25, 2017

Oh, wow. It took me a while to figure out what you were saying, @guybedford, but it gives me a glimpse of how the whole thing is working.

I believe fixing this bug is crucial for people starting to work on migrating from cjs to esm. There will be enough confusion as it is during this migration phase, and having confusing error messages wouldn't be beneficial. It took me a while to figure out what the problem was, and I am deep into trying to understand ESM in Node (for a talk I am giving).

Maybe, as an edge case, if you don't find the correct import, then execute (or, even better, just parse) the module knowing that it is a CJS module (from the extension), and throw the correct error if you figure out that it has ESM syntax inside it.

I would hate to suggest this, because it sounds like a hack, but as I said above, I believe it crucial to to living in a dual CJS-ESM world, which is probably our world for the coming years.

@jdalton
Copy link
Member

jdalton commented Oct 25, 2017

Possibly related to c8a389e#commitcomment-24187496.

@refack
Copy link
Contributor

refack commented Nov 15, 2018

Over a year with no update, so I'm going to close this.
Feel free to ping me if this should be reopened.

@refack refack closed this as completed Nov 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. stalled Issues and PRs that are stalled.
Projects
None yet
Development

No branches or pull requests

7 participants