Skip to content
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

Add live proxy feature #512

Merged
merged 6 commits into from
May 5, 2024
Merged

Add live proxy feature #512

merged 6 commits into from
May 5, 2024

Conversation

ndajr
Copy link
Contributor

@ndajr ndajr commented Jan 5, 2024

Description

This PR implements what is discussed in this issue: #141. I started a new webapp using Go templates and would like to have a live reloading functionality (not just hot reloading when files change). The two possible implementations are with websockets or with server-sent events. I chose the latter because it adds the least amount of complexity and no external dependency is required. I'd like some feedback on the config names (port and app_port, or frontend_port and backend_port?) and whether proxy should be enabled by default or not.

How to use

It is quite simple to use, add the proxy config, in my case I added my views containing the go templates on include_dir, and access the proxy port, e.g. http://localhost:8090.

How it works

There are two handlers on the proxy, the root handler / which is a catch-all and should proxy all requests and the /internal/reload endpoint which implements server-sent events. The root handler proxies all the requests and injects a script that listens to /internal/reload and reloads the browser every time air watches a file change.

@scottmckendry
Copy link

@ndajr I'm trying to test this out in a dockerized chi app. Could I please have some clarification on how this should work?

I've updated my dockerfile to point to your branch and install air as per below:

FROM golang:1.21

WORKDIR /jacc
COPY . /jacc/
RUN apt-get update && apt-get install wkhtmltopdf -y
RUN go mod download
RUN go mod verify
RUN go install github.com/a-h/templ/cmd/templ@latest
RUN git clone https://github.com/ndajr/air ~/.air && cd ~/.air && go install . && cd -
CMD air -c .air.toml

and I've updated my .air.toml file with the following:

[proxy]
  enabled = true
  proxy_port = 8080
  app_port = 3000

I've opened up both ports in my docker-compose. My chi app listens on port 3000. Tested live reloading on port 3000 - updated a file, watched air recompile and restart the server but no refresh on the browser side. The changes only appear after a manual refresh. Port 8080 is unresponsive.

Any idea what I could be doing wrong?

@ndajr
Copy link
Contributor Author

ndajr commented Jan 8, 2024

Hey @scottmckendry, I have changed the proxy config to accept urls not just ports, so that it works also with docker compose. Please try this: https://github.com/cosmtrek/air/pull/512/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R214

@scottmckendry
Copy link

Thanks @ndajr but I'm not convinced that was the issue. I'm running the container locally so using the default http://localhost URL was probably fine.

Either way, I've updated the .air.toml config as per your documentation. But still no difference, unfortunately. Perhaps I need some clarity on expected behaviour. If my app is listening on port 3000 and the proxy is configured to port 8080, which URL should I be using to access the app?

What should I be expecting to see after air reloads?

@ndajr
Copy link
Contributor Author

ndajr commented Jan 9, 2024

@scottmckendry you should access the proxy url, in your case it's 8080. Proxy will try to call your chi app on port 3000 and you should see the exact same page with an added script before the body html tag, which does the live reloading without you having to change the code of your app. You can try to access it outside of docker and see if it works, you can post here the full docker-compose and Dockerfiles and I can try to reproduce

@scottmckendry
Copy link

Hi @ndajr 👋

I've made some progress on my end. Turns out I was cloning the wrong branch in my dockerfile. Adding -b feat-live-proxy resolved that. Now I'm actually getting the log entry for the proxy server starting up.

Sadly, I'm still getting no response on the proxy URL. I suspect this is due to my docker config. I've pushed my current setup to the air-proxy-reload branch in the repo if you want to take a look.

Appreciate your help 🙂

@ndajr
Copy link
Contributor Author

ndajr commented Jan 13, 2024

Hi @scottmckendry, I think now I've got more clarity on what the issue was. Althougth listening on localhost:<port> is more convenient locally and avoids the message "Do you want the application “air” to accept incoming network connections?", it doesn't work with docker, it has to listen on :<port>. Sorry for the back and forth, please revert the config to this:

[proxy]
  enabled = true
  proxy_port = 8080
  app_port = 3000

I had to deal with a lot more edge cases parsing the full url and it wasn't worth it. My branch is updated, let me know how it goes.

@scottmckendry
Copy link

Brilliant! That's working perfectly now. Great stuff.
Hope this gets merged soon, it's a really neat feature👍🏻

@zolrath
Copy link

zolrath commented Jan 27, 2024

When using htmx to perform an update with partials the proxy fails due to the lack of body tag in the response.

Clicking this button:

<button hx-get="/user/more" hx-target="#data">
   Load fresh data
</button>

<div id="data"></div>

Calls an endpoint which returns this partial:

package user

templ More() {
  <p>Injected partial</p>
}

Which causes this error:

{"time":"2024-01-26T17:58:57.853555007-08:00","id":"","remote_ip":"[::1]:41856","host":"localhost:3000","method":"GET","uri":"/user/more","user_agent":"Go-http-client/1.1","status":200,"error":"","latency":5751,"latency_human":"5.751µs","bytes_in":0,"bytes_out":0}
2024/01/26 17:58:57 invalid html page, missing the body tag

@ndajr
Copy link
Contributor Author

ndajr commented Jan 29, 2024

Hi @zolrath, thanks for testing the feature with htmx! Initially I was locating the end of the body tag just to know where to insert the live reload script. As we don't want each partial template to be listening to live reload events, just the main page, I pushed a change to proxy the original fragment in case the body tag is missing (instead of erroring), so only the main page will do live reloading.

@zolrath
Copy link

zolrath commented Jan 29, 2024

Hm, my form POST doesn't seem to work with the latest version of the proxy.

The most obvious difference between the two results I'm about to post are the bytes_in property

When I submit a form with an hx-post to an endpoint with go run the results are:

{
  "time": "2024-01-28T23:45:29.537573732-08:00",
  "id": "",
  "remote_ip": "::1",
  "host": "localhost:3000",
  "method": "POST",
  "uri": "/win",
  "user_agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/121.0.0.0 Safari/537.36 Edg/121.0.0.0",
  "status": 200,
  "error": "",
  "latency": 46378798,
  "latency_human": "46.378798ms",
  "bytes_in": 15,
  "bytes_out": 0
}

If I am running it through the air proxy:

{
  "time": "2024-01-28T23:46:26.249693344-08:00",
  "id": "",
  "remote_ip": "[::1]:45426",
  "host": "localhost:3000",
  "method": "POST",
  "uri": "/win",
  "user_agent": "Go-http-client/1.1",
  "status": 200,
  "error": "",
  "latency": 9799,
  "latency_human": "9.799µs",
  "bytes_in": 0,
  "bytes_out": 0
}

@ndajr
Copy link
Contributor Author

ndajr commented Feb 2, 2024

Hi @zolrath, sorry for the delay, this should be fixed by now. Let me know how it goes

@zolrath
Copy link

zolrath commented Feb 2, 2024

It seems to POST properly now!
Though, the proxy live reload seems to be appending the original url to the end of the proxy url instead of simply the path so it never actually reloads.

image

@ndajr
Copy link
Contributor Author

ndajr commented Feb 2, 2024

@zolrath, good finding, I pushed yet another fix, this time actually simplifying the reload script to simply refresh the page which should be more reliable. On the other hand, you will need some hx-get with hx-trigger="load" on your dynamic content, because after refresh the DOM changes made from javascript (htmx in this case) will be gone. I created myself an htmx small project and it works fine, I hope it works for you as well.

@evanlurvey
Copy link

@ndajr cool feature! Are you familiar with the httputil package? There is a reverse proxy that does a lot of the heavy lifting for you.

@zoop-btc
Copy link

I tried out this branch and it works, but since I don't want to compromise on my port I put the refresh script in manually.
It runs into a cors issue that way. Can you fix this?
Doesn't happen in templs proxy btw, which does the same thing as this.

@Dieman89
Copy link

Is there any update on this?

@ndajr
Copy link
Contributor Author

ndajr commented Apr 25, 2024

I'm happy to move this feature forward, do adjustments or fixes (like CORS or httputil mentioned above), but first I'd like to hear from @cosmtrek or one of the maintainers if they want this

@popisdead
Copy link

This is the killer feature. Imagine having hot reload without live reload and press f5 for every tailwind tweak...

@jkeddari
Copy link

i use this branch, and it's work perfectly to refresh go-htmx-templ project with tailwindcss rebuild.
It's a game changer !

@avilesj
Copy link

avilesj commented Apr 28, 2024

This is amazing work @ndajr

@xiantang xiantang self-requested a review April 28, 2024 12:40
@xiantang
Copy link
Collaborator

well done! I will review this PR within next week.

Copy link

codecov bot commented May 5, 2024

Codecov Report

Attention: Patch coverage is 60.97561% with 48 lines in your changes are missing coverage. Please review.

Project coverage is 67.53%. Comparing base (56d3d58) to head (3809528).
Report is 8 commits behind head on master.

Files Patch % Lines
runner/proxy.go 56.52% 35 Missing and 5 partials ⚠️
runner/engine.go 11.11% 5 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #512      +/-   ##
==========================================
- Coverage   69.22%   67.53%   -1.70%     
==========================================
  Files           9       12       +3     
  Lines        1082     1078       -4     
==========================================
- Hits          749      728      -21     
- Misses        253      262       +9     
- Partials       80       88       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@xiantang
Copy link
Collaborator

xiantang commented May 5, 2024

LGTM. WEEEEELL done!!!

@xiantang xiantang merged commit 8284803 into air-verse:master May 5, 2024
8 of 10 checks passed
@xiantang
Copy link
Collaborator

xiantang commented May 5, 2024

I learnt a lot from your PR. tks

@ndajr ndajr deleted the feat-live-proxy branch May 5, 2024 17:40
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

Successfully merging this pull request may close these issues.

None yet