unexpected `ERR_PACKAGE_PATH_NOT_EXPORTED` thrown

[JounQin]

Reproduction: https://github.com/JounQin/test/tree/repro/import-meta-resolve

Branch: repro/import-meta-resolve

Steps:

yarn
node test.mjs

[wooorm]

What behavior are you looking for?

moduleResolve is an internal Node API that could be useful, but you have to do more things. You probably have to specify conditions, for example.

If you do:

console.log(moduleResolve('@isnotdefined/stylelint-plugin', new URL(import.meta.url), new Set(['node', 'import'])))

It works.

[wooorm]

It looks like the docs do currently say that node, import is the default in moduleResolve.
Looking through the initial code here added 3 years ago, that seems to be incorrect: that is the default for resolve, but not for moduleResolve.

I can remove that in the docs. Is that okay?

[JounQin]

I think the default options should be same? Or why not?

I use moduleResolve in many packages, and the default conditions make more sense IMO.

[wooorm]

why not

It’s an internal API that allows you to specify conditions and preserveSymlinks and you get more errors.

I think it’s the other way around: use resolve. If that is not enough (because you need conditions/preserveSymlinks/etc), then you can use moduleResolve. To pass new Set(['require', 'browser') for example.

I think the default options should be same?

I’m not really against this idea, but it’s also maybe a breaking change? So changing the docs to match the code (default: -> example:) seems safer.

[JounQin]

It says moduleResolve does more things than resolve that's why it's slower, so I'd prefer to use moduleResolve over resolve.

(It seems I misread lower as slower 😅, but I looked through the source codes, it seems resolve catches some moduleResolve error silently)

And also considering the most usage case should use moduleResolve same as resolve, so I believe a breaking change to align the defaults is more worth.

[wooorm]

And also considering the most usage case should use moduleResolve

99% of users should not use moduleResolve.
It exposes internals that could be useful if you need them. But you probably don’t?

Perhaps this is not explained well in the docs.
This package is about import.meta.resolve, which is the resolve function.

Even if we change the conditions default to match the docs, you should likely use resolve, and the docs should discourage moduleResolve.

[JounQin]

Even if we change the conditions default to match the docs, you should likely use resolve, and the docs should discourage moduleResolve.

We're using moduleResolve because we want to catch all errors manually, and fallback to other files for compatibility:

https://github.com/conventional-changelog/commitlint/blob/c085cff2a43003ca40331c12fddf47ee10a9bfd2/%40commitlint/resolve-extends/src/index.ts#L19-L71

When I change to use resolve, there are test cases failing unexpectedly.

[wooorm]

When I change to use resolve, there are test cases failing unexpectedly.

They are different indeed. That does not necessarily mean you should use moduleResolve.

resolve does not throw an error when resolving something that does not exist.
This was changed in Node.js recently, because browser (have to) work like that too. new URL() works like that too: new URL('/does-not-exist', 'https://example.com') does not check if there is a https://example.com/does-not-exist exists. So import.meta.resolve('/does-not-exist', 'https://example.com') and resolve('/does-not-exist', 'https://example.com') work like that too.

To use resolve to find only files that exist, you must then check if files exist.
Similar to what you do on L36: https://github.com/conventional-changelog/commitlint/blob/c085cff2a43003ca40331c12fddf47ee10a9bfd2/%40commitlint/resolve-extends/src/index.ts#L36.