labstack / echo

High performance, minimalist Go web framework
https://echo.labstack.com
MIT License
29.01k stars 2.21k forks source link

Using static middleware, does not raise handler at '/*' path #2630

Closed mehranzand closed 2 months ago

mehranzand commented 2 months ago

Issue Description

When I use the middleware static to embed react project the home handler for example GET or Any does not raise the event but the other paths does.

func New() *echo.Echo {
    e := echo.New()

    e.GET("*", IndexHandler, middleware.StaticWithConfig(middleware.StaticConfig{
        HTML5:      true,
        Filesystem: http.FS(web.DistDirFS),
    }))

}

Checklist

Expected behaviour

IndexHandler will be called at path '/'

Actual behaviour

Nothing

aldas commented 2 months ago

Paths * and /* are the same. When registering routes with same path the last one used when routing is done.

aldas commented 2 months ago

and you probably need to set Root folder for your DistDirFS

    e.GET("*", IndexHandler, middleware.StaticWithConfig(middleware.StaticConfig{
        Root:       "path-in-web.DistDirFS",
        HTML5:      true,
        Filesystem: http.FS(web.DistDirFS),
    }))
mehranzand commented 2 months ago

thanks for your quick reply. IndexHandler does not called when fetching home landing for example localhost:7070 but the other paths (localhost:7070/containers) it will called properly. I was set Root path in static middleware but it not managed to solve my issue.

aldas commented 2 months ago

Try adding static MW with Use

    e.Use(middleware.StaticWithConfig(middleware.StaticConfig{
        Root:       "path-in-web.DistDirFS",
        HTML5:      true,
        Filesystem: http.FS(web.DistDirFS),
    }))
mehranzand commented 2 months ago

I tried this method first but not resolve my issue.

aldas commented 2 months ago

Is that IndexHandler returning something dynamic or static content?

mehranzand commented 2 months ago

It just reading index.html and execute some template on that then using Context.Response.Writer to apply result.

func IndexHandler(c echo.Context) error {
    data := map[string]interface{}{
        "Config": "config json",
    }

    file, err := web.DistDirFS.Open("index.html")
    if err != nil {
        log.Panic(err)
    }
    bytes, err := io.ReadAll(file)
    if err != nil {
        log.Panic(err)
    }

    tmpl, err := template.New("index").Parse(string(bytes))
    if err != nil {
        log.Panic(err)
    }

    err = tmpl.Execute(c.Response().Writer, data)
    if err != nil {
        log.Panic(err)
        http.Error(c.Response().Writer, err.Error(), http.StatusInternalServerError)
    }

    return nil
}
aldas commented 2 months ago

Lets start with this small example

package main

import (
    "embed"
    "errors"
    "github.com/labstack/echo/v4"
    "github.com/labstack/echo/v4/middleware"
    "log"
    "net/http"
)

//go:embed website
var assets embed.FS

func main() {
    e := echo.New()

    e.Use(middleware.Recover())
    e.Use(middleware.Logger())

    indexHandler := func(c echo.Context) error {
        return c.String(http.StatusOK, "HI") // replace with your rendering handler
    }

    e.GET("/", indexHandler)
    e.GET("/*", indexHandler, middleware.StaticWithConfig(middleware.StaticConfig{
        Root:       "website",
        HTML5:      true,
        Filesystem: http.FS(assets),
    }))

    g := e.Group("/api")
    g.GET("/test", func(c echo.Context) error {
        return c.String(http.StatusOK, "OK")
    })

    if err := e.Start(":8080"); err != nil && !errors.Is(err, http.ErrServerClosed) {
        log.Fatal(err)
    }
}

website is folder containing you html/css etc files. For example lets say that folder contains test.html then curl http://localhost:8080/test.html would serve that file. curl http://localhost:8080/xxx.html would serve indexHandler

mehranzand commented 2 months ago

it's works! there was some problem in embed FS reference. it was solved thank you.

aldas commented 2 months ago

very good.

NB: you should not leave that file read and template executing part in that function or at least wrap it into sync.Once. At the moment you are reading that file and parsing the template for every request and this is a lot of unnecessary work that could be done once.

mehranzand commented 2 months ago

I think every time the static page served by echo should render the index.html to inject the config json into react app.So it will be good. Isn't?

aldas commented 2 months ago

for effiency I suggest moving these parts out of handler so these will not be executed for every request.

    file, err := web.DistDirFS.Open("index.html")
    if err != nil {
        log.Panic(err)
    }
    bytes, err := io.ReadAll(file)
    if err != nil {
        log.Panic(err)
    }

    tmpl, err := template.New("index").Parse(string(bytes))
    if err != nil {
        log.Panic(err)
    }

It seems to me that only thing that needs to be executed for every request is err = tmpl.Execute(c.Response().Writer, data)

mehranzand commented 2 months ago

Thank you, I'll definitely take it into consideration.