X Tutup
The Wayback Machine - https://web.archive.org/web/20211022101552/https://github.com/nodejs/node/issues/32103
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

"Cannot find module" when main file not index.js with experimental-specifier-resolution=node #32103

Closed
dandv opened this issue Mar 5, 2020 · 12 comments

Comments

@dandv
Copy link
Contributor

@dandv dandv commented Mar 5, 2020

  • Version: 13.9.0
  • Platform: Linux

What steps will reproduce the bug?

  1. Clone https://github.com/dandv/node-cant-find-module-with-main-not-index.js
  2. npm start

What is the expected behavior?

The script should display Success!, and does do so if mypackage/Lib.js is renamed to mypackage/index.js.

What do you see instead?

internal/modules/esm/resolve.js:61
  let url = moduleWrapResolve(specifier, parentURL);
            ^

Error: Cannot find module /home/dandv/prg/node-cant-find-module-with-main-not-index.js/mypackage imported from /home/dandv/prg/node-cant-find-module-with-main-not-index.js/run.js
    at Loader.defaultResolve [as _resolve] (internal/modules/esm/resolve.js:61:13)
    at Loader.resolve (internal/modules/esm/loader.js:85:40)
    at Loader.getModuleJob (internal/modules/esm/loader.js:191:28)
    at ModuleWrap.<anonymous> (internal/modules/esm/module_job.js:42:40)
    at link (internal/modules/esm/module_job.js:41:36) {
  code: 'ERR_MODULE_NOT_FOUND'
}

Additional information

I'm trying to run node with -experimental-specifier-resolution=node because TypeScript can't output .mjs files and I want to use extension-less import statements. I prefer to use Lib.js instead of index.js to distinguish in my IDE between the main files of multiple packages in my monorepo that otherwise would all look like index.js.

@MylesBorins MylesBorins added the esm label Mar 6, 2020
@MylesBorins
Copy link
Member

@MylesBorins MylesBorins commented Mar 6, 2020

@ljharb
Copy link
Member

@ljharb ljharb commented Mar 6, 2020

The repro link seems correct to me; if so, this seems like a bug.

If you use explicit extensions, and don't use experimental specifier resolution, what happens?

@jkrems
Copy link
Contributor

@jkrems jkrems commented Mar 6, 2020

I'm trying to run node with -experimental-specifier-resolution=node because TypeScript can't output .mjs files and I want to use extension-less import statements.

TypeScript has only limited support for ES modules so far. But in many cases the following workaround works:

  1. Add "type": "module" to package.json.
  2. Use import './some-file.js' in your TypeScript source code. TypeScript will find .ts files if you use .js in the specifier.
  3. Run the app without the experimental resolution flag.

(That's the more verbose version of Jordan's "If you use explicit extensions". :))

@dandv
Copy link
Contributor Author

@dandv dandv commented Mar 7, 2020

If you use explicit extensions, and don't use experimental specifier resolution, what happens?

Then the script runs correctly.

@MylesBorins
Copy link
Member

@MylesBorins MylesBorins commented Mar 10, 2020

So the issue appears to be that to support that we introduced for experimental-specifier-resolution does not respect the package.json main field. I can get the example to work by changing the Lib.js file to be index.js. This is definitely a bug in the support for experimental resolution. I'm not 100% where the bug lives but this is where we should be resolving package main for experimental resolution

https://github.com/nodejs/node/blob/master/src/module_wrap.cc#L1180-L1186

This is where it seems like where we are doing the resolution itself

https://github.com/nodejs/node/blob/master/src/module_wrap.cc#L826-L862

It is possible that some order of operations bug is not even checking for the package.main... and tbh the work we've done around exports is going to confuse this a bit too. I'll try and find some time to dig in but this is going to have to be lower priority for me personally, so if anyone else wants to pick this up please go ahead!

as an aside, thanks so much for these amazing bug reports @dandv

@lingsamuel
Copy link
Contributor

@lingsamuel lingsamuel commented Apr 1, 2020

I figured out the exact reason.
Note: Related C++ code is rewritten using JS in this commit, new related code location link:
https://github.com/nodejs/node/blob/master/lib/internal/modules/esm/resolve.js#L594
More comments can be found in the historical C++ file in the commit link above.

function moduleResolve(specifier /* string */, base /* URL */) { /* -> URL */
  // Order swapped from spec for minor perf gain.
  // Ok since relative URLs cannot parse as URLs.
  let resolved;
  if (shouldBeTreatedAsRelativeOrAbsolutePath(specifier)) {
    resolved = new URL(specifier, base);
  } else {
    try {
      resolved = new URL(specifier);
    } catch {
      return packageResolve(specifier, base);
    }
  }
  return finalizeResolution(resolved, base);
}

The parameter specifier is ./mypackage here, so it passes the relative path check and returns as is directly. So the package main resolve related code won't run.

I am working on this, maybe I will submit a PR in about a few days.

@csvan
Copy link

@csvan csvan commented May 5, 2020

EDIT: This is working as intended, I misunderstood the spec. See https://nodejs.org/api/esm.html#esm_import_specifiers

Leaving the below for history only.

Seeing the same thing on Node 14.1, OSX. In my package.json I have

"type": "module",

I then have one file, server.js, which in turn imports another file, serverModule.js:

server.js

import startServer from './serverModule';

startServer({});

serverModule.js

import args from './get-args';
// ......
export default startServer;

Now, I notice that adding .js to any import in the chain solves it for that particular file. For example, this solves the import of serverModule.js in server.js:

import startServer from './serverModule.js';

But then in turn it complains about import args from './get-args'; in that file, and so on. It seems like file endings are required, but that runs contrary to the description here: https://medium.com/@nodejs/announcing-core-node-js-support-for-ecmascript-modules-c5d6dc29b663

Files ending in .js, or extensionless files, when the nearest parent package.json file contains a top-level field “type” with a value of “module”.

@pujakjha
Copy link

@pujakjha pujakjha commented Jun 17, 2020

Thanks! This solution worked for me.

@ryzokuken
Copy link
Member

@ryzokuken ryzokuken commented Jun 29, 2020

This seems resolved, closing. Feel free to reopen if something needs to be discussed further, thank you.

@ryzokuken ryzokuken closed this Jun 29, 2020
@guybedford
Copy link
Contributor

@guybedford guybedford commented Jun 29, 2020

@ryzokuken I believe this actually still remains an implementation bug and I believe my suggestion in #32612 (comment) might be related to the fix here.

That said, we might end up deprecating this flag before the fix at this rate...

@ryzokuken ryzokuken reopened this Jun 29, 2020
@aduh95 aduh95 closed this in 4e17ffc Jun 13, 2021
danielleadams added a commit that referenced this issue Jun 14, 2021
PR-URL: #38979
Fixes: #32103
Fixes: #38739
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
danielleadams added a commit that referenced this issue Jun 17, 2021
PR-URL: #38979
Fixes: #32103
Fixes: #38739
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
richardlau added a commit that referenced this issue Jul 19, 2021
PR-URL: #38979
Fixes: #32103
Fixes: #38739
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
richardlau added a commit that referenced this issue Jul 20, 2021
PR-URL: #38979
Fixes: #32103
Fixes: #38739
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
@scanner77
Copy link

@scanner77 scanner77 commented Jul 23, 2021

Where's the solution after this much of comments and showing expertise? No solution is found in github forum. Yall show how much yall know. I come here often to github to see some solutions to the problem. There's always talking. Please provide the solutions in simple words my experts. Some people are naive out there. Not everybody pros like yall

@aduh95
Copy link
Contributor

@aduh95 aduh95 commented Jul 23, 2021

The solution is to upgrade to Node.js v16.4.0 or later, or wait for v14.17.4 once it is released next week. I'll try to backport it to v12.x as well.
EDIT: The patch has landed on v12.22.4, v14.17.4 and v16.4.0.

aduh95 added a commit to aduh95/node that referenced this issue Jul 23, 2021
PR-URL: nodejs#38979
Fixes: nodejs#32103
Fixes: nodejs#38739
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
aduh95 added a commit to aduh95/node that referenced this issue Jul 28, 2021
PR-URL: nodejs#38979
Fixes: nodejs#32103
Fixes: nodejs#38739
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
richardlau added a commit that referenced this issue Jul 28, 2021
PR-URL: #38979
Backport-PR-URL: #39497
Fixes: #32103
Fixes: #38739
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
richardlau added a commit that referenced this issue Jul 28, 2021
PR-URL: #38979
Backport-PR-URL: #39497
Fixes: #32103
Fixes: #38739
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
foxxyz added a commit to foxxyz/node that referenced this issue Oct 18, 2021
PR-URL: nodejs#38979
Fixes: nodejs#32103
Fixes: nodejs#38739
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
12 participants
X Tutup