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

Resolve NODE_PATH by default #4479

Closed
ch2ch3 opened this issue Mar 13, 2017 · 5 comments
Closed

Resolve NODE_PATH by default #4479

ch2ch3 opened this issue Mar 13, 2017 · 5 comments

Comments

@ch2ch3
Copy link

ch2ch3 commented Mar 13, 2017

Do you want to request a feature or report a bug?

Request a feature.

What is the current behavior?

If the current behavior is a bug, please provide the steps to reproduce.

Paths defined in the NODE_PATH environment variable are ignored unless manually added in the webpack config under resolve.modules.

I made a quick demo with 3 scripts in the package.json. All of these scripts will have to parse a require('lib/foo') statement.

npm run compile:browserify: ✅
npm run compile:webpack: ❌ errors out, unable to resolve 'lib/foo'
npm test: ✅

What is the expected behavior?

By default, webpack should resolve node_modules as well as directories in NODE_PATH.

If this is a feature request, what is motivation or use case for changing the behavior?

  • Node supports this (1, 2)
  • Other major libraries like browserify and mocha resolve NODE_PATH by default (see demo repo)

This feature was also added to create-react-app (see discussion, implementation) pretty recently. However, that solution only passes NODE_PATH to the generated webpack config. I'd like this to be webpack's default behaviour :)

I've seen a few issues where adding NODE_PATH to resolve.root (webpack v1) was suggested as a solution, but haven't come across any discussion about whether this should be supported by default. Apologies if I've missed it, I'd be keen to know if this has been considered before!

Please mention other relevant information such as the browser version, Node.js version, webpack version and Operating System.

Node v7.4.0
Webpack v2.2.1
macOS Sierra 10.12.3

@timse
Copy link
Member

timse commented Mar 14, 2017

thanks for the detailed description :)!

@sokra
Copy link
Member

sokra commented Mar 15, 2017

NODE_PATH should not be used. Even the node.js documentation discourage usage: https://nodejs.org/api/modules.html#modules_loading_from_the_global_folders

This is omitted by intend.

@sokra sokra closed this as completed Mar 15, 2017
@ch2ch3
Copy link
Author

ch2ch3 commented Mar 15, 2017

Hey @sokra, from my reading of that documentation it sounds like what they are discouraging is installing dependencies in non-standard locations, and not usage of NODE_PATH.

I followed the discussion from nodejs/node#1627 which states that NODE_PATH will not be deprecated, even though usage is discouraged.

Anyway, I'm happy to work around this in my own code.

@fritx
Copy link

fritx commented Oct 17, 2017

How can I force node_modules to be a specific path? like

// webpack 2.4.1
{
  resolve: {
    extensions: ['.js', '.vue'],
    modules: [
      // 'node_modules',
      path.resolve(__dirname, '../node_modules')
    ]
  }
}

I was bundling an entry outside of the pwd like pwd/../../src/, and it keeps printing errors:

ERROR in /Users/ddd/w/uigen/src/components/ui-form/ui-form.vue
Module not found: Error: Can't resolve 'vue-style-loader' in '/Users/ddd/w/uigen/src/components/ui-form'
 @ /Users/ddd/w/uigen/src/components/ui-form/ui-form.vue 4:2-412
 @ /Users/ddd/w/uigen/src/components/ui-form/index.js
 @ /Users/ddd/w/uigen/docs/bootstrap.js
 @ ./app/main.js
 @ multi webpack-hot-middleware/client?path=/__webpack_hmr&noInfo=true&reload=true ./app/main.js

but those modules were in pwd/node_modules

@fritx
Copy link

fritx commented Oct 17, 2017

Just found out and tried resolveloader and it works!
https://webpack.js.org/configuration/resolve/#resolveloader

{
  resolveLoader: {
    modules: [
      // 'node_modules',
      path.resolve(__dirname, '../node_modules')
    ]
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants