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

Make yarn link collision message display existing link path #7215

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

k80w
Copy link

@k80w k80w commented Apr 21, 2019

Currently, running yarn link for a package whose name collides with an existing link spits out a rather unhelpful warning:

There's already a package called "$0" registered. This command has had no effect. If this command was run in another folder with the same name, the other folder is still linked. Please run yarn unlink in the other folder if you want to register this folder.

This PR causes the message to include the path to which the existing link points, as well as removing the "If this command was run in..." sentence, as this command has to have already been run in another folder for this message to show up in the first place.

Output

$ yarn-local link
yarn link v1.16.0-0
warning There's already a package called "matrix-react-sdk" registered pointing to "../../../src/gitlab.com/dnaf/riotdx/matrix-react-sdk". This command has had no effect. Please run yarn unlink in the other folder if you want to register this folder.
Done in 0.36s.

This PR fixes #7054

Copy link
Member

@arcanis arcanis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also update the CHANGELOG.md file at the root of the repository! 🙂

@@ -58,7 +58,8 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg

const linkLoc = path.join(config.linkFolder, name);
if (await fs.exists(linkLoc)) {
reporter.warn(reporter.lang('linkCollision', name));
const linkTarget = await fs.readlink(linkLoc);
reporter.warn(reporter.lang('linkCollision', name, linkTarget));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can improve the message even further by using path.resolve to have an absolute path rather than a relative path, what do you think?

@@ -72,7 +73,8 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg
const binSrcLoc = path.join(linkLoc, binSrc);
const binDestLoc = path.join(globalBinFolder, binName);
if (await fs.exists(binDestLoc)) {
reporter.warn(reporter.lang('binLinkCollision', binName));
const binDestLinkTarget = await fs.readlink(binDestLoc);
reporter.warn(reporter.lang('binLinkCollision', binName, binDestLinkTarget));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

@corysimmons
Copy link

Rebase/merge, then add slightly better message in the future?

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 this pull request may close these issues.

yarn link error message is not helpful
3 participants