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

Running yarn link unlinks any modules linked via yarn link. #4770

Closed
chaddjohnson opened this issue Oct 24, 2017 · 5 comments · Fixed by #4757
Closed

Running yarn link unlinks any modules linked via yarn link. #4770

chaddjohnson opened this issue Oct 24, 2017 · 5 comments · Fixed by #4757
Assignees

Comments

@chaddjohnson
Copy link

Yarn version 1.2.1.

Do you want to request a feature or report a bug?

Bug.

What is the current behavior?

Running yarn add [package-name] unlinks any linked modules. Here is a screenshot showing this:

screen shot 2017-10-24 at 10 18 15 am

If the current behavior is a bug, please provide the steps to reproduce.

cd app
ls -l node_modules | grep sequential --> shows that the module is not linked
yarn link sequential-ids
ls -l node_modules | grep sequential --> shows that the module is now linked
yarn add lodash
ls -l node_modules | grep sequential --> shows that the module is now unlinked

What is the expected behavior?

yarn add [package-name] should never cause linked packages to become unlinked.

Please mention your node.js, yarn and operating system version.

Node 8.6.0.

macOS High Sierra. This also happened on Sierra.

@wclr
Copy link
Contributor

wclr commented Oct 24, 2017

Right, before v1 yarn didn't touch symlinks in node_modules, but now when it performs some actions in node_modles it removes the symlinks of deps folders if it is not link: dependency and replaces it with version from the cache or local folder (if file: dep).

I'm not sure if it is by design behavior.

Also, want to mention yalc as an alternative to yarn link workflow and getting rid of symlinks.

@bestander
Copy link
Member

@arcanis @BYK, raising this to your attention, looks like a noticeable regression.

@AlastairTaft
Copy link

Would be really useful for me if this one was fixed.

@BYK BYK closed this as completed in #4757 Oct 26, 2017
BYK pushed a commit that referenced this issue Oct 26, 2017
**Summary**

 Actual fix: changed fs.readlink to fs.realpath when checking if a symlink is a linked dependency in package-linker.js This fixes yarn removing linked deps when installing or updating.

Fixes #3288, fixes #4770, fixes #4635, fixes #4603.

Potential fix for #3202.

**Test plan**

See #3288 (comment) for repro steps.
See #3288 (comment) for my explanation of the problem.

With a real world test scenario this works, but I'm unable to have it break from a unit test. I added a test in the integration suite but with the bug added back in it still passes because both generated paths are identical. I would like some help with the unit test.
@chaddjohnson
Copy link
Author

I understand this is fixed. Thank you. How can we test?

@wclr
Copy link
Contributor

wclr commented Oct 26, 2017

I understand this is fixed. Thank you. How can we test?

Clone master and build it, install using npm link, or wait for a release)

joaolucasl pushed a commit to joaolucasl/yarn that referenced this issue Oct 27, 2017
…#4757)

**Summary**

 Actual fix: changed fs.readlink to fs.realpath when checking if a symlink is a linked dependency in package-linker.js This fixes yarn removing linked deps when installing or updating.

Fixes yarnpkg#3288, fixes yarnpkg#4770, fixes yarnpkg#4635, fixes yarnpkg#4603.

Potential fix for yarnpkg#3202.

**Test plan**

See yarnpkg#3288 (comment) for repro steps.
See yarnpkg#3288 (comment) for my explanation of the problem.

With a real world test scenario this works, but I'm unable to have it break from a unit test. I added a test in the integration suite but with the bug added back in it still passes because both generated paths are identical. I would like some help with the unit test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants