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

Require with a relative path should take into account it comes from a symbolic links #5481

Closed
julienw opened this issue Feb 28, 2016 · 8 comments
Labels
feature request Issues that request new features to be added to Node.js. module Issues and PRs related to the module subsystem.

Comments

@julienw
Copy link

julienw commented Feb 28, 2016

Please have a look at this gist for context: https://gist.github.com/julienw/7c31988d43b639dcfde2

Basically we have 2 files in 2 different directories. Yet in one another directory we have 2 links to these 2 files. If file1 has require('./file2') it will fail because node's require is looking only in the "real" directory where file1 is. I suggest to augment this behavior to look both in this directory and where the symbolic link is located.

I understand it's not very easy to do given the current code as node doesn't keep track that it loaded the file from a symbolic link in the first place. I'd be happy to contribute with some guidance.

@mscdex mscdex added module Issues and PRs related to the module subsystem. feature request Issues that request new features to be added to Node.js. labels Feb 28, 2016
@d3x0r
Copy link
Contributor

d3x0r commented Feb 28, 2016

have the outer define a prefix variable before requiring them? Or will they both get the modified result... like

var here = __dirname;
var LibPath = here + "/lib2";  // this doesn't start with a dot either (windows drive letter? network share?)
require( "lib1/something.js" );
LibPath = "./lib1"; // change to other directory for another
require( "lib2/otherthing.js" )

something.js
require( LibPath + "config.js" )

otherthing.js
require( LibPath + "config.js" )


I guess no....because LibPath is undefined....

@evanlucas
Copy link
Contributor

Dupe of #3402?

The module loader is locked. Introducing a breaking change to it would be a tough sell.

@julienw
Copy link
Author

julienw commented Feb 29, 2016

Actually in #3402 it's clearly said they don't want to change the behavior for relative paths (see especially #3402 (comment)). Which is my issue here. So it's quite different (and likely more difficult).

The module loader is locked. Introducing a breaking change to it would be a tough sell.

I understand. I can also totally imagine there is a lot of code that depends on this behavior because the behavior makes sense in a lot of cases as well.

So maybe the main issue is the lack of configurability for the resolving mechanism. We can't change the locations where node seaches the dependencies.

In my case, I want to run mocha on a directory that's made of symbolic linked files, that are not in the same source directory, but still the target directory's layout is what makes sense in my case, because it's the result of a build step.

So for example I'd need to tell node "hey node, this is the root of the requires, please look here if you don't find your requirements elsewhere". But even this would not be quite right.

Another way to look at it and that would work in my case, but is less generic because it requires the cooperation of the requiring script. If we keep track of where the parent of a script is (I think we do), we could propose either a specific prefix for the require argument (require('!parent/my/script.js') (we could also use top or root to denotate the root directory of the project, as other bugs are looking for), or a new function itself (requireParent('./my/script.js')).

@mattcollier
Copy link

New PR relevant to this issue here: #5950

@dlongley
Copy link

@julienw,

If PR #5950 is merged, then @kzc has suggested a workaround for you:

#5950 (comment)

@julienw
Copy link
Author

julienw commented Mar 31, 2016

Thanks, I'll be sure to try this !

@kzc
Copy link

kzc commented Apr 26, 2016

Thanks to @lamara and @dlongley, normal symlink behavior is now in the node 6.0.0 release.

It makes module testing much simpler now - no need to copy files and reinstall modules.

@jasnell
Copy link
Member

jasnell commented May 13, 2016

fyi... because of regressions caused by #5950, a new commit putting it's changes behind a --preserve-symlinks flag was just committed to master today. It should hit in the next v6 minor. Closing this as there does not appear to be anything left to do

@jasnell jasnell closed this as completed May 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. module Issues and PRs related to the module subsystem.
Projects
None yet
Development

No branches or pull requests

8 participants