fix(vm): improve error when module is not found

[hi-ogawa]

Description

Follow up of #5045 (review)

It turned out the "non throwing" behavior of import.meta.resolve is only for path based import such as ./bad-path.js but not for bad-package. For the first case, Vitest throws an error during readFileSync while NodeJS still throws ERR_MODULE_NOT_FOUND, so this is a potential inconsistency, but I suppose it's very unlikely that this difference would affect Vitest user in any critical way.
Nonetheless, I'll see if there is a way to align the behavior closer to NodeJS instead of surfacing "readFileSync" exception.

references

• https://nodejs.org/docs/latest-v20.x/api/esm.html#importmetaresolvespecifier
• esm: import.meta.resolve exact module not found errors should return nodejs/node#49038
• `import.meta.resolve` support vitejs/vite#14405 (not exactly related but sapphi-red put together links nicely)

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
• Ideally, include a test that fails without this PR but passes with it.
• Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

Tests

• Run the tests with pnpm test:ci.

Documentation

• If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

• Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

[netlify]

✅ Deploy Preview for fastidious-cascaron-4ded94 canceled.

Name	Link
🔨 Latest commit	315325e
🔍 Latest deploy log	https://app.netlify.com/sites/fastidious-cascaron-4ded94/deploys/65b84e6518830d00082ccaeb

[sheremet-va]

I suppose it's very unlikely that this difference would affect Vitest user in any critical way

I think this is a popular way to import package and fallback when it's not installed. Something like this:

try {
  import('name')
} catch(err) {
  if(err.code === 'ERR_MODULE_NOT_FOUND') {
    // fallback
  }
  throw err
}

[sheremet-va]

I would prefer throwing the same error that Node does. Is there any reason not to do that?

[hi-ogawa]

Sounds good to me. I made a different error so that the test shows the difference, but there's no reason to not align with Node.

[hi-ogawa]

I think this is a popular way to import package and fallback when it's not installed. Something like this:

try {
  import('name')
} catch(err) {
  if(err.code === 'ERR_MODULE_NOT_FOUND') {
    // fallback
  }
  throw err
}

Actually this pattern seems not affected. For the bare package name, import.meta.resolve("name") is still throwing ERR_MODULE_NOT_FOUND (I don't know, maybe NodeJS changes this behavior later, but at least this seems conscious decision as indicated by the test in nodejs/node#49038). What doesn't get thrown is anything with namespace (including file:// which is the result of relative case import.meta.resolve("./name")). This actually include something like node:something, so probably we should mimic the same error with code: ERR_MODULE_NOT_FOUND like you suggested.

[hi-ogawa]

I was checking with a different version of node and it looks there was another change around v20.2.0 -> v20.3.0. It doesn't look like this change is documented in https://nodejs.org/api/esm.html#importmetaresolvespecifier, but probably this is a related PR on Node:

• module: change default resolver to not throw on unknown scheme nodejs/node#47824

# I use volta https://github.com/volta-cli/volta to switch local node version
#   volta pin [email protected]
$ node --version
v20.2.0

$ pnpm -C test/vm-threads test not-found.test.ts

...

 FAIL  test/not-found.test.ts > namespace
AssertionError: expected Error: Only URLs with a scheme in: file a… { …(2) } to match object { code: 'ERR_MODULE_NOT_FOUND', …(1) }

- Expected
+ Received

- Object {
-   "code": "ERR_MODULE_NOT_FOUND",
-   "message": "Cannot find module 'non-existing-namespace:xyz'",
- }
+ [Error: Only URLs with a scheme in: file and data are supported by the default ESM loader. Received protocol 'non-existing-namespace:']

 ❯ test/not-found.test.ts:30:3
     28| 
     29| it('namespace', async () => {
     30|   await expect(() => notFound.importNamespace()).rejects.toMatchObject({
       |   ^
     31|     code: 'ERR_MODULE_NOT_FOUND',
     32|     message: 'Cannot find module \'non-existing-namespace:xyz\'',

Is it fine to keep this error for old node version?

[sheremet-va]

Is it fine to keep this error for old node version?

We don't use the default ESM loader (we don't even support loaders in VM), so I'd say it is fine to have the error we have today.