-
-
Notifications
You must be signed in to change notification settings - Fork 277
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
[feat] implement notify proxy to reload browser #661
Conversation
Let me know if anything is needed. I plan add some example setups for reloading script in typical set. E.g. tailwindcss + npm build + templ + go live reload example. But I think that's better suited in the docs using another PR |
It looks great! Thanks a lot for this. I'd love to see an integration test where it starts a web server to listen to the POST request, runs the CLI If you haven't got time, I can do that though, just let me know! |
I can write a http recorder test to test the new route handle with the notify function call. Would that be enough? Probably much easier to write? |
Great idea. The main reason is to protect against future regressions. |
added a httptest.Server test. Hopefully I didn't miss anything. I find it wired that URL appears in the handler: https://github.com/a-h/templ/blob/main/cmd/templ/generatecmd/proxy/proxy.go#L29. I know it's being used inside the |
### Triggering hot reload from outside `templ generate --watch` | ||
|
||
If you want to trigger a hot reload from outside `templ generate --watch` (e.g. if you're using `air`, `wgo` or another tool to build, but you want to use the templ hot reload proxy), you can use the `--notify-proxy` argument. | ||
|
||
```shell | ||
templ generate --notify-proxy | ||
``` | ||
|
||
This will default to the default templ proxy address of `localhost:7331`, but can be changed with the `--proxybind` and `--proxyport` arguments. | ||
|
||
```shell | ||
templ generate --notify-proxy --proxybind="localhost" --proxyport="8080" | ||
``` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, this is a nice addition to the templ CLI, will be using this once it's released.
Maybe even replace --watch
?
I was thinking that a change to the explanation, to make it more clear, could help this feature be more utilized when it's released.
By specifying that --notify-proxy
takes priority over other flags, and will be exec just once, can help developers better understand the usage for this cmd.
Also, the first time I read this text, it felt like I should add --notify-proxy
as a flag to the templ generate --watch
cmd, which would end in an unexpected outcome. (perhaps just my thinking tho :P )
Could also mention that --notify-proxy
only works if --proxy
is used. You kinda already does this and it's somewhat self explanatory, but one extra sentence could clear up any misunderstandings?
PS: some of this can maybe also be applied to the --notify-proxy
CLI help text.
just some thoughts on my part.
fix #656