Engine server hook

[bruno-]

Hi,

this is a proof of concept for automatically running watch mode alongside any rails server in development.

This is the alternative solution to work done in #300. While puma plugin from that PR works only with puma, this approach works with any server (puma included, of course).

I'm looking for some feedback from the maintainers, also from @npezza93 , @javierav because it affects #327 and #331.

[flavorjones]

I've rebased this branch and added a commit to avoid breaking the debugger (see #346 and #349 for context).

[brunoprietog]

How about a breaking change for a 3.0 version with an update notice? If this is merged, hopefully, we would have 3 ways to run the gem, which means more unnecessary confusion for devs. Couldn't this replace all the paths? Why would we need the other methods having this?

[flavorjones]

@brunoprietog I agree we don't need to support multiple ways of running the watcher in a server if this gets merged, so I intend to remove the other methods, eventually.

However, I'd prefer to separate the introduction of the server hook from the removal of the other methods of running the watcher. Having a deprecation period, even a short one, will allow people to choose when to upgrade. The cost of doing so is not very high, IMHO, and it will save us from having to deal with the support burden.

[flavorjones]

@bruno- Let me know if you want to try to finish this up per my comments above? If not, I'll find time to work on it this week -- I just don't want to duplicate efforts! Thanks.

[bruno-]

@flavorjones thank you for green-lighting this feature.

I can make changes based on your suggestions.

[bruno-]

Quick update - I addressed everything we talked in this thread, but I encountered a problem:

• Puma can be restarted via touch tmp/restart.txt or by sending a USR1 signal to the process.
• It then calls Kernel.exec, reinitializes everything and consequently calls the Engine.server block again - which leads to multiple tailwindcss processes.
• Since Kernel.exec is used, there's no way to maintain a flag like tailwindcss_process_started = true anywhere in the server process. Tailwindcss process doesn't detect changes in the server process.

Some of the approaches I'm thinking about:

• Use tmp/tailwindcss_pid.txt
• Leave this as-is, document this as a limitation/known bug.
• Killing existing tailwindcss subprocess(es) when running server block.

[bruno-]

@flavorjones this PR is ready for another look.

[flavorjones]

@bruno- Thanks for continuing your work on this, in particular the care you took around handling Puma restarts. I tried the code out on my local machine and it all seems to work for me.

It's a sizable chunk of new code, though, which is making me hesitate to ship it in this gem, especially since other gems like cssbundling-rails and dartsass-rails might also be able to use this approach to avoid the Procfile.dev/foreman hoop-jumping.

I'm also curious about test coverage; by any chance was this code adapted from an existing project that tests the edge cases (and if so, should we make sure to pay attention to any licensing requirements)?

I would consider putting the process-management code into a new gem, and taking a dependency on that gem, if we can both add some test coverage and use it in other gems. Or: is there an existing gem that does this already? (I searched briefly but wasn't satisfied with the results.)

@rafaelfranca we had a chat a few months back about providing some shared code for the asset builders. Any thoughts here?

[bruno-]

Hi @flavorjones

It's a sizable chunk of new code, though, which is making me hesitate to ship it in this gem

I get it. Let's take a step back and consider our options.

I'm also curious about test coverage; by any chance was this code adapted from an existing project

No, this code was not adapted from any existing project. I started looking at the code in lib/puma/plugin/tailwindcss.rb and took it from there. My only other references were docs.ruby-lang.org and stackoverflow 😅

...from an existing project that tests the edge cases

I, myself spent quite a bit of time manually testing this, discovering bugs etc.

should we make sure to pay attention to any licensing requirements?

I described my working process above. I confirm I'm the author of the code, we can license it however we want.

I would consider putting the process-management code into a new gem, and taking a dependency on that gem, if we can both add some test coverage and use it in other gems.

• I think extracting this into a new gem is a good idea 👍
• Yes, I can add tests.
• Yes, the gem could be used for other rails gems that need "watcher" processes.

Is there an existing gem that does this already?

I'm also not aware of any other gem.

[bruno-]

I'll extract this code to a gem and add tests.

The only question I have is: do you think "server process" is a good name for the gem, and this feature in general?

[dhh]

I'm not loving the heavy and TW specific ServerProcess implementation here. I think there's definitely something to the idea, but it can't be some TW bespoke thing. It needs to be a generic Rails feature that's then just used here.

[flavorjones]

Agree. @brunoprietog let me know if you'd like to collaborate on something we could use for Rails generally, happy to make time.

[brunoprietog]

Happy to collaborate here! But the author of this is @bruno-. It's a fun coincidence that we are both named Bruno 😂

[flavorjones]

Sorry about that! Autocomplete fail. :rofl:

[coorasse]

Has anyone already explored the option to have it integrated in the same rails process as a middleware? Basically what sprockets does? Sure, it would be slower, but you can always just run it in a separate thread a-la webpacker