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

Feedback, Inputs & Proposals #35

Closed
aight8 opened this issue Jan 23, 2022 · 19 comments
Closed

Feedback, Inputs & Proposals #35

aight8 opened this issue Jan 23, 2022 · 19 comments

Comments

@aight8
Copy link

aight8 commented Jan 23, 2022

I really like templ. I hope it have a great future.
I have made some notes and thoughts while using it which I want to share now - I write it compact, but let's discuss about some points if you want.

1. Pointer Receiver Templates

I commonly use structs as ViewModels. Instead wire up all of those with it's render function manually this could help a lot:

{% templ (x ViewModel) MyTempl() %}

I used this pattern very much in quicktemplate to make all kind of structures "renderable". Currently in templ I have to create a struct method manually and invoke the render function, for every piece of template.

2. Autocompletion

{%! $ %} // cursor

In this scenario a Component is expected. All functions in the current package, which return a Component are possible candidates.

3. Implicit Package selection

This line of code is most time redundant because the package name can be derrived by the directory name.
When using templ on daily basis I forget this line sometimes.
Also, it is not adjusted by the IDE if the name of the package name changes (i.e. the directory name).

{% package my_pkg %}

4. Comments

Comment out quickly some code like {% comment %} in quicktemplate.
What about HTML comment? Or {{/* a comment */}}

5. Alternative START/END-Tag

This point is mabye a matter of taste. On swiss german keyboard, and maybe a lot others, this {% and %} is arduous to write. And you do it a lot when writing templates.
Maybe the {{ and }} which are used in regular go templates is not bad at all.

6. Required Space after START-Tag and before END-Tag

The space after the {% or before the %} took some getting used to. Strict on the one hand, prone to error on the other.
Perhaps the parser could point out this error more clearly - or even autofix it (like go fmt for templ).

7. The Context

A really happy that a context I passed arround the Component Render methods.
This way I can access global data or a localizer helper.
But this is undocumented feature and somehow not so nice.

{% if getUser(ctx) != nil %}
   Username: {% getUser(ctx).Username %}
   Age: {% getLocalizer(ctx).FormatNumber(getUser(ctx).Height) %}
{% endif %}

I have no concrete implementation-idea currently. I make some thoughts...
But in the abstract a shortcut to access a global "struct" variable would do the job.

{% if .user != nil %}
   Username: {% .user.Username %}
   Age: {% .i18n.FormatNumber(.user.Height) %}
{% endif %}

An easier implementation would be:
A pointer to a struct could be passed via context value, while templ make it visible in a variable x.

{% if x.user != nil %}
   Username: {% x.user.Username %}
   Age: {% x.i18n.FormatNumber(x.user.Height) %}
{% endif %}

In both cases the qualified-id must be specified somewhere (for generation and LSP)

Don't get me wrong, I'm not a make it all global type of guy. But it reduces all this argument passtroughts of really important things (i18n, basic user info etc.).
It's just a third source of (typed) data beside receiver variable + argument variables.

8. The Writer

If I'm not mistaken, virtual calls are 5 times slower than direct method calls. And a template calls it thousands of times including the error check.
A benchmark might help.
Maybe it would be interesting to have a look at the implementation of Quicktemplate, where among other things an acquirable byte buffer comes into play.
Another idea would be to be able to define a writer type via CLI argument.
If I manually implement templ components in go, the io.Writer interface is a bit tedious to work with directly.

9. Switch case improvement

Instead:

{% switch p.Type %}
	{% case "test" %}
		<span>{%= "Test user" %}</span>
	{% endcase %}
	{% case "admin" %}
		<span>{%= "Admin user" %}</span>
	{% endcase %}
	{% default %}
		<span>{%= "Unknown user" %}</span>
	{% enddefault %}
{% endswitch %}

What about:

{% switch p.Type %}
	{% case "test" %}
		<span>{%= "Test user" %}</span>
	{% case "admin" %}
		<span>{%= "Admin user" %}</span>
	{% default %}
		<span>{%= "Unknown user" %}</span>
{% endswitch %}

10. Range improvement

Instead:

{% if len(p.Addresses) == 0 %}
   <span>No addresses</span>
{% endif %}
{% for _, v := range p.Addresses %}
	<li>{%= v.City %}</li>
{% endfor %}

What about:

{% for _, v := range p.Addresses %}
	<li>{%= v.City %}</li>
{% default %}
   <span>No addresses</span>
{% endfor %}

This construct is rarely found in template languages. To be honest, I have no idea why.

11. Else-If support

Mabye the switch is a better alternative anyway when it have at least the same capacibilities.

12. Compile a specific file only

Currently recursively all templ files are compiled.
I configured my IDE or modd to autocompile templ files on every change.

@a-h
Copy link
Owner

a-h commented Jan 23, 2022

Thanks so much for the feedback, there's some great ideas in here.

Please let me take a bit of time to think about some of these before I get back to you. However, I can give some immediate thoughts right away...

    1. Implicit Package selection

Totally agree with you on this - I've forgotten to do this myself on more than one occasion. It would be easy to get templ fmt to use name of the directory as the package if it's missing, and add it in.

    1. Comments

Good idea on comments, I know it's something that's lacking from my own use. I hadn't settled on an idea for how it would look yet. The {% comment %} idea looks good, but I do like the idea of using something from Go syntax or allowing HTML comments. I'm looking for something that people would guess at, and just be right the first time. I'm just not sure what that is without doing a bit of research first (i.e. asking people to guess at how you do a comment, and looking at what the most popular templating languages do).

I'll have a chat with some users and canvas some more feedback here.

    1. Alternative START/END-Tag

I've been using templ a lot with a team recently to build email templates, and found the start / end syntax a bit out-of-the-way too, on an English keyboard.

I chose {% and {%= as the start for templ, because originally I was looking to see whether I could implement an LSP for quicktemplate and that's what it uses.

I also thought that since { is a common character in Go code, it wouldn't be a great thing to use, especially since {{ and }} can appear in valid (albeit contrived) Go code, e.g. {{ callFunction([]struct{Name string}{{Name: "example"}}) }} so I thought it could lead to some parser complexity.

The built-in html/template in Go uses {{ but it doesn't support Go expressions in the templates. I thought that using something different would make it clear that it's not Handlebars, Hugo templates or anything else.

At the moment, once the templ start of expression token {%= is found, the parser just reads to the next %} and assumes anything in there is Go code.

This means that Go code inside templ expressions isn't parsed at all... but it could be interesting to try parsing the code instead.

I used goreplace -o=".*.go$" --no-group --no-colors {{ | wc -l on the Go source code, and got 3218 instances, so it looks like a reasonably common pattern (not sure how common outside of test code though).

I'll need to think about this some more. If we were to support an alternative, I'd want to get templ fmt to rewrite old code so there's only one way to write templ expressions.

Any ideas you've got here would be great.

    1. Required Space after START-Tag and before END-Tag

Yeah, that's just annoying isn't it. What was I thinking!? :)

    1. The Writer

I'm not sure what you mean by a virtual call. Do you mean that the io.Writer is an interface, and not a concrete type?

    1. Compile a specific file only

Yes, that would be easy enough.

@joerdav
Copy link
Collaborator

joerdav commented Jan 28, 2022

I can see the benefit of the pointer receiver templates, and could see another Go developer assuming this was possible while writing templ code.
At the same time though I really like how the generated Go code is fairly self-contained, it just uses types outside that exist outside of it, allowing this means you're allowing templ code to alter the interface of a struct.
My view would be that having the receiver explicitly set for view models isn't too cumbersome:

func (x ViewModel) View() templ.Component { return myTemplate(x) } 

@aight8
Copy link
Author

aight8 commented Jan 28, 2022

btw: that is exactly how I use it now.
I have 1-2 dozens of page templates, 1 layout template, and aprox. 100 renderable components/helpers etc.
I name a file like page_a.go another one page_a.templ which generates page_a.templ.go (I made this naming adjustment in a fork)
page_a.go contains a PageA struct which contains variables this page needs. page_a.templ contains the view.
Regarding all the small view components:
I ended up implement all the small components by hand, I use a helper library which make that work also comfortable.
Especially when more go logic is required, templ files are too limited.
But I'm very happy with this workflow, because bigger templates really profit of templ files, and templ also provides the "framework".

@a-h a-h mentioned this issue Mar 13, 2022
2 tasks
@a-h
Copy link
Owner

a-h commented Mar 13, 2022

I've just added a feature for implicit package selection, as per your suggestion, see #36

a-h added a commit that referenced this issue Mar 19, 2022
* reduce strictness of space checks

* test flexible white space handling of parsers

* add missing endtempl spacing
@a-h
Copy link
Owner

a-h commented Mar 20, 2022

@aight8 - implicit package selection and flexible spacing are now available in version https://github.com/a-h/templ/releases/tag/v0.0.162

@a-h
Copy link
Owner

a-h commented Mar 20, 2022

And I've added a new flag to templ generate so that you can generate a single file, e.g. templ generate -f example.templ in https://github.com/a-h/templ/releases/tag/v0.0.164

@aight8
Copy link
Author

aight8 commented Mar 21, 2022

Is it possible to implement "template as struct method" easily? (I referr to "1. Pointer Receiver Templates")
This extra helper functions I have to create to just call the template function are lot of work (hundreds of calls) and clutters the stack trace, the source code and also the modules method list.

@aight8
Copy link
Author

aight8 commented Mar 21, 2022

What I hover achived is to allow the template function names contains underscore. I use it to group templace functions. Maybe templ should allow any valid go function names as template name?

@a-h
Copy link
Owner

a-h commented Mar 21, 2022

I think it should be possible to allow the generated function to generate a receiver.

My understanding is that you'd like it so that you've got:

data.go

package data

type Data struct {
  A string
  B string
}

data.templ

{% templ (d Data) Template() %}
<div>
  {%= d.A %}
</div>
<div>
  {%= d.B %}
</div>
{% endtempl %}

And you'd expect it to produce Go code like this?

data_templ.go

package data

func (d Data) Template() templ.Component {
  //TODO: Return a component.
}

On how much processing the io.Writer stuff takes, I've created a benchmark to compare React and templ, and also creating a buffer inside each component. Creating an internal bytes.Buffer and writing to that instead of the io.Writer saves about 20% of execution speed, at the cost of additional RAM used for the buffer. I assume this is the cost of looking up the underlying type from the interface.

@joerdav made an improvement to the output speed of Go code by introducing a write buffer instead of writing directly to files. This resulted in a large reduction in the number of OS syscalls being made, see 2a47eaf

This approach could probably be reused to place a limit on how much RAM is used, and to limit Write calls to the underlying stream and improve the performance.

Even without this improvement, I think templ is still 6 x faster than React (I haven't had chance to verify that my benchmarks make sense yet) based on the benchmark here: https://github.com/a-h/templ/tree/benchmark/benchmarks

@a-h
Copy link
Owner

a-h commented Mar 21, 2022

I had a play with this. If the input writer happens to be a io.StringWriter, the performance improves from 1159 ns/op to 853.4 ns/op (26%).

If the input is not a StringWriter (e.g. it's a http.ResponseWriter, the most common case), then the performance still improves by 8% to 1074 ns/op, and down to 18 allocations from 21 by using a buffo.Writer.

func Candidate(p Person) templ.Component {
	return templ.ComponentFunc(func(ctx context.Context, ww io.Writer) (err error) {
		w, ok := ww.(io.StringWriter)
		if !ok {
			w3 := bufio.NewWriter(ww)
			w = w3
			defer w3.Flush()
		}
		ctx, _ = templ.RenderedCSSClassesFromContext(ctx)
		ctx, _ = templ.RenderedScriptsFromContext(ctx)
		_, err = w.WriteString("<div>")
		if err != nil {

I'd need to test this a bit more, especially with a real HTTP server for comparison, before modifying the code generation to use this. I want to stick with the standard library as much as possible.

@aight8
Copy link
Author

aight8 commented Apr 3, 2022

@a-h Yes exactly. I try to extend the parser to allow using {% templ -->(d Data)<-- Template() %}.
Ah and I recognized that templ function names must be camel cased (underline is not allowed but can be usefull sometimes and valid go func id) -> for example for big templates to split it to partials for readability reasons.

a-h added a commit that referenced this issue Jun 5, 2022
* feat: start reworking the parser to support bare Go code

* wip: continuing parser work

* wip: add the if expression back in to support the template parser

* wip: fixed broken template file test

* wip: refactor from functions to static variables

* wip: added support for if statements

* wip: add comments to output to show what was parsed

* wip: reimplement switch statements

* wip: add the if unit tests back in

* wip: update README

* wip: add for loop support back in

* feat: don't render scripts if they are not required

* refactor: fix the formatting tests

* feat: add support for templates to be receivers, see #46 and #35 (item 1)

* refactor: have v1 and v2 side-by-side to enable easier migration

* refactor: continue v1 and v2 side-by-side

* feat: throw an error if a legacy format is encountered

* wip: added start of updated migration function

* fix: manually copy between v1 and v2 instead of using copier

* feat: complete migration

* docs: update for version 2

* fix: broken package test.

* fix: broken reformatting of case statements.

* feat: continue case statement work

* fix: remove additional whitespace

* fix: break on newline to allow inline if

* fix: bug in Windows line end parsing affecting CSS

* feat: add a -w parameter to choose how many cores to use during templ generation

* fix: ensure that the server always closes

* fix: ensure that code can be at the end of files too

* refactor: simplify parsing behaviour to be line oriented

* feat: normalise whitespace

* feat: use the number of CPUs available to the machine

* feat: handle trailing whitespace after case statements

* feat: improve the formatting behaviour

* feat: set tables to format block style

* feat: add style and script elements that ignore the contents

* feat: add textDocument/codeLens support

* refactor: remove TODO

* refactor: copy code from github.com/sourcegraph/go-lsp

* refactor: migrate to go.lsp.dev/protocol

* feat: migrate the LSP server and rewrite functionality to go.lsp.dev

* feat: add sourcemap visualisation

* feat: improve sourcemap rendering

* feat: fix the source location mapping

* refactor: use zero-indexed line indices to match the LSP specification

* security: CVE-2022-28948

* chore: upgrade goquery to 1.8.0

* chore: move example to _example to get Go tools to ignore from the top level

* feat: rewrite the hover output positions

* fix: handle that hover can return nil

* feat(lsp): add Implementation

* feat: improve LSP capabilities

* feat: add declaration and definition support

* feat: add DocumentLink, DocumentColor

* feat: update LSP edits, replace SourceGraph code to deprecate sourceLength

* refactor: use an array of lines to store content, instead of storing the string, to make updates less expensive

* fix: update snippets for new syntax

* docs: add updated GIF

* fix: DidChange storage of updates
@ghost
Copy link

ghost commented Aug 26, 2022

Hello, since the {% %} operator was removed, whats the way to insert go code into templ templates?

@a-h
Copy link
Owner

a-h commented Aug 27, 2022

for, if, switch and function calls are inserted by writing them on their own lines.

For example, here's a for loop just mixed in with the markup.

The example is at https://github.com/a-h/templ/blob/main/example/posts.templ

templ postsTemplate(posts []Post) {
	<div data-testid="postsTemplate">
		for _, p := range posts {
			<div data-testid="postsTemplatePost">
				<div data-testid="postsTemplatePostName">{ p.Name }</div>
				<div data-testid="postsTemplatePostAuthor">{ p.Author }</div>
			</div>
		}
	</div>
}

Where the line doesn't start with a for, switch or other branching statement, the { and } pair are used. For example, this is how p.Name is output inside the div. templ knows that the p.Name is being rendered within a HTML tag, so it automatically HTML encodes it.

<div data-testid="postsTemplatePostName">{ p.Name }</div>

There are also examples that use the features in https://github.com/a-h/templ/tree/main/generator

There's currently no way to define a variable within templ components (I'm not sure that defining variables inside a template is a particularly good idea in any templating language, including JSX), but you can call functions and output their results with braces:

<div>{ theFunction(p.Name) }</div>

If you want to define theFunction inside a templ file, you can do that by placing it inside the templ file, but I usually keep my functions out of the templ files and in standard .go files.

package main

import "strings"

func toUpper(s string) string {
	return strings.ToUpper(s)
}

templ Name(name string) {
	<div>{ toUpper(name) }</div>
}

Hope that helps!

@aight8
Copy link
Author

aight8 commented Nov 24, 2022

New idea:

13. Renderable Component

Generate Render for Named type when no func name and param list passed:

Example:

templ (c *Component) {
...
}

should generate:

func (c *Component) Render(ctx, io.Writer) {
...
}

@aight8
Copy link
Author

aight8 commented Dec 1, 2022

New idea:

14. (WIP!) Omit empty string valued attribute

Generate Render for Named type when no func name and param list passed:

Example:

<input zindex??={strVar} />

should generate:

func (c *Component) Render(ctx, io.Writer) {
...
}

@aight8
Copy link
Author

aight8 commented Dec 9, 2022

New idea:

15. Use "//line" go prefix in generated file so that go directly points to the bad code in the template

@joerdav
Copy link
Collaborator

joerdav commented Dec 9, 2022

@aight8 I think some of these might be easier to track in separate issues.

@a-h
Copy link
Owner

a-h commented Apr 30, 2023

There's been a lot of changes in the last year. Idea 14 was dealt with by conditional attribute expressions, and I think 15 is dealt with by improvements to the LSP handling, in particular, the new features that help with debugging (there's now a HTTP server that provides a window into the LSP internals).

I'm not thinking to think too much about 13 right now. My main focus over the next few months is going to be adding examples, more docs (see the new docs website at https://templ.guide), and HTMX.

I think it's time to close this issue. Happy for you to raise new issues though. :)

@a-h a-h closed this as completed Apr 30, 2023
@a-h
Copy link
Owner

a-h commented Jun 23, 2023

@aight8 - just an update on the number of virtual calls. @joerdav previously added pooled buffers which improved the performance of output writing a lot, but he's also just massively reduced the number of calls by combining what used to be multiple calls to Write into a single operation #96

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

3 participants