puzzle / skills

Open source skill management web application
GNU Affero General Public License v3.0
61 stars 19 forks source link

Add `esbuild-rails` and upgrade `esbuild` #760

Closed marcoroth closed 1 month ago

marcoroth commented 1 month ago

This pull request implements esbuild-rails so that the Stimulus controllers don't have to be manually registered. Instead they are picked up and registered automatically.

Additionally, this pull request bumps esbuild to the latest version.

Robin481 commented 1 month ago

Thanks for your pull request @marcoroth!

Just had a few minutes to look at it. The changes all seem reasonable to me.

I looked at the new autoreloading logic and added js: yarn build --reload to the Procfile.dev which in turn makes the autoreloading work as expected, lovely! :partying_face:

However, we also have the Procfile.assets which only starts the asset watchers. (this is my preferred way as i can just run rails s in a seperate terminal and pry debugging works without any extra steps)

I can't seem to make it work with the Procfile.assets file though as when adding --reload there the script fails as it complains about the port 3000 already being used. I am assuming this is because I now started rails in a seperate terminal and it is not foreman handling the rails process anymore (?).

Do you have an input for me on how to solve this? :smile_cat:

Steps to reproduce:

marcoroth commented 1 month ago

Hey @Robin481, I changed the Procfile.assets to just yarn build --reload and running rails s and bin/assets in separate terminals it works just fine.

Maybe you had another Rails server running somewhere?

Robin481 commented 1 month ago

@marcoroth Thanks for your input, it was just the bin/assets task itself that runs foreman on port 3000 :man_facepalming: Could've figured that one out earlier. I guess you have the PORT env variable set so it doesn't start foreman on 3000 for you.

Thanks for your PR and your help :heart: