yortus / require-self

Lets you require('foobar') from within foobar itself
MIT License
19 stars 5 forks source link

Creating a symlink instead of a proxy file #6

Closed zkochan closed 7 years ago

zkochan commented 7 years ago

This package solves the problem for when the main file is accessed. Frequently though, other files are accessed, like require('lodash/reduce'). Would you accept a PR that would link the package to node_modules? I'd use this package of mine for symlinking - symlink-dir. It works well cross-platform and is used at pnpm as well.

P.S. some packages already use this approach, like webpack https://github.com/webpack/webpack/blob/master/ci/travis-install.sh

yortus commented 7 years ago

The problem with symlinks in this case is that it would create an endless nesting of directories. I don't think it's even permitted to symlink to a parent directory.

zkochan commented 7 years ago

It is allowed. Circular symlinks are mostly fine. There might be an issue only with travis when it will try to cache node_modules. Haven't tested it though..

However, the benefits of this approach are tremendous compared to proxying just the main file.

Additionally to the use case in my first message, it will also work with typings

yortus commented 7 years ago

I expect it would cause problems with tools that traverse directories that are not written to expect cyclic symlinks. The top google results for 'circular symlinks' are mostly about the problems they cause and advise not to create them.

They do seem a nice solution for requiring oneself, especially the 'self/x' pattern, but the problems they cause for tools is the trade-off. Since this tiny lib should make as few assumptions about the user's setup as possible, I don't think circular symlinks are justifyable given they will break some tools and IDEs.

Having said that, if you can demo it working cross platform with a bunch of typical JS tools and IDEs, I'd be curious to see it.

zkochan commented 7 years ago

Well, until recently pnpm was creating a node_modules structure that was having a lot of circular symlinks. In fact, that latest v0.50 still has circular symlinks. Only v0.51 (which is not latest yet) has changed the structure and now avoids circular symlinks.

All tools that I used on Windows and Linux worked well with circular dependencies.

The only issues we had was caching on Travis and using node-newrelic, which seems like is having an infinite loop when detecting a circular symlink.

zkochan commented 7 years ago

There is another solution that solves circular symlinks but might have unintended results.

The symlink can be created in a node_modules folder outside of the project folder

├─ node_modules/
|     └─ foo/              -> ../foo
└─ foo/                    installation was done in this directory
yortus commented 7 years ago

I think there are plenty of other tools that may break. VSCode has an issue logged about it to name just one.

If you have that much control over a project's structure, then you don't need symlinks at all. If you work in a folder structure like:

├─ node_modules/
|     └─ foo/

...then foo can already require itself, no symlinks needed. But again, I don't see how an npm-installed lib like this one can dictate the folder arrangement of the modules using it.

zkochan commented 7 years ago

No, I mean foo is your project and it is linked to a node_modules folder in the parent directory. In that case there is no circular symlink

yortus commented 7 years ago

I know what you mean, I'm just pointing out that if your project is 'foo' and you control the directory structure, then just work on foo inside an enclosing directory named 'node_modules', and foo will be able to require itself without any symlinks.

zkochan commented 7 years ago

I think there are plenty of other tools that may break. VSCode has an issue logged about it to name just one.

Weird, I use vscode and it seems to work fine. Anyway, I don't insist. This can be done as a separate package, it is just a few lines of code. Your points are valid.

brillout commented 7 years ago

@zkochan's implementation; https://github.com/zkochan/self-import