Skip to content

resolve.sync doesn't respect NODE_PATH #39

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

Closed
shlevy opened this issue Dec 31, 2013 · 8 comments
Closed

resolve.sync doesn't respect NODE_PATH #39

shlevy opened this issue Dec 31, 2013 · 8 comments

Comments

@shlevy
Copy link

shlevy commented Dec 31, 2013

There is an inconsistency in how resolve.sync and require.resolve resolve paths:

$ NODE_PATH=/opt/grunt-0.4.2/lib/node_modules node
> require.resolve('grunt')
'/opt/grunt-0.4.2/lib/node_modules/grunt/lib/grunt.js'
> require('resolve').sync('grunt')
Error: Cannot find module 'grunt' from '.'
    at Function.module.exports [as sync] (/home/shlevy/test/node_modules/resolve/lib/sync.js:32:11)
    at repl:1:21
    at REPLServer.self.eval (repl.js:110:21)
    at Interface.<anonymous> (repl.js:239:12)
    at Interface.EventEmitter.emit (events.js:95:17)
    at Interface._onLine (readline.js:202:10)
    at Interface._line (readline.js:531:8)
    at Interface._ttyWrite (readline.js:760:14)
    at ReadStream.onkeypress (readline.js:99:10)
    at ReadStream.EventEmitter.emit (events.js:98:17)
>
@qfox
Copy link

qfox commented Sep 8, 2014

Feels like it should be closed. Right?

@shlevy
Copy link
Author

shlevy commented Sep 8, 2014

Not sure, no longer using node-resolve. Feel free to close

@carenas
Copy link

carenas commented Aug 20, 2015

still see the same inconsistency when using node-resolve 0.3.1, this affects sequelize-cli as can be seen in sequelize/cli#104

@dsilva
Copy link

dsilva commented Mar 15, 2016

This issue breaks eslint_d per mantoni/eslint_d.js#36

@dsilva
Copy link

dsilva commented Mar 15, 2016

Restoring consistency here between resolve.sync and require.resolve should be a matter of defaulting the "paths" option to require('module').globalPaths, right?

@guncha
Copy link

guncha commented Jun 5, 2017

I don't think that without this the package can make claims like:

implements the node require.resolve() algorithm

@substack, a lot of people, including me, would be happy to fix this, as long as you or someone else will review and merge the PRs.

@ljharb
Copy link
Member

ljharb commented Jun 5, 2017

#47 (comment) seems clear.

@guncha you'll note that line in the readme links to the official node resolve algorithm, which by design omits all mention of NODE_PATH. If you file an issue on node's website, and they update that page, it'll be a much stronger case to add the behavior here.

Using NODE_PATH is a really bad idea; and when node ships ES module support, it won't have any concept that enables requiring modules in a nonstandard place, so it's best to wean yourself off it asap.

The proper solution right now is to pass process.env.NODE_PATH into opts.path if you absolutely need to rely on it for some reason.

Per the above comment and the OP, I'll close this for now.

@ljharb ljharb closed this as completed Jun 5, 2017
@guncha
Copy link

guncha commented Jun 5, 2017

Alright, makes sense. I guess I read the "implements the node require.resolve() algorithm" a little too literally.

sternenseemann added a commit to sternenseemann/browserify-handbook that referenced this issue Jul 21, 2020
browserify doesn't implicitly set opts.paths to NODE_PATH anymore
unlike stated previously in this documentation. For background see:

* browserify/resolve#39 (comment)
* browserify/resolve#47
* browserify/browserify#1626
sternenseemann added a commit to sternenseemann/browserify-handbook that referenced this issue Jul 21, 2020
browserify doesn't implicitly set opts.paths to NODE_PATH anymore
unlike stated previously in this documentation. For background see:

* browserify/resolve#39 (comment)
* browserify/resolve#47
* browserify/browserify#1626
sternenseemann added a commit to sternenseemann/browserify that referenced this issue Jul 21, 2020
Due to browserify/resolve#39 browserify will
ignore NODE_PATH on the command line. As this seems to be intended
behavior, remove NODE_PATH from documentation.
sternenseemann added a commit to sternenseemann/browserify that referenced this issue Feb 3, 2021
Due to browserify/resolve#39 browserify will
ignore NODE_PATH on the command line. As this seems to be intended
behavior, remove NODE_PATH from documentation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

6 participants