Repository navigation
Don't resolve symlinks when requiring #3402
Description
Activity
- changed the title
[-]Don't resolve symlinks[/-][+]Don't resolve symlinks when requiring[/+]on Oct 16, 2015 This would break
npm link, wouldn't it?if you are requiring dep1, and it is a symlink, and it doesn't have dep2 in its path then something is off is it not? Should the module not include its own node_module folder?
That being said flat dependencies in npm@3 do offer a weird edge case to this. But it is worth bringing up that I am pretty sure they are deprecating peer dependencies.
@Trott why would it? In theory, everything that works with the real path should also work fine with the virtual path. The only real problem that i see is when a module gets required multiple times through different symlinks it would not be cached by npm and get loaded multiple times. But if that really happens, something is badly designed i think.
@thealphanerd
No, because its a peer dependency.
I don't think NPM is depreciating peer dependencies in gerenal, but changing its behavior to just warn when a peer dependency is not fullfilled. Maybe they're even gonna drop the peerDependencies property, but that does not prevent people to peer depend on things. Devs will then just need to manage them on their own.I don't think the need for peer dependencies will go away in the future.
Reacted by Md. Shahadat Hossain KhanI've also asked for a new npm command that would only work if we fix the behavior of require like i suggested here: npm/npm#10000 (comment)
@VanCoding I was thinking of certain edge cases but I wasn't thinking very deeply about it, so yeah, that particular comment may be a non-issue.
The real point I was sorta kinda trying to gently make was better put by @othiym23 (emphasis added):
I do think you're right that require()'s behavior is unhelpful in this case, but I also think that given how important having that behavior locked down is to the stability of the whole Node ecosystem, it's going to be very tough to change now.
Any semver-major change to
require()could result in all sorts of (potentially hard-to-predict) ecosystem breakage. So the bar is very, very high.I know, but I think the current behavior maybe even counts as a bug because
there seems to be no reason to behave like this..
Am 17.10.2015 7:06 nachm. schrieb "Rich Trott" notifications@github.com:Moreover, the Modules API (which require is a part of) is Locked which
means:Only fixes related to security, performance, or bug fixes will be accepted.
Please do not suggest API changes in this area; they will be refused.—
Reply to this email directly or view it on GitHub
#3402 (comment).The litmus test is this: is there a non-zero chance the proposed change is going to break someone's existing application? If the answer is 'yes' (and I think it is), then it should be rejected.
Reacted by Joakim L. Christiansen, August Saint Freytag, almosnow, caramboleyo-no2fa, Michael Allwright and chrono-meterWell, how are we going to calculate that chance? And what does a zero
chance mean? Zero like not even module is going to break or not 1 % or more
of the pakages? If it's the latter, then i really doubt that 1% of the
modules would be affected by that change. If there really are modules that
break with this change then theyre making use of undocumented (in my
opinion buggy) behavior and therefore it's okay when it breaks.Let's look at it like this: one can always get the real path from the
virtual path, but if one gets the real path, the virtual path is lost.So even if we break some modules, isn't the right decision to fix this? I
really think it would help fix issues for npm as well.But it of course is your decision.
Am 17.10.2015 7:34 nachm. schrieb "Ben Noordhuis" <notifications@github.com:
The litmus test is this: is there a non-zero chance the proposed change is
going to break someone's existing application? If the answer is 'yes' (and
I think it is), then it should be rejected.—
Reply to this email directly or view it on GitHub
#3402 (comment).@VanCoding, maybe hard links would help. I haven't tried it.
@dlongley You can't hardlink a directory I think?
I am having the exact same issue as @VanCoding and I'm also a bit confused by the current behaviour...?
I would expect a linked package to resolve dependencies based on the location it was linked to. It should of course check it's own node_modules first, but when going back "up the tree" I would expect it to check the folder structure it was linked to, not where it was linked from.
Reacted by Eisi Sig, Toniodev, aMarCruz, Joe, Martin Charles, Anton Pawlik, Luke Street, Edu, Jacob Page, Matt Hirsch and 5 moreYou can't hardlink a directory I think?
No, you can't, but maybe you could write a script to create a mock directory and hard link any
jsfiles. I think that would be all it would take for simple modules. It's not a perfect solution but maybe it would help in some cases. Clearly there needs to be a better way to link peer dependencies together during development.I would expect a linked package to resolve dependencies based on the location it was linked to.
Me too, but I believe this has been discussed before -- and there may be projects out there that are depending on the current behavior. We'll just need to come up with a way to specify that the other behavior is desired.
191 remaining items
@jasnell Peer dependencies was just one advantage. The 6.0.0 module resolution behavior also facilitates testing of modules with symlinks without the need for copying files or installs. It also allows the layout of customized hand built installs to save disk space on constrained devices. It offered a lot of flexibility that the old scheme lacks.
It would be useful if the 6.0.0 module resolution behavior could exist behind a command line flag defaulted to false rather than removing it altogether.
That very well could be an option but we need to unbreak things first, and
then move forward from there.I am definitely considering the flag approach to enable the new behavior.
I've already started exploring that, in fact, and it shouldn't be too
difficult to do at all. I'd just also like to continue looking into whether
we can solve it without a flag tho.Reacted by Dave Longley, David I. Lehn, Alexander Gugel, Tim Kevin Oxley and Anil KulkarniTl;Dr is this: we had a bug in the module loader that prevented symlinked peer dependencies from finding each other. We fixed it but the fix broke other things. So we are currently looking to revert that fix and look at solving it a different way.
I don't entirely agree that this is a bug.
However, if we are going to say that the
require()look path should include thenode_modulesfolders based on the symlinked location of a module, then it isn't acceptable to remove thenode_modulesfolders based on the realpath location of a module.Here is a git repo with 2 clear examples of what changed, and why this is either subtle and inefficient, or outright harmful and surprising, based on how module dependencies have been linked: https://github.com/isaacs/node6-module-system-change
(Note: I've written programs that
require()a module from on a symlinked location and expect it to still be able to load its deps, so while the example is contrived in its minimal-ness, it's not contrived in principle and does reflect some real-world usage.)An ideal solution, if it is a goal to have symlinked modules find one another if they are not otherwise dependent on one another (which, again, I am highly skeptical about as being a good idea) would have to prioritize the pre6.0 realpath-location-based lookup behavior as the first priority lookup path, and then add the
node_moduleslookup locations as a lower-priority set of paths.The cache entry should still be based on the realpath for the sake of efficiency and minimizing the semantic change in what is a singleton.
If you are affected by this today, you can easily work around the bug by setting the
NODE_PATHenvironment variable to thenode_modulesfolder where you're sticking stuff.I think that it'd be an interesting idea to add the main module's lookup paths to the
require()calls done by other modules, but even that should be messaged front and center as a significant and potentially hazardous change.Reacted by Alexander Gugel, Troy Astorino and Iceberg Editorial TeamUpdated the git repo with a few specific proposals.
if it is a goal to have symlinked modules find one another if they are not otherwise dependent on one another (which, again, I am highly skeptical about as being a good idea)
Well, with the growing trend of the monorepo approach and a significant boost to development speed that thought deserves the benefit of the doubt.
Reacted by Zoltan Kochan, Chris Slater, Rhy Moore and chrono-meter- added a commit that references this issue
on Aug 11, 2016 - added a commit that references this issue
on Nov 29, 2016 Since this issue has the discuss label, I thought I could ask here ...
The second best thing to being able to do
const WebSocket = require('../vendors/node/node_modules/ws');(which cannot be done) is having a symlinked node_modules directory like below.Which version of the behavior would allow the following?
/var/www/project/ ├── app │ └── ws │ ├── node_modules -> /var/www/project/vendors/node/node_modules │ └── wsserver.js └── vendors └── node ├── node_modules └── package.json- added a commit that references this issue
on Jun 15, 2018 - added a commit that references this issue
on Jun 17, 2018 @majid4466 symlinking that node_modules dir should work with the current node.
As for the original opening question: dep1 probably shouldn't require dep2, but instead provide a factory that can produce whatever based on dep2. The factory could have a meta data property to declare which other modules the app (or a plugin manager) needs to provide. It's one flavor of dependency injection.
Reacted by kakysha, Zbyszek Tenerowicz, David Wheeler and almosnow
Currently, Node.js resolves symlinks when requiring and then uses the real location of the package/file as its __filename and __dirname instead of the symlinked one.
This is a problem because symlinked modules don't act the same as locally copied modules.
For example:
works, but
does not, because dep1 does not act like it's located in the node_modules directory of module and thus cannot find dep2.
This is especially a problem when one wants to symlink modules that have peer-dependencies.
Therefore, I suggest changing the behavior to no longer resolve symlinks.
What do you think?