Misleading error that module does not provide export

[dandv]

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]

format be assigned as commonjs

node/lib/internal/modules/esm/get_format.js

Lines 58 to 59 in c49286b

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

and loaderInstance is the commonjs loader

node/lib/internal/modules/esm/loader.js

Lines 201 to 218 in c49286b

	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);
	}

node/lib/internal/modules/esm/translators.js

Lines 92 to 116 in c49286b

	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]

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

[MylesBorins]

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]

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

[hybrist]

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]

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]

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]

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

node/lib/internal/modules/esm/translators.js

Line 109 in 73f2dbc

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

[hybrist]

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]

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]

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

[dfabulich]

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]

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.