From da8bf3890cf0e49545084959a21c62b17c6ac6f4 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Fri, 8 Aug 2025 18:00:59 +0200 Subject: [PATCH] refactor(templates): be explicit about dependencies Instead of blindly compiling all the common/ templates for every view/ ones, let's be explicit about the dependencies. This should significantly decrease the resident memory consumption, as ParseTemplate is responsible for ~10M of the current 11M of heap memory on my instance, so any win there is interesting. This will also allow better factorization of templates, now that everything is explicit. Another side-effect is that it'll make testing easier, as we now have a comprehensive list of views/ templates affected by a change in a file in common/ --- internal/template/engine.go | 80 ++++++++++++------- .../{standalone => views}/offline.html | 0 internal/ui/ui.go | 4 +- 3 files changed, 50 insertions(+), 34 deletions(-) rename internal/template/templates/{standalone => views}/offline.html (100%) diff --git a/internal/template/engine.go b/internal/template/engine.go index da15cec1..27e18080 100644 --- a/internal/template/engine.go +++ b/internal/template/engine.go @@ -7,7 +7,6 @@ import ( "bytes" "embed" "html/template" - "log/slog" "time" "miniflux.app/v2/internal/locale" @@ -21,9 +20,6 @@ var commonTemplateFiles embed.FS //go:embed templates/views/*.html var viewTemplateFiles embed.FS -//go:embed templates/standalone/*.html -var standaloneTemplateFiles embed.FS - // Engine handles the templating system. type Engine struct { templates map[string]*template.Template @@ -38,43 +34,66 @@ func NewEngine(router *mux.Router) *Engine { } } -// ParseTemplates parses template files embed into the application. -func (e *Engine) ParseTemplates() error { +func (e *Engine) ParseTemplates() { funcMap := e.funcMap.Map() - commonTemplates := template.Must(template.New("").Funcs(funcMap).ParseFS(commonTemplateFiles, "templates/common/*.html")) - - dirEntries, err := viewTemplateFiles.ReadDir("templates/views") - if err != nil { - return err + templates := map[string][]string{ // this isn't a global variable so that it can be garbage-collected. + "about.html": {"layout.html", "settings_menu.html"}, + "add_subscription.html": {"feed_menu.html", "layout.html", "settings_menu.html"}, + "api_keys.html": {"layout.html", "settings_menu.html"}, + "bookmark_entries.html": {"item_meta.html", "layout.html", "pagination.html"}, + "categories.html": {"layout.html"}, + "category_entries.html": {"item_meta.html", "layout.html", "pagination.html"}, + "category_feeds.html": {"feed_list.html", "layout.html"}, + "choose_subscription.html": {"feed_menu.html", "layout.html"}, + "create_api_key.html": {"layout.html", "settings_menu.html"}, + "create_category.html": {"layout.html"}, + "create_user.html": {"layout.html", "settings_menu.html"}, + "edit_category.html": {"layout.html", "settings_menu.html"}, + "edit_feed.html": {"layout.html"}, + "edit_user.html": {"layout.html", "settings_menu.html"}, + "entry.html": {"layout.html"}, + "feed_entries.html": {"item_meta.html", "layout.html", "pagination.html"}, + "feeds.html": {"feed_list.html", "feed_menu.html", "item_meta.html", "layout.html", "pagination.html"}, + "history_entries.html": {"item_meta.html", "layout.html", "pagination.html"}, + "import.html": {"feed_menu.html", "layout.html"}, + "integrations.html": {"layout.html", "settings_menu.html"}, + "login.html": {"layout.html"}, + "offline.html": {}, + "search.html": {"item_meta.html", "layout.html", "pagination.html"}, + "sessions.html": {"layout.html", "settings_menu.html"}, + "settings.html": {"layout.html", "settings_menu.html"}, + "shared_entries.html": {"layout.html", "pagination.html"}, + "tag_entries.html": {"item_meta.html", "layout.html", "pagination.html"}, + "unread_entries.html": {"item_meta.html", "layout.html", "pagination.html"}, + "users.html": {"layout.html", "settings_menu.html"}, + "webauthn_rename.html": {"layout.html"}, } - for _, dirEntry := range dirEntries { - fullName := "templates/views/" + dirEntry.Name() - slog.Debug("Parsing template", slog.String("template_name", fullName)) - commonTemplatesClone, err := commonTemplates.Clone() - if err != nil { - panic("Unable to clone the common template") + + for name, dependencies := range templates { + tpl := template.New("").Funcs(funcMap) + for _, dependency := range dependencies { + template.Must(tpl.ParseFS(commonTemplateFiles, "templates/common/"+dependency)) } - e.templates[dirEntry.Name()] = template.Must(commonTemplatesClone.ParseFS(viewTemplateFiles, fullName)) + e.templates[name] = template.Must(tpl.ParseFS(viewTemplateFiles, "templates/views/"+name)) } - dirEntries, err = standaloneTemplateFiles.ReadDir("templates/standalone") - if err != nil { - return err + // Sanity check to ensure that all templates are correctly declared in `templates`. + if entries, err := viewTemplateFiles.ReadDir("templates/views"); err == nil { + for _, entry := range entries { + if _, ok := e.templates[entry.Name()]; !ok { + panic("Template " + entry.Name() + " isn't declared in ParseTemplates") + } + } + } else { + panic("Unable to read all embedded views templates") } - for _, dirEntry := range dirEntries { - fullName := "templates/standalone/" + dirEntry.Name() - slog.Debug("Parsing template", slog.String("template_name", fullName)) - e.templates[dirEntry.Name()] = template.Must(template.New(dirEntry.Name()).Funcs(funcMap).ParseFS(standaloneTemplateFiles, fullName)) - } - - return nil } // Render process a template. func (e *Engine) Render(name string, data map[string]any) []byte { tpl, ok := e.templates[name] if !ok { - panic("This template does not exists: " + name) + panic("The template " + name + " does not exists.") } printer := locale.NewPrinter(data["language"].(string)) @@ -89,8 +108,7 @@ func (e *Engine) Render(name string, data map[string]any) []byte { }) var b bytes.Buffer - err := tpl.ExecuteTemplate(&b, "base", data) - if err != nil { + if err := tpl.ExecuteTemplate(&b, "base", data); err != nil { panic(err) } diff --git a/internal/template/templates/standalone/offline.html b/internal/template/templates/views/offline.html similarity index 100% rename from internal/template/templates/standalone/offline.html rename to internal/template/templates/views/offline.html diff --git a/internal/ui/ui.go b/internal/ui/ui.go index 4784d606..37c37ada 100644 --- a/internal/ui/ui.go +++ b/internal/ui/ui.go @@ -19,9 +19,7 @@ func Serve(router *mux.Router, store *storage.Storage, pool *worker.Pool) { middleware := newMiddleware(router, store) templateEngine := template.NewEngine(router) - if err := templateEngine.ParseTemplates(); err != nil { - panic(err) - } + templateEngine.ParseTemplates() handler := &handler{router, store, templateEngine, pool}