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

Use with precompiled templates, eg. with Quicktemplate #239

Closed
frederikhors opened this issue Jul 29, 2019 · 5 comments
Closed

Use with precompiled templates, eg. with Quicktemplate #239

frederikhors opened this issue Jul 29, 2019 · 5 comments

Comments

@frederikhors
Copy link
Contributor

Issue opened for the creation of a wiki page that summarizes the doubts and problems for newbies (#210).


I'm trying to use authboss with quicktemplate but I'm having some problems.

If I read the code for html.go I can see Load() and Render() methods.

I started copying that code and if I understand correctly:

  • the Load() method: I think I don't need it (for my basic work). Am I wrong? Some comments of mine in the original code:
func (h *HTML) Load(names ...string) error {
    if h.layout == nil {
        b, err := loadWithOverride(h.overridePath, "html-templates/layout.tpl") // I use an interface for layout page with quicktemplate
        if err != nil {
            return err
        }
        h.layout, err = template.New("").Funcs(h.funcMap).Parse(string(b)) // I don't need parsing anymore
        if err != nil {
            return errors.Wrap(err, "failed to load layout template")
        }
    }
    for _, n := range names {
        filename := fmt.Sprintf("html-templates/%s.tpl", n) // this is already an object in my code, right?
        b, err := loadWithOverride(h.overridePath, filename)
        if err != nil {
            return err
        }
        clone, err := h.layout.Clone() // this is already an object in my code, right?
        if err != nil {
            return err
        }
        _, err = clone.New("authboss").Funcs(h.funcMap).Parse(string(b)) // this is already an object in my code, right?
        if err != nil {
            return errors.Wrapf(err, "failed to load template for page %s", n)
        }
        h.templates[n] = clone // maybe something like this with functions?
    }
    return nil
}
  • for the Render() method I can use something like this:
func (h *HTML) Render(ctx context.Context, page string, data authboss.HTMLData) (output []byte, contentType string, err error) {
	// -------> Original authboss one, commented now
	// buf := &bytes.Buffer{}
	// tpl, ok := h.templates[page]
	// if !ok {
	// return nil, "", errors.Errorf("template for page %s not found", page)
	// }
	// err = tpl.Execute(buf, data)
	// if err != nil {
	// return nil, "", errors.Wrapf(err, "failed to render template for page %s", page)
	// }

	// -------> Mine
	buf := &bytes.Buffer{}
	templates.WritePage(buf, &templates.LoginPage{
		Data: data,
	})
	return buf.Bytes(), "text/html", nil
}

which has the problem I cannot dynamically load pages in templates.WritePage() method based on page parameter.

LoginPage is coming from a template like this:

{% import "github.com/volatiletech/authboss" %}

{% code
    type LoginPage struct { Data authboss.HTMLData } 
%}

{% func (p *LoginPage) Title() %}
    Login
{% endfunc %}

{% func (p *LoginPage) Body() %}
    <b>Data: {%v p.Data.Something %}</b>
    <form action="/login" method="POST">
        <input type="email">
        <input type="password">
        <button>Login</button>
        {% if p.Data["modules"] != nil %} 
            Something else with modules...
        {% endif %}
    </form>
{% endfunc %}

Maybe with reflection? Really? I read everywhere reflection is really slow and I need to use something else if possible.

I tried also with something like this:

{% import "github.com/volatiletech/authboss" %}

{% code var ALL_TEMPLATES map[string]*LoginPage %}

{% code
    func init() {
        ALL_TEMPLATES = make(map[string]*LoginPage)
        ALL_TEMPLATES["login"] = &LoginPage{}
    }
%}

{% code
    type LoginPage struct { Data authboss.HTMLData } 
%}

{% func (p *LoginPage) Title() %}
    Login
{% endfunc %}

{% func (p *LoginPage) Body() %}
    <b>Data: {%v p.Data.Something %}</b>
    <form action="/login" method="POST">
        <input type="email">
        <input type="password">
        <button>Login</button>
        {% if p.Data["modules"] != nil %} 
            Something else with modules...
        {% endif %}
    </form>
{% endfunc %}

but I think something is wrong here. I don't like ALL_TEMPLATES slice way of doing this.

What do you suggest?


I already opened an issue on quicktemplate repo: valyala/quicktemplate#58.

@aarondl
Copy link
Member

aarondl commented Jul 31, 2019

I'd say that your problem is that you're not using the Load() stuff correctly. Load should compile the template and store them in a map[string]template or what have you. That way when Render is called, the right template can be referenced.

@frederikhors
Copy link
Contributor Author

frederikhors commented Jul 31, 2019

As I have already written here: valyala/quicktemplate#58 (comment)

What have I done

Now I'm using authboss.Renderer interface's Load() method:

type HTML struct {
  ...
  templates    map[string]func(authboss.HTMLData) templates.PageImpl
  ...
}

func InitializeLoginPageType(data authboss.HTMLData) templates.PageImpl {
	return &templates.LoginPage{Data: data}
}

func (h *HTML) Load(names ...string) error {
	for _, n := range names {
		switch n {
		case "login":
			h.templates[n] = InitializeLoginPageType
		}
	}
	return nil
}

func (h *HTML) Render(ctx context.Context, page string, data authboss.HTMLData) (output []byte, contentType string, err error) {
	buf := &bytes.Buffer{}
	tpl, ok := h.templates[page]
	if !ok {
		return nil, "", errors.Errorf("...")
	}
	templates.WritePage(buf, tpl(data))
	return buf.Bytes(), "text/html", nil
}

The million dollar question

Do you see problems with this code?

Load should compile the template

Is my final code correct using it with quicktemplate precompiled templates?
Do you see parts to improve?

@aarondl
Copy link
Member

aarondl commented Aug 1, 2019

This seems fine to me. Though I would not use the lambda. I'd probably try to use an interface and create all the structs inline in the switch case. :)

@aarondl aarondl closed this as completed Aug 1, 2019
@frederikhors
Copy link
Contributor Author

frederikhors commented Aug 2, 2019

Your advice is invaluable, thank you very much.

I am still learning Go and every problem is an opportunity to learn, but only if I am "guided" by skilled and generous people like you.

I have created two different versions of the problem and would like to know which one is best or if there is a third one better.

  1. https://github.com/frederikhors/authbossQuicktemplate_1, factory pattern and initializators

  2. https://github.com/frederikhors/authbossQuicktemplate_2, with interface and a SetData(data authboss.HTMLData) (page PageImpl) method

I think you prefer the latter, but I don't know what to improve for performances.

  • Is it possible to do the same thing differently and less in terms of hardware resources?

  • Do you think I can improve using pointers somewhere?

@aarondl
Copy link
Member

aarondl commented Aug 20, 2019

Second one seems fine. Remember I'm not a code review service ;) I just make the library which is enough work on it's own. I think you're doing fine.

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

No branches or pull requests

2 participants