Add websocket endpoint to advanced options #192
Conversation
|
Talked to @Ujstor about the generated websocket handler and they made the fair suggestion of changing the generated handler to be more in line with what websockets are typically used for; That is that they should be sending data without interaction of the client beyond the initial connection request. The initial idea is for the server to send a timestamp to the client every 0.5 seconds. |
|
During hand testing this feature the Footnotes
|
README.md
Outdated
|
|
||
| You can now use the `--advanced` flag when running the `create` command to get access to the following features. This is a multi-option prompt; one or more features can be used at the same time: | ||
|
|
||
| - [HTMX](https://htmx.org/) support using [Templ](https://templ.guide/) | ||
| - CI/CD workflow setup using [Github Actions](https://docs.github.com/en/actions) | ||
|
|
||
| - [Websocket](https://en.wikipedia.org/wiki/WebSocket) sets up a websocket endpoint |
There was a problem hiding this comment.
I think this needs to either link to some Go related source material and not just a wikipedia article on web sockets
There was a problem hiding this comment.
I could link the most used websocket library 1 in the generated code but that would not encompass fiber as an option as that is using it's own websocket implementation.
Is that okay with you?
Footnotes
cmd/program/program.go
Outdated
| importsPlaceHolder := "" | ||
| if p.AdvancedOptions["Websocket"] { | ||
| importsPlaceHolder += string(p.FrameworkMap[p.ProjectType].templater.WebsocketImports()) | ||
| } |
There was a problem hiding this comment.
Why would this be necessary if this function is only called when the use selects web socket as an option?
Can we omit the if?
There was a problem hiding this comment.
Ye good point, I think this is a left over if statement from the refactor I did before sending this up.
cmd/program/program.go
Outdated
|
|
||
| importTmpl, err := template.New("imports").Parse(importsPlaceHolder) | ||
| if err != nil { | ||
| log.Fatal(err) |
There was a problem hiding this comment.
Nit: I want to fix the error handling in go-blueprint.
Can we do something like
if err != nil {
log.Fatalf("function failed: %v", err)
}
cmd/program/program.go
Outdated
| if err != nil { | ||
| log.Fatal(err) | ||
| } |
| socket, err := websocket.Accept(w, r, nil) | ||
|
|
||
| if err != nil { | ||
| log.Print("could not open websocket") |
There was a problem hiding this comment.
We need to surface the error to understand what the error is
| log.Print("could not open websocket") | ||
| _, _ = w.Write([]byte("could not open websocket")) | ||
| w.WriteHeader(http.StatusInternalServerError) | ||
| return nil |
There was a problem hiding this comment.
Same here - we need to surface the error
There was a problem hiding this comment.
PR #184 is merged and introduced an inside flag feature. Some minor changes are needed so that the WS flag can be used.
Fixed wrong import Implemented websocket dependency import
MitchellBerend
force-pushed
the
feature/websocket-endpoints
branch
from
8a254f4
to
84054a5
Compare
Co-authored-by: Brian Barrow <briancbarrow@gmail.com>
By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.
Problem/Feature
Resolves #174
Description of Changes:
This PR adds an option to the
--advancedflag that let's users add a websocket endpoint to theirroutes.gofile automatically.#184 is still open so if that get's merged before this one, this pr will also include the flag option like that pr describes.
Checklist