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

test_runner: using module mocking confuses all future esm imports to use cjs path resolution #53807

Closed
ChALkeR opened this issue Jul 11, 2024 · 3 comments · Fixed by #53846
Closed
Labels
loaders Issues and PRs related to ES module loaders test_runner Issues and PRs related to the test runner subsystem.

Comments

@ChALkeR
Copy link
Member

ChALkeR commented Jul 11, 2024

Version

v22.4.1

Platform

Darwin Nikitas-Air.lan 23.5.0 Darwin Kernel Version 23.5.0: Wed May  1 20:14:59 PDT 2024; root:xnu-10063.121.3~5/RELEASE_ARM64_T8122 arm64

Subsystem

test_runner

What steps will reproduce the bug?

// Run with and without --experimental-test-module-mocks

import { writeFile } from 'node:fs/promises'
import { mock } from 'node:test'

await writeFile('./test-2.js', "export default 42")

try {
  if (mock.module) mock.module('Whatever, this is not significant', { namedExports: {} })
} catch {}

const { default: x } = await import('./test-2')
console.log(`Found x: ${x}`) // prints 42

Or, alternatively:

// Run with and without --experimental-test-module-mocks

import { writeFile } from 'node:fs/promises'
import { mock } from 'node:test'

await writeFile('./test-1.mjs', "import x from './test-2'; console.log(`Found x: ${x}`)")
await writeFile('./test-2.js', "export default 42")

try {
  if (mock.module) mock.module('Whatever, this is not significant', { namedExports: {} })
} catch {}

await import('./test-1.mjs') // prints 42

Or, alternatively (dir imports):

// Run with and without --experimental-test-module-mocks

import { writeFile, mkdir } from 'node:fs/promises'
import { mock } from 'node:test'

await mkdir('./test-3', { recursive: true })
await writeFile('./test-3/index.js', "export default 43")

try {
  if (mock.module) mock.module('Whatever, this is not significant', { namedExports: {} })
} catch {}

const { default: x } = await import('./test-3')
console.log(`Found x: ${x}`) // prints 43

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

Always, when run with --experimental-test-module-mocks

What is the expected behavior? Why is that the expected behavior?

Error [ERR_MODULE_NOT_FOUND]: Cannot find module './test-2' imported from ./test-1.mjs
Did you mean to import "./test-2.js"?

What do you see instead?

Found x: 42

Additional information

Likely from #52848, cc @cjihrig, @nodejs/test_runner

Also: long time no see, hi all!

@ChALkeR ChALkeR changed the title test_runner: using module mocking confuses all future esm imports to load .js suffix by default test_runner: using module mocking confuses all future esm imports to use cjs path resolution Jul 11, 2024
@benjamingr
Copy link
Member

cc @GeoffreyBooth

@benjamingr
Copy link
Member

Also: long time no see, hi all!

Also Hi! Kind reminder you're welcome to come back :P!

@RedYetiDev RedYetiDev added test_runner Issues and PRs related to the test runner subsystem. loaders Issues and PRs related to ES module loaders labels Jul 11, 2024
@syi0808
Copy link
Contributor

syi0808 commented Jul 14, 2024

I'd like to solve this in two days.

@aduh95 aduh95 closed this as completed in 5bfebfe Aug 6, 2024
targos pushed a commit that referenced this issue Aug 14, 2024
PR-URL: #53846
Fixes: #53807
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
loaders Issues and PRs related to ES module loaders test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants