a-h / templ

A language for writing HTML user interfaces in Go.
https://templ.guide/
MIT License
8.33k stars 274 forks source link

cmd: rebuild and refresh when a go file is changed in `--watch` mode #646

Open Giftzwerg02 opened 7 months ago

Giftzwerg02 commented 7 months ago

I have tried the example given in the docs: https://templ.guide/commands-and-tools/hot-reload

When trying to comment-out for example the "/" route, and then saving the main.go file, I don't see any reloads in my terminal and the route also still exists (when I refresh in the browser it still sends me the page).

I didn't find any related issues here and it's unclear to me how to debug this since no error or anything is logged.

The only thing that maybe causes this (given that it works for others (?)) is, that I am developing on NixOS.

I am using the following flake.nix:

{
  description = "Go proj flake";

  inputs.nixpkgs.url = "github:nixos/nixpkgs";  
  inputs.flake-utils.url = "github:numtide/flake-utils";

  outputs = { self, nixpkgs, flake-utils, ... }:
    flake-utils.lib.eachDefaultSystem (system:
      let
        pkgs = import nixpkgs { inherit system; };
      in
      {
        devShells.default = pkgs.mkShell
          {
            buildInputs = with pkgs;[
                go
                templ
                gcc
            ];
          };
      }
    );
}

I use it via nix develop and run the given example command: templ generate --watch --proxy="http://localhost:8080" --cmd="go run ."

joerdav commented 7 months ago

Templ only watches templ files, I recommend using templ watch in conjunction with air, similar to my example here:

https://github.com/joerdav/shopping-list

Giftzwerg02 commented 7 months ago

I see, I didn't know that; I thought it would also react to changes of go files since it is mentioned like this in the docs:

If the *.go files change, #3 and #4 must be ran.

Do they just mean the go-files generated by templ here?

a-h commented 7 months ago

Yes, just the Go files generated by templ, as per: https://github.com/a-h/templ/blob/ad6950151f9c0a33ff0c3123ac8128ded4d0dfe2/cmd/templ/generatecmd/watcher/watch.go#L57-L68

However, it would be no trouble to update it to monitor all *.go files... maybe that would be better. What do you think @joerdav?

joerdav commented 7 months ago

We could, my worry is that theres entire projects based on watching Go projects. I can imagine the templ watch tool gaining more complexity as we add more features that air covers.

Though, we could have a crude watch feature that covers 80% of cases, then recommend other tools if theres more niche scenarios.

Maybe I'm a bit biased since I'm very much enjoying the air+templ DX!

a-h commented 7 months ago

I think doing the 80%, and making sure we support the use of air and similar tools for advanced use cases is ideal.

I added the change to not do all files here: https://github.com/a-h/templ/pull/470/commits/dd1ee1e752c20db660a9af8fc0d99dea28a50995 but maybe I was being over-cautious with that...

I think I could change it to include *.go files (if the --cmd arg is also set) and not break anyone else's expected workflows. If you want to get air or something else to build the software, don't add the --cmd flag to templ generate and air can pick up the change.

joerdav commented 7 months ago

Makes sense, we could look at including a glob or regex param to help cover the 80%, unless you think that would be too much too soon?

jackielii commented 7 months ago

I stumbled across this with lots of search. I was using air to reload, but I miss the browser auto refresh feature. Initially I used browser-sync's proxy mode. However, I just couldn't reliably get the reload event to sent: It has to be after the server is restarted. In a way, I much prefer a polling method.

templ generate --watch gets most of them correctly: it refreshes the browser, although not instantly, but at least I don't have to switch over and cmd+r and switch back.

I don't mind the polling interval to be shorter as well, I'm fine trading more cpu with refresh speed.

Thanks & Regards

jackielii commented 7 months ago

Aha, actually found a pretty good enough solution:

templ generate --watch --proxy="http://localhost:8080" --cmd="wgo run ."

So wgo watches go files, and templ watches .templ files. All covered!

jackielii commented 7 months ago

Can I make one small suggestion:

Add templ generate --watch --notify to notify default listening port, i.e. 7331 a reload event

This makes other watcher to send the browser refresh event down easily.

Use case: I have tailwindcss watching files and generating css. Although this is mostly covered by changes in *.templ, but it's always lagging behind. I.e.

  1. *.templ changes
  2. templ regenerates _templ.txt and tailwindcss regenerates output.css
  3. usually templ is faster so the pages reload without the output.css change

If this feature is added, I could easily do a wgo -file output.css temple generate --watch --reload

jackielii commented 7 months ago

@a-h are you willing to accept a PR that adds templ generate --watch --notify to notify an update progmatically? Thanks

a-h commented 7 months ago

Thanks for writing that up into a nicely written up proposal @jackielii.

This issue can remain focused on whether templ should also support hot reload for *.go files.

I'm currently thinking that it should include *.go files by default, but have an option of --ignore-non-templ-go-files to switch it off for people that are using air / wgo etc.

jackielii commented 7 months ago

Thanks for writing that up into a nicely written up proposal @jackielii.

This issue can remain focused on whether templ should also support hot reload for *.go files.

I'm currently thinking that it should include *.go files by default, but have an option of --ignore-non-templ-go-files to switch it off for people that are using air / wgo etc.

I think there might be a problem with it:

Imagine if we include *.go files for restarting the --cmd. The sequence of events will go like so:

templ file change

  1. templ detects a .templ file change and re-creates _templ.txt
  2. templ proxy server sends an reload event and browser reloads
  3. browser requests the route and the go server responds with the newly generated _temple.txt

go file changes

  1. templ detects a .go file change and kills the --cmd process, templ send reload event?
  2. templ re-runs the --cmd process which is probably a go run process
  3. templ sends the browser reload event

So the question is should templ send the browser reload event on 1 or 3? Intuitively it should send reload event on step 3, but:

Even if templ didn't send the reload event, the browser will still send a request because the SSE connection will try to re-connect ref Event if templ tries to send the reload event, the browser client might not be connected.

The above are my suspicions, I didn't dig into them. However, the former does happen. You can test it in my setup https://github.com/a-h/templ/issues/667, I talked about my suspicion here: https://github.com/a-h/templ/discussions/596#discussioncomment-9012143

Finally, there is another case already handled, but worth mentioning:

When the go code related part of a templ file changes, the _templ.go changes, so the go server needs to be restarted. This is handled in templ generate's --cmd, and can be handled by air to include all go files. But I did make the mistake by excluding *_templ.go in air.

Update: After a few tests, I think sending reload event in 1 or 3 in the go file changes will both work: When the reload event is sent to the browser, the proxy server should be connected, so there is no "reconnect" happening here. The reload will hang for a bit to wait for the server to become available.

joerdav commented 5 months ago

I agree that it would be fine to trigger based on go files changing in the case of --cmd. We also now have thorough documentation on how to use air to support the templ watch workflow: https://templ.guide/commands-and-tools/live-reload-with-other-tools

Do we still want to add this feature?

joerdav commented 3 months ago

I think we are in agreement that we should be able to watch more than just templ files.

I've also received feedback that some would like to watch on more than just go and templ files. Use cases include embeded css files that require a rebuild if changed.

My suggestion is to switch to one flag:

--watch-pattern or similarly named.

The default of it would be *.(\.go|\.templ).

Air users could override to *.\.templ.

And users who want to build on a css change could do *.(\.go|\.templ|\.css).

We could avoid using regex if we wanted, opt for some kind of comma separated glob instead.

Ilyes512 commented 2 months ago

I also was going through https://templ.guide/commands-and-tools/live-reload and thought it would watch all my .go files and was surprised it didn't work as (I) expected.

So I was about to open an issue, but first searched the existing issues using watch and found this issue.

I think its a good decision to focus on the templ generated files and so I fully agree with that, but my suggestion would be to clearly mention this on the above doc page and also state that it is a conscious decision (maybe even mention this issue).

edit:

I created a PR with a possible suggestion for the note @ https://github.com/a-h/templ/pull/896 Feel free to edit it before merging of course.

sedyh commented 2 months ago

+1 for making templ to watch go files. I'm still confused how to use --watch and --notify-proxy together with wgo since both --watch and go run are blocking.

melkishengue commented 2 months ago

+1 would also like to have this feature, then i could only use templ to watch all my files and not use air anymore