-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Deployment instructions for Heroku apps potentially problematic #3212
Comments
The ruby buildpack should now include Node for Webpacker out of the box, so is this necessary at all? Could we just remove the node buildpack recommendation from the docs? |
Technically it's not (at least not right now). However, I would advise against removing for two reasons:
|
@guillaumebriday I made a small update here: https://github.com/rails/webpacker/pull/3231/files @andreas-venturini Would be great if you can submit a PR with som add some additional details regarding the duplicate yarn install. @dwightwatson I think having both the ruby and node buildpacks makes logical sense for Heroku applications using webpack. |
Great! I'm pretty sure we can close this issue right? |
@guillaumebriday, yes, I think so. In the future, somebody can open up a more specific issue with a problem and maybe a proposed solution. While I think it's suboptimal for yarn to run twice with Heroku, it runs quite fast the second time, so it's not a huge issue. |
In the docs section about Heroku deployments it's recommended to include the Heroku nodejs buildpack.
However, this is potentially problematic as in Rails apps
yarn install
will again be run as part of the asset precompilation step if the yarn binstub is present (which is probably the case for the majority of Rails apps)This leads to yarn install being executed twice, as described in this comment .
In the same issue one suggested workaround is to manually disable the yarn install rake task - I think this should be added as a disclaimer to the deployment docs section.
If this is an agreeable change I can send a PR.
The text was updated successfully, but these errors were encountered: