refactor(core): use require.resolve instead of Module internals

[Globegitter]

Types of changes

• Bug fix (a non-breaking change which fixes an issue)
• New feature (a non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Description

Is there any reason to not use require.resolve but rather a private module function? We use Bazel as our build tool to call nuxt and it implements a custom module resolution. Using this private method currently does not work, but using require.resolve does. Would be great to get this in if there is no specific reason to use this private method.

Checklist:

• My change requires a change to the documentation.
• I have updated the documentation accordingly. (PR: #)
• I have added tests to cover my changes (if not applicable, please state why)
• All new and existing tests are passing.

[pi0]

Historically coming from #1136. It seems require.resolve also accepts second paths param so looks a valid fix. We also dropped this so don't need Module anymore.

[Globegitter]

@pi0 I did some local testing and it seems the returned errors are different from require.resolve, so this should hopefully fix it.

[clarkdo]

@pi0 It looks like that paths in _resolveFilename and resolve are different.

paths in _resolveFilename is parent, in resolve is module location.

https://github.com/nodejs/node/blob/master/lib/internal/modules/cjs/helpers.js#L21

[Globegitter]

Good point @clarkdo, but I guess the main question is, does it still achieve the same thing, or is it maybe even the more correct thing to do?

[clarkdo]

@Globegitter You're right, options.paths in resolve will be converted to fake parent in _resolveFilename, but I'm not sure why the error thrown is not MODULE_NOT_FOUND, I need more investigation for avoiding unexpected behaviour.

[Globegitter]

@clarkdo Yeah that is strange indeed - but at least all the tests are passing now, so that is something. In my local testing error.code was not set in most of the cases, not sure why that is the case. Anyway, I can understand that you need more time to investiage.

[clarkdo]

@Globegitter Error is from https://github.com/facebook/jest/blob/master/packages/jest-runtime/src/index.ts#L650-L654 which doesn't set code, so it only happens in jest.

[codecov-io]

Codecov Report

Merging #5796 into dev will decrease coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##              dev    #5796      +/-   ##
==========================================
- Coverage   95.69%   95.66%   -0.04%     
==========================================
  Files          82       82              
  Lines        2674     2676       +2     
  Branches      685      686       +1     
==========================================
+ Hits         2559     2560       +1     
- Misses         97       98       +1     
  Partials       18       18

Flag	Coverage Δ
#e2e	100% <ø> (ø)	⬆️
#fixtures	50.56% <100%> (+0.03%)	⬆️
#unit	92.63% <100%> (-0.04%)	⬇️

Impacted Files	Coverage Δ
packages/core/src/resolver.js	100% <100%> (ø)	⬆️
packages/vue-renderer/src/renderer.js	93.49% <0%> (-0.82%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e75d65b...05de98c. Read the comment docs.