Vitest loads same module twice from dependencies

[JohnCosta27]

Describe the bug

I found this problem to be reproducible only with Vitest, running Jest or plain node, doesn't seem to cause this issue.

I have two packages. slate and @udecode/slate. @udecode/slate is a small wrapper around slate, and @udecode/slate dependent (through peer dependencies), on slate.

The package slate has some global state (a WeakMap). If you run test cases which mix the use of @udecode/slate and slate, you will get two loads of the same modules from slate, making the state of WeakMap incorrect.

Some other notes.

• During some attempts at debugging, I wrote console.logs inside the node_modules dist files and saw that slate/dist/index.js was being loaded twice, only in Vitest. Running the same with just node works.
• We seem to load slate/dist/index.js instead of slate/dist/index.es.js which I would expect to be loaded under these conditions, I have forced vitest to load the esm version, but this causes the same issue, where you simply load the esm version twice, and state is duplicated again.
• I have tried the resolves.dedup option, and that didn't seem to work either.

Thank you for your help, Vitest has been a great addition to our codebase, and hopefully this issue can make it a little bit better.

Reproduction

This demonstrates that two different versions of the same module are correct, shown inside the test folder.

I have also provided another file called test.node.mjs which has the same test cases, but just outputs them to stdout. In this case the results are different from vitest, and what we expect.

https://stackblitz.com/edit/vitest-dev-vitest-dkrj52?file=test%2Fbasic.test.ts&initialPath=__vitest__/

System Info

System:
    OS: Linux 6.10 Garuda Linux
    CPU: (12) x64 Intel(R) Core(TM) i7-8700K CPU @ 3.70GHz
    Memory: 15.39 GB / 31.29 GB
    Container: Yes
    Shell: 3.7.1 - /bin/fish
  Binaries:
    Node: 22.8.0 - /usr/bin/node
    Yarn: 1.22.22 - /usr/bin/yarn
    npm: 10.8.3 - /usr/bin/npm
    pnpm: 9.7.1 - /usr/bin/pnpm
  npmPackages:
    @vitest/ui: ^2.1.0 => 2.1.0
    vite: ^5.4.5 => 5.4.5
    vitest: ^2.1.0 => 2.1.0

Used Package Manager

yarn

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]

Not sure what's tripping up Vitest's resolution, but it looks like slate is not properly externalized and that's causing double modules (direct import slate is processed by Vitest while indirect import of slate from @udecode/slate is loaded directly on Node). You see something like this when running with DEBUG:

$ DEBUG=vite-node:* npm test
...
  vite-node:client:native /home/hiroshi/Downloads/vitest-dev-vitest-dkrj52/node_modules/@udecode/slate/dist/index.mjs +16ms
  vite-node:server:request /home/hiroshi/Downloads/vitest-dev-vitest-dkrj52/node_modules/slate/dist/index.js +26ms
  vite-node:client:execute /home/hiroshi/Downloads/vitest-dev-vitest-dkrj52/node_modules/slate/dist/index.js +164ms
....

Potential workaround is to use test.server.deps.external: ["slate"] to avoid Vitest processing slate like this:
https://stackblitz.com/edit/vitest-dev-vitest-quwptx?file=vite.config.ts

export default defineConfig({
  test: {
    server: {
      deps: {
        external: ['slate']
      }
    }
  },
})

[12joan]

@hi-ogawa Thanks for the this! Do you know if there's a way we can fix this problem in the slate package?

[hi-ogawa]

Thanks for the this! Do you know if there's a way we can fix this problem in the slate package?

I think this could be a bug of Vitest. There's some heuristics to decide for vite-node to whether process an external package or not and probably slate is hitting some edge cases here. Let me check a bit:

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

Lines 56 to 83 in 16aa76c

	// The code from https://github.com/unjs/mlly/blob/c5bcca0cda175921344fd6de1bc0c499e73e5dac/src/syntax.ts#L51-L98
	async function isValidNodeImport(id: string) {
	const extension = extname(id)
	if (BUILTIN_EXTENSIONS.has(extension)) {
	return true
	}
	if (extension !== '.js') {
	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 fsp.readFile(id, 'utf8').catch(() => '')
	return !ESM_SYNTAX_RE.test(code)
	}

[12joan]

Could it be because Slate using the extension .es.js for its module export? I don't see that extension listed anywhere in Vitest's code.

https://github.com/ianstormtaylor/slate/blob/main/packages/slate/package.json#L7-L11

[hi-ogawa]

It looks like ESM_SYNTAX_RE is false-detecting a code inside comments.

$ node
> const ESM_SYNTAX_RE = /(?:[\s;]|^)(?:import[\s\w*,{}]*from|import\s*["'*{]|export\b\s*(?:[*{]|default|class|type|function|const|var|let|async function)|import\.meta\b)/m;
undefined
> code = `// import { thisIsComment } from "test"`
'// import { thisIsComment } from "test"'
> ESM_SYNTAX_RE.test(code)
true

This is already fixed on mlly unjs/mlly#196, so we can probably copy the same thing to vite-node.

[hi-ogawa]

It looks like mlly's regex /\/\*.+?\*\/|\/\/.*(?=[nr])/g in unjs/mlly#196 doesn't strip multi line jsdoc comment, which is used by slate https://github.com/ianstormtaylor/slate/blob/85a1e1d3f3b9c47144a5a3b299989a088b45f5b4/packages/slate/src/interfaces/scrubber.ts#L18

Not sure whether we should go with regex or maybe we can use legitimate tokenizer like https://github.com/guybedford/es-module-lexer or https://github.com/antfu/strip-literal

[12joan]

I think \/\*.*?\*\/gs should work to target multi-line comments. The ? in .*? makes it lazy so it doesn't match multiple comments at once, and the s flag at the end tells . to match newline characters.

[12joan]

Oh, but it fails completely on this case

https://github.com/unjs/mlly/pull/155/files#diff-17d68be1f96cd54b8a5473b77ebcc090f7dcdf6537f426046c9d10526ef9352eR148-R150

[hi-ogawa]

Yeah, dotall flag s should help this case, but I wasn't entirely sure how much we can trust regex in general. I asked chatgpt and also checked stackoverflow https://stackoverflow.com/questions/5989315/regex-for-match-replacing-javascript-comments-both-multiline-and-inline and they are all slightly different and I was confused.

Using es-module-lexer would be fool-proof, but perf impact is the concern.

[12joan]

I think regex is probably good enough for the purpose of detecting whether or not the code contains imports. Even if we accidentally strip out some code that looks like a comment but isn't, there's a good chance that the remaining code will still tell us whether we're using ESM or not.

If the goal was to get a definite list of imports, using a lexer would be a much better solution, but I'm not sure it's necessary here.

[JohnCosta27]

Thanks @hi-ogawa, this has been very helpful