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

Nix yarn binstub #367

Merged
merged 3 commits into from
May 12, 2017
Merged

Nix yarn binstub #367

merged 3 commits into from
May 12, 2017

Conversation

gauravtiwari
Copy link
Member

Lets just use yarn executable directly instead of using the same executable through a binstub to make things simple across all platforms. Since we check for yarn executable presence and version before installing Webpacker this shouldn't be a problem.

Companion PR #363

@gauravtiwari gauravtiwari merged commit c07bc8e into rails:master May 12, 2017
@gauravtiwari gauravtiwari deleted the use-yarn-executable branch May 12, 2017 06:53
@gauravtiwari
Copy link
Member Author

Thanks for reviewing 😄

@swrobel
Copy link
Contributor

swrobel commented May 23, 2017

Rails 5.1 task yarn:install still expects ./bin/yarn to exist. What is the recommended solution? Currently rake assets:precompile fails with error Command "webpack" not found.

@gauravtiwari
Copy link
Member Author

@swrobel I think we change this behaviour in Rails as well. Could you open a ticket there please?

swrobel added a commit to swrobel/rails that referenced this pull request May 23, 2017
Webpacker has [removed the yarn binstub](rails/webpacker#367)
@rmacklin
Copy link
Contributor

I actually liked the flexibility provided by having this binstub. It allows application owners to change the binstub's contents and webpacker will invoke the changed version.

For example, one use case is to change the binstub to be a standalone copy of yarn (which is distributed with each release: https://github.com/yarnpkg/yarn/releases). Doing so ensures that all developers, as well as CI and production machines are using the same exact yarn version, which makes it impossible for issues to arise due to one user/machine using a different yarn version than another user/machine. And since yarn is contained in the repo, upgrading it is trivial, and automatically applies to all developers when they git pull. These are very nice properties to have.

Would you be open to accepting a PR to add back the binstub?

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