Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

node_modules lookup optimization breaks certain project structures #1177

Closed
mafintosh opened this issue Jun 13, 2011 · 6 comments
Closed

node_modules lookup optimization breaks certain project structures #1177

mafintosh opened this issue Jun 13, 2011 · 6 comments

Comments

@mafintosh
Copy link
Member

There is a problem with the node_modules lookup optimization.
The problem is it breaks the following simple project structure

~/projects/node_modules/    // a general purpose folder for all modules
~/projects/node_modules/general.js
~/projects/foo
~/projects/foo/node_modules/bar.js

now bar.js cannot do require('general'); because of the optimization.

Since the require is mostly being done a program boot time it seems like a bad trade-off to not allow the above structure because of an optimization.

@isaacs
Copy link

isaacs commented Jun 13, 2011

So then, you're proposing that this bit be removed?

Second, if the file calling `require()` is already inside a `node_modules`
hierarchy, then the top-most `node_modules` folder is treated as the
root of the search tree.

I have no problem with that. Wanna try a patch? It should be a pretty simple change to the Module._nodeModulePaths method in lib/module.js. (And the doc change, of course.)

@mafintosh
Copy link
Member Author

Yes - that's exactly what I mean.

I'll try to do the patch.

@mafintosh mafintosh reopened this Jun 13, 2011
@mafintosh
Copy link
Member Author

I've created a patch that fixes the issue. https://gist.github.com/1023916

@isaacs
Copy link

isaacs commented Jun 14, 2011

Awesome. Sorry, I forgot to mention, can you also sign the node CLA? http://nodejs.org/cla.html

Patch looks fine, otherwise :)

@mafintosh
Copy link
Member Author

Great :) I signed the CLA.

@isaacs isaacs closed this as completed in 39246f6 Jun 14, 2011
@isaacs
Copy link

isaacs commented Jun 14, 2011

Landed on 39246f6. Thanks!

lo1tuma added a commit to lo1tuma/node that referenced this issue Sep 3, 2014
The behavior of the `node_modules` lookup algorithm was
changed in nodejs#1177, but the documentation was not updated completely
to describe the new behavior.

The pseudocode of the lookup algorithm also did not metion that
`index.json` is tried to be loaded, if you require a folder.
lo1tuma added a commit to lo1tuma/node that referenced this issue Sep 3, 2014
The behavior of the `node_modules` lookup algorithm was
changed in nodejs#1177, but the documentation was not updated completely
to describe the new behavior.

The pseudocode of the lookup algorithm did not metion that
`index.json` is tried to be loaded if you require a folder.
indutny pushed a commit that referenced this issue Sep 15, 2014
The behavior of the `node_modules` lookup algorithm was
changed in #1177, but the documentation was not updated completely
to describe the new behavior.

The pseudocode of the lookup algorithm did not metion that
`index.json` is tried to be loaded if you require a folder.

Reviewed-By: Fedor Indutny <[email protected]>
mscdex pushed a commit to mscdex/node that referenced this issue Dec 25, 2014
The behavior of the `node_modules` lookup algorithm was
changed in nodejs#1177, but the documentation was not updated completely
to describe the new behavior.

The pseudocode of the lookup algorithm did not metion that
`index.json` is tried to be loaded if you require a folder.

Reviewed-By: Fedor Indutny <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants