-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
Node 12: require.resolve(relativePath, { paths: [ '.' ] }) throws without ./
#27583
Node 12: require.resolve(relativePath, { paths: [ '.' ] }) throws without ./
#27583
Comments
This was reported to me by @devinivy in a separate channel. The change was intentional. I believe I overlooked the case of relative paths though, which would make this a bug. EDIT: After reviewing this report I think it's working as expected. |
The node v12 Create a module.exports = function something () {} Then create this reproduction script const path = require('path')
function log (request, paths) {
try {
console.log(require.resolve(request, { paths }))
} catch (err) {
console.log('Failed:', request, paths)
}
}
log('lib/index.js', [ '.' ])
log('lib', [ '.' ])
log('lib/index.js', [ process.cwd() ])
log('lib', [ process.cwd() ])
log('index.js', [ 'lib' ])
log('index.js', [ './lib' ])
log('index.js', [ path.resolve('./lib') ]) Each of these repro invocations fail (with "cannot find module") in node v12 yet pass in node v10. Node v10 output on my laptop:
Node v12 output:
|
I believe there is a bug here, but it is also my impression that the docs haven't changed because in node v12 the intent was to align All of those examples you provide above include requests that do not begin with './' or '/' or '../', so by the module resolution algorithm they're actually being looked for inside |
The idea behind this feature is that for each item in |
In which case I think I'll step away from using
Currently, I use const modulePath = require.resolve(pluginNameOrPath, {
paths: [
'.',
path.resolve(__dirname, '..', 'plugins')
]
}) If
In which case I'd expect at least one of the following examples to work in node v12, which they don't (notice i added the
|
Yes, |
I agree, that's exactly the bug we're dealing with here. |
Not really. Say the input request is I'm testing your #27598 patch locally. I created two projects "one" and "two". In "one" i installed lodash, project "two" is empty. Directory structure looks like this:
For a moment I was worried because, after a cd to the "two" directory this command failed:
I was confused because lodash definitely should be found in that path! But it was because I supplied an invalid
So in this case, could we change the error message from "module not found" to "paths value must be an array"? That would help the developer more quickly understand and fix the issue. |
This commit adds support for relative paths in require.resolve()'s paths option. PR-URL: nodejs#27598 Fixes: nodejs#27583 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
This commit adds input validation to require.resolve()'s paths option. Prior to this change, passing in a non-array value lead to a misleading 'module not found' error. Refs: nodejs#27583
This commit adds input validation to require.resolve()'s paths option. Prior to this change, passing in a non-array value lead to a misleading 'module not found' error. Refs: nodejs#27583 PR-URL: nodejs#27613 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This commit adds input validation to require.resolve()'s paths option. Prior to this change, passing in a non-array value lead to a misleading 'module not found' error. Refs: #27583 PR-URL: #27613 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Node 12 has corrected how the require.resolve function applied un-documented behavior, so that it is needed to make relative search paths explicit (./). The new approach should be backwards-compatible. Related: nodejs/node#27583
Node 12 has corrected how the require.resolve function applied un-documented behavior, so that it is needed to make relative search paths explicit (./). Related: nodejs/node#27583
I have a module
lib/something.js
which looks like this:In the node v10 REPL, this
require.resolve
invocation works fine:In node v12, it no longer works.
In node v12, you need to prepend
./
to the module ID like this:This is an issue for my users because the path passed to
require.resolve
comes from command-line input, e.g.Is this the expected new behaviour of
require.resolve
or a bug? Do I need to update my tools to prepend./
?The text was updated successfully, but these errors were encountered: