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

Correctly resolve scoped packages in yarn create. #5983

Merged
merged 1 commit into from
Jul 19, 2018

Conversation

chriskrycho
Copy link
Contributor

@chriskrycho chriskrycho commented Jun 13, 2018

Summary

Fixes #5981 by using the scope as part of the path lookup, if it exists.

Note: this does not yet work on Windows, and it seems like we'll need to do some explicit platform checking to make it work on Windows.

Test plan

Output on macOS:

yarn create @olo/ember-app
yarn create v1.7.0
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
[4/4] 📃  Building fresh packages...

success Installed "@olo/create-ember-app@0.1.0" with binaries:
      - @olo/create-ember-app
[#######################################################################################] 1007/1008
App name:

Current (broken) output on Windows:

yarn create @olo/ember-app
yarn create v1.6.0
[1/4] Resolving packages...
warning @olo/create-ember-app > ember-cli > exists-sync@0.0.4: Please replace with usage of fs.existsSync
warning @olo/create-ember-app > ember-cli > broccoli-stew > broccoli-funnel > exists-sync@0.0.4: Please replace with usage of fs.existsSync
warning @olo/create-ember-app > ember-cli > ember-cli-preprocess-registry > exists-sync@0.0.3: Please replace with usage of fs.existsSync
[2/4] Fetching packages...
info fsevents@1.2.4: The platform "win32" is incompatible with this module.
info "fsevents@1.2.4" is an optional dependency and failed compatibility check. Excluding it from installation.
[3/4] Linking dependencies...
[4/4] Building fresh packages...
warning Your current version of Yarn is out of date. The latest version is "1.7.0", while you're on "1.6.0".
success Installed "@olo/create-ember-app@0.1.0" with binaries:
      - @olo/create-ember-app
The directory name is invalid.
error Command failed.
Exit code: 1
Command: C:\Users\chris\AppData\Local\Yarn\bin\@olo\create-ember-app
Arguments:
Directory: C:\Code\platform
Output:

info Visit https://yarnpkg.com/en/docs/cli/create for documentation about this command.

@chriskrycho chriskrycho force-pushed the yarn-create-scoped-package-3848 branch from 4a86fa6 to 274fd51 Compare June 13, 2018 17:58
@chriskrycho
Copy link
Contributor Author

chriskrycho commented Jun 13, 2018

I have taken a number of stabs at this on Windows, but am running into a bigger problem there: the binaries aren’t getting installed to the correct location at all in my local testing. It’s working fine with unscoped packages (create-react-app, for example) but while it reports that it installs the binaries, they don’t show up in the same location, possibly because Windows is unhappy about the @ in the package name? Would appreciate some guidance!

@hulkish
Copy link
Contributor

hulkish commented Jun 13, 2018

@chriskrycho would be good to add a test for this. Heck, I guess that means you'd have to make a __tests__/commands/create.js, since there isn't one yet, lol.

@chriskrycho
Copy link
Contributor Author

chriskrycho commented Jun 17, 2018 via email

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.

Sounds good, thanks! :)

@arcanis arcanis merged commit 59377f6 into yarnpkg:master Jul 19, 2018
@chriskrycho chriskrycho deleted the yarn-create-scoped-package-3848 branch July 19, 2018 15:56
@fenduru
Copy link
Contributor

fenduru commented Aug 14, 2018

I believe this is the change that caused the regression in #6266. path/prefix/bin/@scope/create-foo simply is not created when running yarn global add @scope/create-foo, create-foo is added to path/prefix/bin/create-foo (which is necessary for it to be included in the PATH that Yarn installer adds).

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.

4 participants