Awesome!
Thanks

Great PR! Seems good to merge, except for the way you're accessing the command line parameter -
you should use the config object to store it, and retrieve it from there later (unless there's a reason why you can't?). Otherwise, it won't work properly with the .yarnrc command line arguments :)

ping @mgcrea, we'd love to have it in 0.25 that we plan to cut tomorrow

@arcanis / @bestander Is there precedent for that storage in config that we could follow?

Sure, you can take a look at this commit, for example. You just have to use getOption in the Config's init() method to set the variable to the right value, and after that you can access it from the config object (usually available via this.config).

@arcanis Is this what you had in mind?

@benlangfeld Not exactly, but not far! Look at the commit I just pushed if you're interested; basically, instead of adding a linkFileDependencies argument to the resolver, we just use the config object as source of truth.

@mgcrea I've added a test to make sure that everything works, and unfortunately it seems there's still a few issues (I quickly checked, and I think one of them is because the install command is trying to read the manifest from the location. The other one is inside the check command, which fails because it is trying to make sure that the package path exists, which might not be true if the link is a broken link, or if the link target has no package.json). Can you take a look at this?

Don't bother about the rebase, I'll fix them directly on Github before merging.

I've tested this branch and the package name is not inferred from the path:

$ yarn add link:../i18n-import-webpack-plugin --dev
yarn add v0.25.0-0
[1/4] 🔍  Resolving packages...
error Package "@0.0.0" doesn't have a "name".

I have to run the following instead:

yarn add i18n-import-webpack-plugin@link:../i18n-import-webpack-plugin --dev

The first command works with file: specifier.

Ping @mgcrea ? 🙂

@arcanis I'll have a look today. Thanks for the patches.

Did finish a first squash+rebase+update, had to add code since the functionality was broken by a few recent commits on master (mostly from the cli/commands/check.js related checks), I've kept the commit separated for now to ease the review of these new patches since initial commit. Some tests seems a bit hacky, but tried to keep the refactoring to a minimum.

Looks like my existing workflow is still broken on master, so I'll have to dig a bit deeper to make sure I have the relevant unit tests.

Thanks, @mgcrea

I think we're getting close to something "ready". I've fixed and added a unit-test for the issue encountered by @bazyli-brzoska. Test are OK, rebase done.

@bestander, regarding the package-fetcher.js patch, I tried again to use a LinkFetcher but it would have still required a lot of branching in PackageFetcher as we need to opt out cache entirely. So I decided against it (would lead to more LOCs & increased complexity).

Let me know if there is something else I can do.

@mgcrea, I agree about the fetcher, makes sense to have an if block rather than more complexity.

Amazing work, @mgcrea!
You've lead the whole feature from design to implementation.
I am sure quite a lot of people would want to use it if they know about it, do you want to post in the blog about it?

It is ready to merge although windows tests fail:

__tests__\commands\install\integration.js (120.573s)
  ● creates a symlink to a non-existing directory when using the link: protocol
    Error: expect(received).toEqual(expected)
    
    Expected value to equal:
      "../baz"
    Received:
      "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\yarn-install-link-1495898245160-0.05036491088610817\\baz\\"

@bestander can fix the tests (add a platform ternary) if you want.

Blogpost is a great idea, I'll try to come up with something.

I've fixed the tests for windows, (looks like the test are still failing there but for unrelated reasons).

Thanks, yeah one test got broken earlier, investigating

https://github.com/zertosh/yarn-link-test is an example of how link breaks down due to Node's realpath module resolution (nodejs/node#3402)

Much ❤️ to everyone involved here, particularly @mgcrea. You guys are awesome and this will make my team very happy. We owe you at least some drinks!

@bestander I guess this will now go in 0.26.0? By the recent cadence of minor releases, that would likely be cut within the next two weeks. Is that a fair estimate?

We plan to cut on Monday.
There are a couple of hi pri issues we want to fix before cutting 0.26.

So ... is this documented somewhere and/or example usage available?

Is

"my-package": "link:../some/path",  // package.depencencies entry

equivalent to

cd ../some/path
yarn link
cd -
yarn link my-package

?

(Equivalent meaning: will both approaches result in the same symlink being present in ./node_modules?)

Why is this feature not documented ? Can we use it safely ?