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

NODE_PATH deprecation (TC voted negative) #1627

Closed
silverwind opened this issue May 5, 2015 · 30 comments · May be fixed by purplebacon/node#4
Closed

NODE_PATH deprecation (TC voted negative) #1627

silverwind opened this issue May 5, 2015 · 30 comments · May be fixed by purplebacon/node#4
Labels
discuss Issues opened for discussions and feedbacks. module Issues and PRs related to the module subsystem.

Comments

@silverwind
Copy link
Contributor

This came up a few times now, and has been suggested again in #1452 (comment)

What is the recommended alternative to NODE_PATH should we choose to remove it? There is probably a lot of misuse with NODE_PATH around, but is it really so bad that we want to drop it completely? How should we handle warnings?

@silverwind silverwind added the module Issues and PRs related to the module subsystem. label May 5, 2015
@domenic
Copy link
Contributor

domenic commented May 5, 2015

It wasn't just suggested, it was decided, a few TC meetings ago.

@silverwind
Copy link
Contributor Author

Ack, I haven't been following that closely on this one. That leaves the question on alternatives open. Do we want users to only use local modules? Any other alternatives?

@domenic
Copy link
Contributor

domenic commented May 5, 2015

I think there are approximately 30 npm modules that duplicate this functionality. I see at least three at https://www.npmjs.com/search?q=require

@domenic
Copy link
Contributor

domenic commented May 5, 2015

And in general yes, you should be using either local modules or putting things in node_modules if you want to require them by name.

@rvagg
Copy link
Member

rvagg commented May 5, 2015

npm link is the appropriate replacement, although even that's suboptimal - basically just use local modules

@rlidwka
Copy link
Contributor

rlidwka commented May 7, 2015

Suppose we have a bunch of private modules all stored in one CVS repository.

With NODE_PATH the file structure looks like this:

application/
  bin/
    start.js
  node_modules/
    commander/
    express/
    /* other public modules */
  private_modules/
    CVS/
    companyname-mod/
    companyname-stuff/
    companyname-web/
    /* other private modules with prefix */

... and an application is started as NODE_PATH=./private_modules bin/start.js.

Now how is one supposed to run a project this way without using NODE_PATH?

Making symlinks from private_modules/* to node_modules/* isn't a good idea because somebody could add a new module to the repo, and it should be discovered automatically on cvs up.

Funny structures like putting everything to node_modules folder to abuse node.js module lookup system doesn't work well either.

NODE_PATH is the right tool for the job here, and I'm sure it has other uses as well.

@bnoordhuis
Copy link
Member

I've changed my mind about NODE_PATH. It's been semi-deprecated for ages but we have been stupid enough to leave it in the documentation without so much as a deprecation notice.

As a result, it's in active use and people do useful things with it. See for example this recent reddit topic about scripting.

Breaking such use cases is not nice. I think NODE_PATH must stay.

@chrisdickinson
Copy link
Contributor

As a result, it's in active use and people do useful things with it. See for example this recent reddit topic about scripting.

I'm loathe to admit it, but yeah, I think it's going to be around for a long time to come, especially given the google search results for the term.

@Fishrock123
Copy link
Contributor

I'd rather just remove the "require('.') with NODE_PATH" hack (#1452) and be done with it for the mean time.

@develar
Copy link

develar commented May 13, 2015

Another use case: Docker dev environment.

  1. Docker is not native for OS X/Windows, so, if you map your dev machine path to VM, node_modules will contain os-dependent files (Linux). In this case should be a way to store node modules somewhere else.
  2. If I want to build my project using gulp, I want to provide simple docker command to build from scratch (without installed node, npm and so on). In this case my docker image can contain pre-installed node_modules and, so, should be a way to specify custom path.
    For example, to install bower deps, I don't have to install bower/node/npm, I can just use docker run --rm -v $PWD/path/to/dir:/data develar/nodejs-bower. I am going to use NODE_PATH to store node_modules inside docker image instead of doing "npm install" runtime.

@Fishrock123 Fishrock123 added the discuss Issues opened for discussions and feedbacks. label May 13, 2015
@Fishrock123
Copy link
Contributor

Maybe we should bring this up again at a tc meeting and reconsider our previous sentiments?

@silverwind
Copy link
Contributor Author

Yeah, please discuss it so we know how to process come 3.0.

@silverwind
Copy link
Contributor Author

So it was decided to keep this docs-only. The current NODE_PATH section includes

These are mostly for historic reasons. You are highly encouraged to place your dependencies locally in node_modules folders. They will be loaded faster, and more reliably.

I think the reasons for discouraging NODE_PATH should be laid out. Any suggestions?

@rvagg
Copy link
Member

rvagg commented May 14, 2015

I'd like to see some npm client folk contribute to this discussion; @othiym23, @linclark, @iarna, @smikes, @timoxley, (other?) - summary for you - we want to be clearer about a recommendation against using NODE_PATH in our docs, it's not going to be deprecated (yet) but we want to discourage its use in favour of more idiomatic node_modules / npm use. Can you help at least guide the documentation around this?

@smikes
Copy link
Contributor

smikes commented May 14, 2015

I would suggest structuring the docs something like this --

NODE_PATH was originally created to support loading modules from varying paths before the current
module resolution algorithm was frozen.

NODE_PATH is still supported, but is very likely to be deprecated in a future release of node. Our experience shows that deployments that rely on NODE_PATH sometimes show surprising behavior when people are unaware that NODE_PATH must be set, or when a module's dependencies change, causing a different version (or even a different module) to be loaded as the NODE_PATH is searched.

We expect, as node moves forward with ES6 support, to implement a new module loader which can be configured to address the needs that NODE_PATH serves without falling into the same pitfalls. Once the new module loader has stabilized, NODE_PATH will very likely be deprecated.

Possibly related case on npm issue tracker - npm/npm#8222 (comment)

@silverwind
Copy link
Contributor Author

I like that very much 👍. We should probably still call it io.js until the merger is done, but other than that, would love if you can file a PR that adds this on top of the NODE_PATH section, whil removing the old paragraph.

@othiym23
Copy link
Contributor

I'd strike the third paragraph of @smikes's proposed text. Speaking for npm, it's not at all clear how things are going to shake out with System.loader at the spec level, much less the eventual Node implementation, and I think it's pretty presumptuous to assume that however that shakes out, it's going to be configurable in a way that provides behavior analogous to NODE_PATH. Node moved away from NODE_PATH in the first place for very good reasons.

@rlidwka Your example is interesting, because as stated, that example doesn't need NODE_PATH at all – packages required from companyname-mod and its siblings will find the modules in application/node_modules just by using the stock Node module resolution algorithm.

I do think the proper solution lives somewhere between npm link, local packages, and using third-party tools like @timoxley's linklocal to fix up your directory trees if you want to share packages across multiple applications or develop lots of modules in concert. I don't think there's a single obvious cowpath for npm to pave here, but if one emerges, I think it would make sense for npm to pave it / bring that functionality into the tool.

@rvagg
Based on discussions I've had with npm users, I think that NODE_PATH is used enough that removing it is likely to cause enough pain to not justify it, at least without a formal deprecation cycle. I agree with @bnoordhuis and @chrisdickinson. Its continued existence doesn't impinge on npm much one way or the other, and I discourage people from using it, but it's something that a certain subset of users are very attached to.

@rvagg
Copy link
Member

rvagg commented May 14, 2015

for the record I'm not proposing deprecation of NODE_PATH and never have, I don't use it but it also doesn't get in the way and I recognise some people have novel uses for it, see last TC meeting for details of discussion about this

@othiym23
Copy link
Contributor

@rvagg I'm just responding to your RFC upthread, not making any assumptions about your own position on the change. ;)

@isaacs
Copy link
Contributor

isaacs commented May 14, 2015

I am -1 on removing NODE_PATH support. It's not hurting anyone, and it's a perfectly reasonable way to get certain things done. Ok, so it's maybe not the most elegant way possible, and it's abused. Who cares? People also do crazy stuff with throw and RegExps and global variables.

The point of Node is not to create the most elegant system possible. The point of Node is to make it easy for people to write software. Any changes that can only make it harder while providing no tangible benefits to people writing software are bad changes.

Compare this to require.paths, which did cause demonstrable harm when used, because it involved a very spooky action at a distance. Also, require.paths was removed several years ago, when we had less than 1/10 the userbase we do now.

This is JavaScript, not Haskell. Either purity serves the goal of creating software, or we throw purity overboard.

@rvagg
Copy link
Member

rvagg commented May 14, 2015

@isaacs, @othiym23, etc. please note this issue is now inappropriately named. The TC explicitly disagreed with deprecation and this is now simply a call to adjust documentation to discourage its usage rather than suggest it's an idiomatic way to use Node.

If you disagree with even changing documentation then please let us know.

@Fishrock123 Fishrock123 changed the title NODE_PATH deprecation NODE_PATH deprecation (TC voted negative) May 14, 2015
@Fishrock123
Copy link
Contributor

I guess it may also be worthwhile discussing how this may eventually play out regarding es6 modules. (if we want differences. More of an NG topic though.)

@othiym23
Copy link
Contributor

@rvagg My comments on the documentation changes suggested by @smikes are enough, I think. I'm good otherwise.

@othiym23
Copy link
Contributor

@Fishrock123

I guess it may also be worthwhile discussing how this may eventually play out regarding es6 modules.

It's premature to do that, in the absence of a viable loader spec or even a candidate implementation of the static syntax in V8. It's also more or less completely unrelated to this issue.

@isaacs
Copy link
Contributor

isaacs commented May 14, 2015

@rvagg I'm super ok with updating the docs to discourage use of globals and environment variables, sure.

I made a rather forceful case for not deprecating NODE_PATH because I think this is a point of priorities and leadership that is important to make very clear. We should not be afraid to change stuff if it's a demonstrable improvement, but we should also not change stuff just to change stuff.

@smikes
Copy link
Contributor

smikes commented May 14, 2015

New version incorporating feedback:

NODE_PATH was originally created to support loading modules from varying paths before the current
module resolution algorithm was frozen.

NODE_PATH is still supported, but is less necessary now that the node ecosystem has settled on a convention for locating dependent modules. Sometimes deployments that rely on NODE_PATH show surprising behavior when people are unaware that NODE_PATH must be set. Sometimes a module's dependencies change, causing a different version (or even a different module) to be loaded as the NODE_PATH is searched.

@othiym23
Copy link
Contributor

@smikes 👍

@iarna
Copy link
Member

iarna commented May 15, 2015

@smikes That looks great

@isaacs
Copy link
Contributor

isaacs commented May 15, 2015

@smikes 👍

@Fishrock123
Copy link
Contributor

Closing since we aren't deprecating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Issues opened for discussions and feedbacks. module Issues and PRs related to the module subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.