hot-reload feature: experience report

[veggiemonk]

Hey @a-h,

I tried the hot-reload you recommended. (see #144)

Workflow

# Install templ
go install github.com/a-b/templ/cmd/templ@latest

air -c .air.toml

# In another terminal

templ generate --watch --proxy="http://localhost:8080"

# modified a .templ file

Issues

Here are the issues I've encountered while trying out templ.

Issue 1 - Race condition between the proxy and the server

Generated code for 1 templates with 0 errors in 3.164459ms
Opening URL: http://127.0.0.1:8080
Proxying from http://127.0.0.1:7331 to target: http://127.0.0.1:8080
Proxy to target error: http: proxy error: dial tcp 127.0.0.1:8080: connect: connection refused
Proxy to target error: http: proxy error: dial tcp 127.0.0.1:8080: connect: connection refused

Workaround: Delay opening the browser

Issue 2 - Filtering on header Content-Type is a really bad idea.

text/html is not the only valid value for Content-Type header. For example, text/html; charset=utf-8.

Solution: replace contentType != "text/html" with !strings.HasPrefix(contentType, "text/html")

Issue 3 - The sending of event is triggered at the same time that the server is rebuilding which causes to miss the update.

workaround: delay the sending of the event

Issue 4 - Mandatory use of --cmd flag which is not really documented what is expected/allowed.

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

It was truncated to only one argument, i.e. go run. Once fixed, the previous go run . was not killed in time for the new one to start and got a port conflict.

Solution: just set a placeholder i.e. --cmd="echo" and move on.

Note: If a nonexistent command is given, on the first pass, it works fine. On the second, it segfaults.

Issue 5 - With all the time.Sleep added, it takes more than 5s reload. Very slow.

Gave up.

See all the code in the draft PR #145

Here is the project I used to test the hot-reload: https://github.com/veggiemonk/hot-templ

I should also mentioned that I had some issue with using --proxy="http://localhost:8080 and had to change it to --proxy="http://127.0.0.1:8080 because the ReverseProxy used the IPv6 localhost. 🤷 I'm not sure I knew what was happening.

By the way, I'm using M1 Pro with latest OS.

[a-h]

Thanks for taking the time to try out the feature and write it up. It's a new feature, and quite complex, hence the rough edges. Your testing is very useful.

On Issue 1 - Race condition between the proxy and the server, it's that the backend server isn't available yet (rather than the proxy).

Opening the browser uses some backoff to wait for the proxy to be ready to connect. time.Second is probably a bit too long for an initial start time tbh.

func openURL(url string) error {
	backoff := backoff.NewExponentialBackOff()
	backoff.InitialInterval = time.Second
	var client http.Client
	client.Timeout = 1 * time.Second
	for {
		if _, err := client.Get(url); err == nil {
			break
		}
		d := backoff.NextBackOff()
		fmt.Printf("Server not ready. Retrying in %v...\n", d)
		time.Sleep(d)
	}
	return browser.OpenURL(url)
}

Seems like it might be a good idea to add a round tripper inside the proxy to do a similar retry as per this: https://stackoverflow.com/questions/57317383/repeating-an-http-request-multiple-times-inside-a-reverse-proxy

Using backoff instead of a sleep should be more reliable, and faster.

2 - Good catch, definitely should be a prefix of text/html

3 - The sending of event is triggered at the same time that the server is rebuilding which causes to miss the update.

Do you mean sending the SSE to the browser to cause it to reload? I think that adding the retry to the proxy would also resolve this, because the proxy would hold open the request, and keep trying until the backend is up and running. That should also avoid the time.Sleep.

if args.Command != "" {
	fmt.Printf("Executing command: %s\n", args.Command)
	if _, err := run.Run(ctx, args.Path, args.Command); err != nil {
		fmt.Printf("Error starting command: %v\n", err)
	}
	// Send server-sent event.
	if p != nil {
		p.SendSSE("message", "reload")
	}
}

4 - Mandatory use of --cmd flag which is not really documented what is expected/allowed.

Ah, I see there's a bug that truncates the arguments to the cmd by mistake. It's not supposed to be mandatory though, I think moving the send outside of the if statement is all that's required, like this:

if args.Command != "" {
	fmt.Printf("Executing command: %s\n", args.Command)
	if _, err := run.Run(ctx, args.Path, args.Command); err != nil {
		fmt.Printf("Error starting command: %v\n", err)
	}
}
// Send server-sent event.
if p != nil {
	p.SendSSE("message", "reload")
}

5 - Waiting too long.

Yes, that's not good! 😁

I'll pick these up when I get the chance, or if you're up for contributing, let me know and I'll hang on and review.

[veggiemonk]

By all means, go ahead @a-h

I hope it didn't come too harsh. I re-read what I wrote and I'm sorry if this was lacking empathy. It was late and I was getting straight to the point. 😅

Note sure if you saw the changes I tried: https://github.com/a-h/templ/pull/145

Regarding 1. not sure what happened but it didn't work for me out-of-the-box.

Regarding 3. this is what I mean

                             nothing changed
templ -----------------------X---------------------
        ^    ^      ^    ^
        |    |      |     \ reloading browser
        |    |      |
        |    |       \_ sending SSE to browser
        |    |     
        |     \_ generate new files 
        |   
         \_ change to _.templ file

air   --------------------------------------------
                 ^     ^        ^
                 |     |        \ new build ready and serving
                 |     |
                 |      \_ building
                 |
                 \_ detect changes

[joerdav]

Closing as the hot-reload approach has drastically changed since this report.

[a-h]

Just to add some detail to this - the two step changes in performance are:

• fsnotify based watcher - https://github.com/a-h/templ/pull/470
• The update to --watch mode so recompilation is only required if you change Go code - https://github.com/a-h/templ/pull/366

There's a short video at https://github.com/a-h/templ/pull/470

The other issues you raised have been resolved.

Thanks for spending your time to put together the report!