Virtual modules in lib mode no longer work

[jprosevear]

Describe the bug

Upgrading to 1.5.0 or newer causes library mode with virtual modules to break with error:
Error: Only URLs with a scheme in: file, data, and node are supported by the default ESM loader. Received protocol 'virtual:'

#5416 seems to have causes this, because the following patch applied with pnpm causes the error to pass and all tests to pass:

diff --git a/dist/server.cjs b/dist/server.cjs
index 840c3cf7e77f84e513feb974e3cd37cc0e03c646..697ac6f7ceb8d82e2d3ff67213f4797d95ae9c11 100644
--- a/dist/server.cjs
+++ b/dist/server.cjs
@@ -59,12 +59,12 @@ async function isValidNodeImport(id) {
     return true;
   if (extension !== ".js")
     return false;
+  if (/\.(\w+-)?esm?(-\w+)?\.js$|\/(esm?)\//.test(id))
+    return false;
   id = id.replace("file:///", "");
   const package_ = await utils.findNearestPackageData(pathe.dirname(id));
   if (package_.type === "module")
     return true;
-  if (/\.(\w+-)?esm?(-\w+)?\.js$|\/(esm?)\//.test(id))
-    return false;
   const code = await fs.promises.readFile(id, "utf8").catch(() => "");
   return !ESM_SYNTAX_RE.test(code);
 }
diff --git a/dist/server.mjs b/dist/server.mjs
index c9ac6f94ed9e09c607124e2d0544dafc6b7f4611..afd3ee31d9a50faa92325f4a7ba0b495371951a1 100644
--- a/dist/server.mjs
+++ b/dist/server.mjs
@@ -57,12 +57,12 @@ async function isValidNodeImport(id) {
     return true;
   if (extension !== ".js")
     return false;
+  if (/\.(\w+-)?esm?(-\w+)?\.js$|\/(esm?)\//.test(id))
+    return false;
   id = id.replace("file:///", "");
   const package_ = await findNearestPackageData(dirname(id));
   if (package_.type === "module")
     return true;
-  if (/\.(\w+-)?esm?(-\w+)?\.js$|\/(esm?)\//.test(id))
-    return false;
   const code = await promises.readFile(id, "utf8").catch(() => "");
   return !ESM_SYNTAX_RE.test(code);
 }

1.2.2 does not have this issue nor does 1.4.0.

Reproduction

See - rhino-project/rhino-project/pull/106

pnpm i
pnpm test:lib

this should pass

pnpm patch-remove [email protected]
pnpm test:lib

Will fail

System Info

System:
    OS: macOS 14.4.1
    CPU: (12) arm64 Apple M2 Max
    Memory: 7.14 GB / 64.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.10.0 - ~/.nvm/versions/node/v20.10.0/bin/node
    npm: 10.2.3 - ~/.nvm/versions/node/v20.10.0/bin/npm
    pnpm: 8.14.1 - ~/Library/pnpm/pnpm
    Watchman: 2024.01.22.00 - /opt/homebrew/bin/watchman
  Browsers:
    Chrome: 124.0.6367.91
    Firefox Nightly: 116.0a1
    Safari: 17.4.1
  npmPackages:
    @vitest/coverage-istanbul: ^1.2.1 => 1.2.1
    vite: ^5.2.10 => 5.2.10
    vitest: ^1.5.2 => 1.5.2

Used Package Manager

pnpm

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[hi-ogawa]

As you might have seen the related issues on that PR:

• When upgrading from 1.4.0 to 1.5.0, an error related to CommonJS modules appears. #5555
• instanceof checks fail when using peerDeps?  #5601
• ES module import fails with v1.5.0 (but works up to v1.4.0) #5573

I think that the previous behavior was more broken for how it's special-casing the package with a path like /dist/esm/index.ts.

As suggested in the other issues, can you try server.deps.inline: [... your packages which has /esm/...]?

[sheremet-va]

I think this is also an error with Vitest.

   const code = await promises.readFile(id, "utf8").catch(() => "");
   return !ESM_SYNTAX_RE.test(code);

If it failed, then it cannot be imported, because Node.js will try to read the file anyway. Also accepting any file schemas that are not supported by node.js looks like a bug on our side.

[jprosevear]

Should it make a difference that the virtual modules are already aliased https://github.com/rhino-project/rhino-project/blob/beta/packages/core/vite.config.ts#L48? (#5601 (comment))

Based on the docs https://vitest.dev/guide/mocking.html#virtual-modules

[hi-ogawa]

@jprosevear Your repro is a little too big to quickly see through, but I'm expecting that server.deps.inline will give the previous behavior (and Vitest not requiring server.deps.inline previously was a bug). Can you try it to start with?

[hi-ogawa]

If it failed, then it cannot be imported, because Node.js will try to read the file anyway. Also accepting any file schemas that are not supported by node.js looks like a bug on our side.

isValidNodeImport is used only for node_modules files, so I don't think virtual:... case will end up here. Also virtual: specifically is forced inline before that:

vitest/packages/vite-node/src/externalize.ts

Lines 14 to 16 in becabb5

	const defaultInline = [
	/virtual:/,
	/\.[mc]?ts$/,

(though I agree that catching readFile and making it external is probably wrong, but I think this case has been unlikely so far.)

What I suspected is that OP has an ESM package, which uses import "virtual:..." inside, something like:

// op-package/package.json
{
  "type": "module",
  "exports": "./dist/esm/index.js"
}

// op-package/dist/esm/index.js
import "virtual:something";

In this case, previously Vitest inlined op-package due to /esm/ heuristics. But after we removed this heuristics, op-package is now external and let NodeJS do import "op-package" and thus NodeJS seeing virtual:... transitively.

What I couldn't get from a glance is that OP's project seems to be monorepo, so I didn't think workspace package would end up isValidNodeImport routine, so I might be missing something.

[jprosevear]

What I suspected is that OP has an ESM package, which uses import "virtual:..." inside, something like:

// op-package/package.json
{
  "type": "module",
  "exports": "./dist/esm/index.js"
}

// op-package/dist/esm/index.js
import "virtual:something";

This is essentially what is going on.

In this case, previously Vitest inlined op-package due to /esm/ heuristics. But after we removed this heuristics, op-package is now external and let NodeJS do import "op-package" and thus NodeJS seeing virtual:... transitively.

What I couldn't get from a glance is that OP's project seems to be monorepo, so I didn't think workspace package would end up isValidNodeImport routine, so I might be missing something.

It is another workspace package that is doing the virtual import.

server.deps.inline worked - thanks!

https://vitest.dev/guide/mocking.html#virtual-modules might need some tweaking?