Skip to content

Naming the build script build makes PaaS detect a Rails apps as a Node app #16

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

Closed
czj opened this issue Sep 10, 2021 · 17 comments
Closed

Comments

@czj
Copy link

czj commented Sep 10, 2021

When deploying a Rails app to a platform like Heroku with multiple buildpacks (Node + Ruby), the app is detected as as Node app because package.json contains a build script.

Would you consider naming the build script build:js like in cssbundling-rails where the build script is named build:css ?

I would be happy to create a PR for that.

@czj czj changed the title Naming the build script "build" makes PasS detect a Rails apps as a Node app Naming the build script `buildù makes PasS detect a Rails apps as a Node app Sep 10, 2021
@czj czj changed the title Naming the build script `buildù makes PasS detect a Rails apps as a Node app Naming the build script build makes PasS detect a Rails apps as a Node app Sep 10, 2021
@czj czj changed the title Naming the build script build makes PasS detect a Rails apps as a Node app Naming the build script build makes PaaS detect a Rails apps as a Node app Sep 10, 2021
@dhh
Copy link
Member

dhh commented Sep 10, 2021

Think we should get Heroku to be smarter about it's detection. Build is the standard indeed. Maybe @schneems can help adjust the buildpack detection?

@czj
Copy link
Author

czj commented Sep 10, 2021

To be precise, this happened to us on a large / popular European PaaS that uses Heroku’s buildpacks.

@dhh
Copy link
Member

dhh commented Sep 10, 2021

What's the PasS? Maybe we can tag them here so they can update and be ready for Rails 7.

@dhh
Copy link
Member

dhh commented Sep 10, 2021

Because the detection is IN PART correct! A Rails 7 app with a build script in package.json is both a node app AND a Rails app at the same time. Needs both aspects configured correctly.

@czj
Copy link
Author

czj commented Sep 10, 2021

They are https://github.com/Scalingo

I'm currently checking with them if they have some internal scripts around/over Heroku's buildpacks that might be interfering.

@czj
Copy link
Author

czj commented Sep 10, 2021

They have a custom flag NPM_NO_BUILD=true for the Node.js Buildpack to disable runnings scripts on deploy. They'll document the practice in the future.

Another solution seems to be relying on the Rails' Buildpack's default node/yarn versions (which can be a bit dated).

Sorry for the trouble.

@dhh
Copy link
Member

dhh commented Sep 12, 2021

No worries. This is a new path. Gotta smooth it all out.

@schneems
Copy link
Member

the app is detected as as Node app because package.json contains a build script.

If an app has both a package.json and a Gemfile we have written our detect order so that the heroku/ruby buildpack "wins". If that's not what you're seeing then let me know. I tried to reproduce the behavior you described and couldn't.

$ cat Gemfile
source 'https://rubygems.org'
gem 'rake'
$ cat package.json
{
  "scripts": {
    "start": "node index.js",
    "build": "webpack"
  }
}
$ git push heroku
Enumerating objects: 7, done.
Counting objects: 100% (7/7), done.
Delta compression using up to 16 threads
Compressing objects: 100% (5/5), done.
Writing objects: 100% (7/7), 672 bytes | 672.00 KiB/s, done.
Total 7 (delta 1), reused 0 (delta 0), pack-reused 0
remote: Compressing source files... done.
remote: Building source:
remote:
remote: -----> Building on the Heroku-20 stack
remote: -----> Determining which buildpack to use for this app
remote:  !     Warning: Multiple default buildpacks reported the ability to handle this app. The first buildpack in the list below will be used.
remote:       Detected buildpacks: Ruby,Node.js
remote:       See https://devcenter.heroku.com/articles/buildpacks#buildpack-detect-order
remote: -----> Ruby app detected
remote: -----> Installing bundler 2.2.21
remote: -----> Removing BUNDLED WITH version in the Gemfile.lock

Even with a package.json containing a build script, it still detected as a ruby app which I believe is the desired behavior.

Think we should get Heroku to be smarter about it's detection.

Originally I misunderstood the issue and thought this comment on build:js was in relation to needing to specify a build script in the package.json. On re-read I see that's not the case. Still, it might be helpful to get a status update on the buildpack ecosystem.

The Ruby buildpack doesn't know anything about node, package.json or any of it's ecosystem.. The interface is that: if the app has execjs or webpacker bundled in the Gemfile then we ensure a version of node is on the app. https://github.com/heroku/heroku-buildpack-ruby/blob/41a5b1fc25e1a7e3627f5a836e8f91c5c884507b/lib/language_pack/ruby.rb#L1056-L1060.

Then we guarantee we will call rake assets:precompile after dependencies are installed. That's our standard interface. The way webpacker works with this is it enhances/extends the assets:precompile task to also invoke a yarn:install task first.

That being said, there's change in the wind. I want to get the Ruby buildpack out of the node installation business. It knows too much already and it's really confusing that we have a separate heroku/nodejs buildpack meaning that devs who use the two together get different behavior than just the devs who use only heroku/ruby. The nodejs buildpack DOES know about build and does execute it, it's a common interface for that ecosystem in the same way that assets:precompile is the interface for the Ruby community (not just rails, we call it for any app...it's essentially like an unofficial npm build that Rails made and we endorsed).

The way forward is for the Ruby buildpack to depend on the node buildpack. Unfortunately this way forward not easy. Heroku is going through the process of switching our standard of buildpack interface from the existing "legacy" aka "v2" interface https://devcenter.heroku.com/articles/buildpack-api in favor of the fully spec-d Cloud Native Buildpack (CNB) interface (https://buildpacks.io/). Cloud Native Buildpacks allow one buildpack to "depend" on another.

This allows the Ruby buildpack to say "I see you've got execjs/webpacker, make sure to run the nodejs buildpack first".

We are currently re-writing buildpacks to support the CNB interface. You can see my work here github.com/heroku/buildpacks-ruby along with an "application contract" of what an application can expect while using the buildpack.

There's a few more complicating factors: Heroku build system doesn't yet support running CNBs so we must support 2 buildpacks right now. There are issues depending on the node buildpack, for instance since it calls yarn install and rake assets:precompile also calls yarn install it gets run twice. The node buildpack clears dev dependencies after executing but the rake task needs them. (Documented as a TODO in the application contract).

Organizationally we've also decided to move away from each buildpack defining what language to be written in (Previously Ruby is written in bootstrapped Ruby while all other buildpacks are written in bash). We landed on a choice of re-writing in Rust. This means my work on my re-write I linked to today is stopped and exists only as a spike/reference as we ramp up to re-write it yet again. It's fairly well spec-d out so It should go faster, but it's not here today as a thing we can use right now.

Long term

In an ideal world our build ecosystem would support CNBs, we'll have customers transitioned over to that interface, the Ruby buildpack will be re-written in Rust and it will depend on the heroku/nodejs buildpack, the issues around clearing dev-dependencies between buildpacks will be offloaded (possibly to a RFC for a "clean" phase in cloud native buildpacks). And then you get your execution of npm build for free.

I doubt all that will happen before Rails 7 hits the streets. Any ballpark when that might optimistically/pessimistically happen? If it's awhile away then maybe there's a chance. If it's soon, then we'll have to look elsewhere

Short term

In the short term the workaround would be (if I understand the requirements) that developers add in the heroku/nodejs buildpack before deploying:

heroku buildpacks:add heroku/nodejs
heroku buildpacks:add heroku/ruby

It does mean that deploying to Heroku is no longer a "zero config, one step process" which we worked to get to with Rails 5.

I'm wondering if there might be other interfaces that could be leveraged to better tell heroku "do this thing by default". We do have the concept of an app.json file which you can specify buildpacks, but unfortunately, it only affects review apps and apps created by "heroku button" (https://devcenter.heroku.com/articles/heroku-button). It does not affect apps created by heroku create which in my personal opinion is a huge missed opportunity, otherwise, we could just have Rails add one in by default and it would "just work" ™️. There might be some other ways around the issue. None come to mind though.

Into the future, once we get Ruby CNB shipped and these issues ironed out then it will go back to a "zero config" deploy and will "just work" and anyone who previously specified to use both heroku/nodejs and heroku/ruby will resolve to get the same behavior (basically there's not a downside to pushing people to use both buildpacks that we'll regret later).

I know that's a wall of text. I want to be very transparent about the state of the buildpack is currently in and where we're headed. Let me know if you've got additional questions about any of this.

@dhh
Copy link
Member

dhh commented Sep 13, 2021

I'll review all this is detail, but a few quick notes, there's no Webpacker or execjs with Rails 7. jsbundling-rails uses node directly without any of that, and the default path doesn't use node at all. So shouldn't base any detection scheme for Rails 7 for any of that.

With jsbundling-rails, though, it hooks onto assets:precompile to do its yarn install && yarn build automatically prior to running that task.

Rails 7 is going to ship an alpha as soon as this week. Then probably the first beta within a month. Maybe quicker. And then final 1-2 months after that. So let's say final is 2-3 months away.

@schneems
Copy link
Member

Thanks for the update. I can try to port our Rails docs over to use Rails 7 as soon as a pre-release or alpha is available. If you're curious I use a custom build tool to dynamically "run" my documentation to ensure it actually deploys on Heroku. Here's the actual file I use for Rails 6 https://github.com/schneems/rundoc/blob/main/test/fixtures/rails_6/rundoc.md.

If it uses rake assets:precompile then it the asset generation should be good to go out of the box. We might need to coordinate in the future on how to not call yarn install twice for people who have both heroku/nodejs and heroku/ruby buildpacks. Either way for the rake yarn:install task to know that yarn install has already been run or a standard interface that the Ruby buildpack can use to skip it (the ruby buildpack knows whether or not the node buildpack has run yarn install or not).

The only incompatibility with Rails 7 I know of is these two tasks were previously deprecated and I believe they've been removed https://github.com/heroku/heroku-buildpack-ruby/blob/41a5b1fc25e1a7e3627f5a836e8f91c5c884507b/lib/language_pack/test/rails2.rb#L56-L57. We call them for test setup. It won't be a blocker for people trying Rails 7 on Heroku but will limit use of our CI feature until it gets fixed on our end to use whatever "propper" interface we deprecated those in favor of (I don't know off the top of my head).

@dhh
Copy link
Member

dhh commented Sep 17, 2021

Got the alpha out the door for Rails 7. It ships with sprockets as-is, which I think default adds execjs to the Gemfile.lock? So the detection should work correctly?

@schneems
Copy link
Member

schneems commented Sep 18, 2021 via email

@dhh
Copy link
Member

dhh commented Sep 18, 2021

Looking good for import maps. Is everything also working with jsbundling? You can do rails new test -j esbuild to test that setup 👍

@dhh
Copy link
Member

dhh commented Nov 17, 2021

Confirmed that things work out of the box with rails new test -j esbuild on Heroku 👍

@dhh dhh closed this as completed Nov 17, 2021
@czj
Copy link
Author

czj commented Nov 17, 2021

Confirmed fine on other PaaS too. Thanks !

@rsanheim
Copy link

@schneems I've read thru this issue quite a few times, and don't see a clear way to avoid the multiple yarn install issue on Heroku w/ jsbundling and the nodejs buildpack installed. Opening a new issue at #130 to see if there is a nice way to do that. 🙏

@monfresh
Copy link

monfresh commented Feb 8, 2024

UPDATE: I ended up fixing the issue below by removing the build entry from package.json, and overriding the jsbundling-rails build_command so that the equivalent of yarn build runs later, during rake assets:precompile. Not the best solution, but it does the job. Now I can have both Node and Ruby buildpacks, and Heroku won't try to run yarn build before Ruby is installed.

# in lib/tasks/build.rake
module Jsbundling
  module Tasks
    extend self

    def build_command
      "node_modules/.bin/webpack --config webpack.config.js"
    end
  end
end

@schneems I am still experiencing the issue where Heroku detects the Rails app as a Node.js app due to the build script in package.json:

"scripts": {
  "lint": "eslint",
  "build": "webpack --config ./webpack.config.js"
}

In my case, I depend on rails-erb-loader to process a js.erb file, and it depends on Ruby being present first because it calls bin/rails runner. Here's an excerpt from the webpack.config.js:

module: {
    rules: [
      {
        test: /\.erb$/,
        enforce: 'pre',
        exclude: /node_modules/,
        use: [{
          loader: "rails-erb-loader",
          options: {
            runner: "bin/rails runner",
          },
        }],
      }
    ],
  },

I tried placing the Ruby buildpack before the Node buildpack, but then I run into the issue where it defaults to Node 20 instead of the one specified in the package.json.

Note that I don't have the execjs nor the webpacker gems in the app.

What would be the solution in this case? How can I ensure Ruby gets built first, and that Heroku can respect the Node version in package.json?

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

No branches or pull requests

5 participants