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

Misleading error that module does not provide export #32137

Closed
dandv opened this issue Mar 7, 2020 · 21 comments
Closed

Misleading error that module does not provide export #32137

dandv opened this issue Mar 7, 2020 · 21 comments
Labels
doc Issues and PRs related to the documentations. esm Issues and PRs related to the ECMAScript Modules implementation.

Comments

@dandv
Copy link
Contributor

dandv commented Mar 7, 2020

What steps will reproduce the bug?

  1. git clone https://github.com/dandv/lib-does-not-provide-export-foo
  2. cd lib-does-not-provide-export-foo/use-lib
  3. node use-it.js # node v13.9.0

What is the expected behavior?

The script should run and output foo.

What do you see instead?

import { foo } from '../lib/index.js';
         ^^^
SyntaxError: The requested module '../lib/index.js' does not provide an export named 'foo'

Additional information

Within the context of a large project, this was a pretty odd mystery, when lib/index.js clearly exports foo. Feel free to have some fun figuring out it (not hard with the minimal repro repo, but imagine the confused looks in a real-world scenario). Solution below.

.
.
.
.
.
.
.
.
.
.

Solution

lib/package.json lacks "type": "module".

Proposal

A clearer error would indicate that the imported file is not a module at all. As it is, the error suggested that other symbols were exported fine, so I kept double checking for typos and what not. This happened during a conversion to TypeScript that involved adding one export. The error made me think the problem was that export; it was not - it was that I forgot to add "type": "module" to package.json.

The full title of the bug would be "Misleading error that module does not provide export when "module" is not in fact a module", but that would have been a spoiler.

@himself65
Copy link
Member

format be assigned as commonjs

format = getPackageType(parsed.href) === TYPE_MODULE ?
'module' : 'commonjs';

and loaderInstance is the commonjs loader

if (format === 'dynamic') {
if (typeof this._dynamicInstantiate !== 'function')
throw new ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK();
loaderInstance = async (url) => {
debug(`Translating dynamic ${url}`);
const { exports, execute } = await this._dynamicInstantiate(url);
return createDynamicModule([], exports, url, (reflect) => {
debug(`Loading dynamic ${url}`);
execute(reflect.exports);
}).module;
};
} else {
if (!translators.has(format))
throw new ERR_UNKNOWN_MODULE_FORMAT(format);
loaderInstance = translators.get(format);
}

translators.set('commonjs', function commonjsStrategy(url, isMain) {
debug(`Translating CJSModule ${url}`);
const pathname = internalURLModule.fileURLToPath(new URL(url));
const cached = this.cjsCache.get(url);
if (cached) {
this.cjsCache.delete(url);
return cached;
}
const module = CJSModule._cache[
isWindows ? StringPrototypeReplace(pathname, winSepRegEx, '\\') : pathname
];
if (module && module.loaded) {
const exports = module.exports;
return new ModuleWrap(url, undefined, ['default'], function() {
this.setExport('default', exports);
});
}
return new ModuleWrap(url, undefined, ['default'], function() {
debug(`Loading CJSModule ${url}`);
// We don't care about the return val of _load here because Module#load
// will handle it for us by checking the loader registry and filling the
// exports like above
CJSModule._load(pathname, undefined, isMain);
});
});

@himself65
Copy link
Member

ESMLoader directly use v8 esm import, I think we currently haven't checker if a source is esm or cjs

@MylesBorins MylesBorins added the esm Issues and PRs related to the ECMAScript Modules implementation. label Mar 9, 2020
@MylesBorins
Copy link
Contributor

There seems to be a bug here in the loader itself. This module should not be loaded as ESM, but rather as CJS. Was able to make the repro even smaller

index.mjs

import { foo } from './index.cjs';
console.log(foo());

index.cjs

export function foo() {
    return 'foo';
}

Even though this has ESM syntax in it it should fail to even import as you are attempting to do a named exports of the CJS module. pinging @jkrems and @guybedford to take a look

@himself65
Copy link
Member

I don't think it's a bug, because you not mark the file as the esm module. and it's just an incorrect error message. and can we add support that esm loader can check the js file as mjs file and log the message like Load js as commonjs error

@jkrems
Copy link
Contributor

jkrems commented Mar 10, 2020

Even though this has ESM syntax in it it should fail to even import as you are attempting to do a named exports of the CJS module.

Maybe I'm missing something but this looks to error correctly because of the lack of an export named foo, at least according to the issue description:

SyntaxError: The requested module '../lib/index.js' does not provide an export named 'foo'

@himself65
Copy link
Member

himself65 commented Mar 10, 2020

Even though this has ESM syntax in it it should fail to even import as you are attempting to do a named exports of the CJS module.

Maybe I'm missing something but this looks to error correctly because of the lack of an export named foo, at least according to the issue description:

SyntaxError: The requested module '../lib/index.js' does not provide an export named 'foo'

why correctly?

when

import { foo } from '../lib/index.js';
import {foo} from '../lib/index.js';
        ^^^
SyntaxError: The requested module '../lib/index.js' does not provide an export named 'foo'

but, when

import foo from '../lib/index.js';
``

```bash
export function foo() {
^^^^^^

SyntaxError: Unexpected token 'export'

this is two kinds of different error types, and the first error message will misleading users.

@himself65
Copy link
Member

another example

// change https://github.com/dandv/lib-does-not-provide-export-foo/blob/master/lib/index.js
exports.foo = 123

the error message still occur

import { foo } from '../lib/index.js';
         ^^^
SyntaxError: The requested module '../lib/index.js' does not provide an export named 'foo'

but I change use-lib/index.js to

import md from '../lib/index.js';
console.log(md);

thing go weird

(node:24052) ExperimentalWarning: The ESM module loader is experimental.
{ foo: 123 }

@himself65
Copy link
Member

I guess that cjs module loader make all codes export default.

return new ModuleWrap(url, undefined, ['default'], function() {

@jkrems
Copy link
Contributor

jkrems commented Mar 10, 2020

I'm not saying it's not confusing right now. The issue is just: This happens before we even try to run the code and that's when a CommonJS syntax errors would happen. It may be possible to somehow detect this specific error and try to gather some more context but it would be tricky.

To go into what you were seeing in your last comment: Exports in ES modules are very different from the "exports" object in CommonJS, confusingly. Each module export is a variable/identifier. The following CommonJS code doesn't create a new variable, it only sets a property on one:

exports.foo = 123

So afterwards the CommonJS still only has a single export - the module.exports object. That's all a CommonJS file exports, from an ES module perspective. Your later code could be rewritten as:

import { default as md } from '../lib/index.js';
console.log(md);

So your importing the default export (the module.exports object) from ../lib/index.js which is an object that has a property foo, as set above.

@himself65
Copy link
Member

I'm not saying it's not confusing right now. The issue is just: This happens before we even try to run the code and that's when a CommonJS syntax errors would happen. It may be possible to somehow detect this specific error and try to gather some more context but it would be tricky.

To go into what you were seeing in your last comment: Exports in ES modules are very different from the "exports" object in CommonJS, confusingly. Each module export is a variable/identifier. The following CommonJS code doesn't create a new variable, it only sets a property on one:

exports.foo = 123

So afterwards the CommonJS still only has a single export - the module.exports object. That's all a CommonJS file exports, from an ES module perspective. Your later code could be rewritten as:

import { default as md } from '../lib/index.js';
console.log(md);

So your importing the default export (the module.exports object) from ../lib/index.js which is an object that has a property foo, as set above.

Yeah, I get you

@guybedford guybedford added the doc Issues and PRs related to the documentations. label Mar 12, 2020
@guybedford
Copy link
Contributor

Marking as a documentation issue since this is a common error that should be well documented. //cc @GeoffreyBooth

@dfabulich
Copy link
Contributor

I think this needs more than just documentation. If you directly run node lib/index.js, you'll see a really nice little warning:

$ node index.js
(node:83643) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.
(Use `node --trace-warnings ...` to show where the warning was created)
/private/tmp/tp/lib-does-not-provide-export-foo/lib/index.js:1
export function foo() {
^^^^^^

SyntaxError: Unexpected token 'export'

I would have wanted this case to show a better SyntaxError, and maybe a warning, something like:

(node:83643) Warning: '../lib/index.js' is a CommonJS module,
but ES modules can't import named exports from CommonJS modules
SyntaxError: The requested module '../lib/index.js' does not provide an export named 'foo'

I'll note that the error is marginally better if you try to import the default export without braces, like this:

import foo from '../lib/index.js'

export function foo() {
^^^^^^

SyntaxError: Unexpected token 'export'

@dfabulich
Copy link
Contributor

I just realized that the scope of this is bigger than "ESM misinterpreted as CJS." This error applies even when the imported script is legit CJS with named exports.

dependency.cjs

module.exports.name = "foo";

dependent.mjs

import {name} from './dependency.cjs';
console.log({name});

You get this: SyntaxError: The requested module './dependency.cjs' does not provide an export named 'name' which is only "true" because ES modules aren't allowed to import CJS named exports.

The SyntaxError in this case should be much clearer:

SyntaxError: The requested module './dependency.cjs' is a CommonJS module, which can only provide a default export, not named exports, to an ES module script.

@kathy-ems
Copy link

I just realized that the scope of this is bigger than "ESM misinterpreted as CJS." This error applies even when the imported script is legit CJS with named exports.

dependency.cjs

module.exports.name = "foo";

dependent.mjs

import {name} from './dependency.cjs';
console.log({name});

You get this: SyntaxError: The requested module './dependency.cjs' does not provide an export named 'name' which is only "true" because ES modules aren't allowed to import CJS named exports.

The SyntaxError in this case should be much clearer:

SyntaxError: The requested module './dependency.cjs' is a CommonJS module, which can only provide a default export, not named exports, to an ES module script.

How do you re-write this so it works?

@dfabulich
Copy link
Contributor

dfabulich commented Sep 3, 2020

This error was considerably enhanced in Node 14.8 I believe. Now the error message says:

SyntaxError: The requested module './dependency.cjs' is expected to be of type
CommonJS, which does not support named exports. CommonJS modules can be
imported by importing the default export.
For example:
import pkg from './dependency.cjs';
const {name} = pkg;

And, indeed, that's what you'd do to fix dependent.mjs.

import pkg from './dependency.cjs';
const {name} = pkg;
console.log({name});

@ctavan
Copy link
Contributor

ctavan commented Dec 10, 2020

@dandv @dfabulich @kathy-ems Node.js 14.13.0 added support for named CJS exports, see #35249 and this change has also been backported to Node.js v12.20.0.

Can you confirm that

import { foo } from '../lib/index.js';

now also works for you if index.js is a CJS file?

@MahendraBishnoi29
Copy link

Screenshot (91)

How Do I Solve This I'm New to Node JS?

@GeoffreyBooth
Copy link
Member

How Do I Solve This I’m New to Node JS?

Use the suggested code from the error message.

Closing as I think this has been addressed.

@tcoulter
Copy link

What do we do when it's a dependency that's doing the import'ing?

I'm receiving this error and don't have control over the dependency's code. Any suggestions?

@ctavan
Copy link
Contributor

ctavan commented Jan 29, 2022

Have you tried reporting this as a bug to the dependency which is causing the issue?

@activenode
Copy link

What do we do when it's a dependency that's doing the import'ing?

I'm receiving this error and don't have control over the dependency's code. Any suggestions?

Yeah you have to provide a fix in the repo. I'm facing this currently and it's a bit frustrating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. esm Issues and PRs related to the ECMAScript Modules implementation.
Projects
None yet
Development

No branches or pull requests