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

Resolution fails on missing exports array path #44282

Open
privatenumber opened this issue Aug 18, 2022 · 6 comments
Open

Resolution fails on missing exports array path #44282

privatenumber opened this issue Aug 18, 2022 · 6 comments

Comments

@privatenumber
Copy link
Contributor

Version

v18.7.0

Platform

macOS 12.4

Subsystem

No response

What steps will reproduce the bug?

Import a package with an exports array where the first path doesn't exist. For example:

/node_modules/package/package.json:

{
	"exports": ["./missing.js", "./entry.js"]
}

/node_modules/package/entry.js:

console.log('works')

/index.mjs:

import 'package'

Reproduction using Node.js v16.14.2 on StackBlitz:
https://stackblitz.com/edit/node-52xlzc?file=index.js

How often does it reproduce? Is there a required condition?

Always. No.

What is the expected behavior?

For the resolution error to be ignored and to attempt resolution on the next path.

Based on the proposal spec, I believe that's how fallbacks should work.

What do you see instead?

The resolution fails at the first path.

$ node index.mjs
node:internal/errors:477
    ErrorCaptureStackTrace(err);
    ^

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/test-node-exports/node_modules/package/missing.js' imported from /test-node-exports/index.mjs
    at new NodeError (node:internal/errors:387:5)
    at finalizeResolution (node:internal/modules/esm/resolve:404:11)
    at moduleResolve (node:internal/modules/esm/resolve:965:10)
    at defaultResolve (node:internal/modules/esm/resolve:1173:11)
    at nextResolve (node:internal/modules/esm/loader:173:28)
    at ESMLoader.resolve (node:internal/modules/esm/loader:852:30)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:439:18)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:76:40)
    at link (node:internal/modules/esm/module_job:75:36) {
  code: 'ERR_MODULE_NOT_FOUND'
}

Additional information

Worth mentioning that the fallback is used when a condition prevents the path from even being attempted:

{
	"exports": [
		{ "some-condition": "./missing.js" },
		"./entry.js"
	]
}
@ljharb
Copy link
Member

ljharb commented Aug 18, 2022

cc @nodejs/modules

@aduh95
Copy link
Contributor

aduh95 commented Aug 18, 2022

Duplicate of #37928? Specifically, #37928 (comment) seems to apply here: ./missing.js can be resolved to a file path, but it fails when trying to load because there's nothing on that file address.

@jkrems
Copy link
Contributor

jkrems commented Aug 19, 2022

The proposal makes a difference between invalid / not supported entries (those are skipped) and failure to load a resource (which is a straight error). The reason here is that checking for resource existence could be expensive (think network requests etc.) or ambiguous (permissions, redirects, etc.).

@privatenumber
Copy link
Contributor Author

Thanks for explaining @jkrems. That reason makes sense.

Would it be worth clarifying the behavior in the docs/proposal?

Based on #37928, vitejs/vite#4439, and seeing how both Webpack and TypeScript has implemented them, a lot of people seem to interpret the fallback array as a way to add fallback file paths.

It might actually be fine for packages that are designed to be consumed by bundlers (and for their own resolvers to deviate), but it might set an incorrect precedent for Node.js packages, which may be particularly damaging because it throws.

@guybedford
Copy link
Contributor

@unional did you delete your comment here? In the case:

{
  "exports": [{ "a": { "b": "./b.js" } }, "./c.js"]
}

import('pkg') should work in the scenario when both the "a" and the "b" conditions are both not set, but if they are both set, it should either give a correct resolution or not found error.

The array fallbacks are only for syntactical validations - this allows us to add new types of resolution semantics in future that do not start with ./. For example, we could possible enable this pattern for builtin module checks:

{
  "imports": {
    "#newbuiltin": [{
      "node": "node:newbuiltin"
    }, "./newbuiltin-polyfill.js"]
  }
}

as a way of supporting new core modules with a fallback. The above might be permissable since it doesn't need to check the file system but can use an internal list. Similarly even for any other types of protocol systems in future. This is all hypothetical though.

@unional
Copy link

unional commented Jan 4, 2023

Thanks. Yes I deleted my message because I realized the code does support it correctly.

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

No branches or pull requests

6 participants