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

Add %LocalAppData%\Yarn\.bin to PATH as part of installation #1129

Merged
merged 2 commits into from
Oct 17, 2016

Conversation

Daniel15
Copy link
Member

Summary
Updates the Windows installer to add %LocalAppData\Yarn\.bin to the PATH, so that we can remove the "path setup" section that was added in the docs (https://yarnpkg.com/en/docs/install)

Test plan
Ran installer on my computer, ensured the PATH was updated correctly:

Uninstalled Yarn, ensured both PATH additions were removed.

Permanent="no"
Part="last"
Action="set"
System="yes"
Copy link

@Jessidhia Jessidhia Oct 17, 2016

Choose a reason for hiding this comment

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

This is a per-user path, so other users in the same system don't have access to it.

This needs to be set up for each user that runs yarn. This will work on single-user systems, though.

An advantage of making it not System is that it will(?) roam with the user profile. While the local appdata folder won't roam, local installs with yarn will go to the local appdata, so they'll show up in the path once you create the folder with yarn.

Choose a reason for hiding this comment

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

Can you, maybe, set the path to be literally %LocalAppData%/Yarn/.bin, and hope the shell expands it when evaluating the PATH?

This, of course, will definitely not work on windows xp.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, thanks @Kovensky! I updated it to add it to the user environment variable rather than the system one. That should work fine. I don't think the shell expands environment variables like %LocalAppData% when resolving the PATH.

@Daniel15 Daniel15 merged commit c091ac4 into yarnpkg:master Oct 17, 2016
@Daniel15 Daniel15 deleted the win-path-rebased branch October 17, 2016 05:36
neojp added a commit to neojp/yarn that referenced this pull request Feb 4, 2017
* Change PATH from `%LocalAppData%\Yarn\.bin` to `%LocalAppData%\Yarn\config\global\node_modules\.bin` in the Windows installer
* Fixes the Windows PATH introduced in c091ac4 & yarnpkg#1129
* Allows global packages to be accessible through Windows PowerShell and CMD
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.

2 participants