require.resolve with options seems have different result in some case

[fisker]

🐛 Bug Report

require.resolve with options seems had different result in some case

require.resolve('foo/node_modules/bar', {paths: [process.cwd()]});

result differently with require.resolve from Node.js

To Reproduce

Steps to reproduce the behavior:

A reproduction repo /~https://github.com/fisker/jest-require-resolve-bug , see readme.md in that repo.

Expected behavior

Should try resolve foo package in node_modules, instead of returning foo/node_modules/bar/index.js file

Link to repl or repo (highly encouraged)

reproduction repo

/~https://github.com/fisker/jest-require-resolve-bug

issue found on this PR on prettier

prettier/prettier#7508

envinfo

  System:
    OS: Windows 10 10.0.18362
    CPU: (8) x64 Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
  Binaries:
    Node: 13.5.0
    Yarn: 1.21.1
    npm: 6.13.4
  npmPackages:
    jest: 25.1.0 => 25.1.0

[SimenB]

Implementation of paths came from #6471 - it seems to me (without having dug deep into it) that we make paths behave relative meaning we find the foo directory without the path provided being relative.

/~https://github.com/facebook/jest/blob/964dc1619b8c5345a648ef79bcd87c087995255a/packages/jest-runtime/src/index.ts#L685

@jeysal would you be able to take a quick look?

[jeysal]

Yes, perhaps this evening, not sure

[jeysal]

Sooo this actually used to be Node's behavior back when I implemented it. Try it with Node <= 11 and see that it also resolves test.js. It changed in Node 12, probably with nodejs/node#23683.
I'm not sure how to deal with this, looks like we'd need to introduce a conditional on the Node version? :/

[fisker]

How about try require.resolve test on self(__filename) to detect node behavior. Will that work? I'm new here

[jeysal]

Yeah I guess feature detection would be nicer than plain version checks.
Although I guess the PR I linked was considered a 'fix' type thing to align the behavior with the docs, so maybe we just 'fix' (change) it here as well?

[SimenB]

Seeing how many issues were opened due to the changed behavior in v12, I think feature detection is the correct way. That way people avoid false negatives (and positives) across node versions. Jest's require should behave like the native require as much as possible, warts and all.

I'm fine with fixing it for node 12 and 13 though, even though that might break people

[jeysal]

I'll put it on my list, but if anyone else wants to pick this up, just leave a message here and have a go!

[SimenB]

Since we've merged the migration to resolve now (which implements paths) we might wanna move over to jest-resolve and use that?

[jeysal]

Not sure, I haven't looked into it, would that integrate with mocks etc., so all the things that jest-runtime handles?

[github-actions]

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 14 days.

[github-actions]

This issue was closed because it has been stalled for 7 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.