Skip to content

Commit

Permalink
refactor: improve void element parsing perf, fixes #886 and #857 (#887)
Browse files Browse the repository at this point in the history
  • Loading branch information
a-h authored Aug 18, 2024
1 parent c24c8e4 commit ae99146
Show file tree
Hide file tree
Showing 12 changed files with 171 additions and 67 deletions.
2 changes: 1 addition & 1 deletion generator/test-html/template.templ
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ templ render(p person) {
<hr noshade?={ true }/>
<hr optionA optionB?={ true } optionC="other" optionD?={ false }/>
<hr noshade/>
<input name="test">Text</input>
<input name="test"/>Text
}
2 changes: 1 addition & 1 deletion generator/test-html/template_templ.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 34 additions & 0 deletions parser/v2/benchmarks_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package parser

import (
_ "embed"
"strings"
"testing"
)

//go:embed benchmarktestdata/benchmark.txt
var benchmarkTemplate string

func BenchmarkParse(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
if _, err := ParseString(benchmarkTemplate); err != nil {
b.Fatal(err)
}
}
}

func BenchmarkFormat(b *testing.B) {
b.ReportAllocs()
sb := new(strings.Builder)
for i := 0; i < b.N; i++ {
tf, err := ParseString(benchmarkTemplate)
if err != nil {
b.Fatal(err)
}
if err = tf.Write(sb); err != nil {
b.Fatal(err)
}
sb.Reset()
}
}
18 changes: 18 additions & 0 deletions parser/v2/benchmarktestdata/benchmark.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package benchmarktestdata

templ Benchmark() {
<div>Hello</div>
<br>
<br>
<br>
</br>
<div>Lorem ipsum dolor sit amet, consectetur adipiscing elit.</div>
<br>
<br>
<ul>
<li>Item 1</li>
<li>Item 2</li>
<li>Item 3</li>
</ul>
}

19 changes: 0 additions & 19 deletions parser/v2/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package parser

import (
"errors"
"fmt"
)

type diagnoser func(Node) ([]Diagnostic, error)
Expand Down Expand Up @@ -35,7 +34,6 @@ func walkNodes(t []Node, f func(Node) bool) {

var diagnosers = []diagnoser{
useOfLegacyCallSyntaxDiagnoser,
voidElementWithChildrenDiagnoser,
}

func Diagnose(t TemplateFile) ([]Diagnostic, error) {
Expand Down Expand Up @@ -64,20 +62,3 @@ func useOfLegacyCallSyntaxDiagnoser(n Node) ([]Diagnostic, error) {
}
return nil, nil
}

func voidElementWithChildrenDiagnoser(n Node) (d []Diagnostic, err error) {
e, ok := n.(Element)
if !ok {
return
}
if !e.IsVoidElement() {
return
}
if len(e.Children) == 0 {
return
}
return []Diagnostic{{
Message: fmt.Sprintf("void element <%s> should not have child content", e.Name),
Range: e.NameRange,
}}, nil
}
15 changes: 0 additions & 15 deletions parser/v2/diagnostics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,21 +134,6 @@ templ template () {
}`,
want: nil,
},
{
name: "voidElementWithChildrenDiagnoser: with diagnostics",
template: `
package main
templ template () {
<div>
<input>Child content</input>
</div>
}`,
want: []Diagnostic{{
Message: "void element <input> should not have child content",
Range: Range{Position{46, 5, 4}, Position{51, 5, 9}},
}},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
45 changes: 34 additions & 11 deletions parser/v2/elementparser.go
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,34 @@ var (
})
)

// Void element closer.
var voidElementCloser voidElementCloserParser

type voidElementCloserParser struct{}

var voidElementCloseTags = []string{"</area>", "</base>", "</br>", "</col>", "</command>", "</embed>", "</hr>", "</img>", "</input>", "</keygen>", "</link>", "</meta>", "</param>", "</source>", "</track>", "</wbr>"}

func (voidElementCloserParser) Parse(pi *parse.Input) (n Node, ok bool, err error) {
var ve string
for _, ve = range voidElementCloseTags {
s, canPeekLen := pi.Peek(len(ve))
if !canPeekLen {
continue
}
if !strings.EqualFold(s, ve) {
continue
}
// Found a match.
ok = true
break
}
if !ok {
return nil, false, nil
}
pi.Take(len(ve))
return nil, true, nil
}

// Element.
var element elementParser

Expand All @@ -396,28 +424,19 @@ func (elementParser) Parse(pi *parse.Input) (n Node, ok bool, err error) {

// Once we've got an open tag, the rest must be present.
l := pi.Position().Line
endOfOpenTag := pi.Index()

// If the element is self-closing, even if it's not really a void element (br, hr etc.), we can return early.
if ot.Void {
if ot.Void || r.IsVoidElement() {
// Escape early, no need to try to parse children for self-closing elements.
return addTrailingSpaceAndValidate(start, r, pi)
}

// Void elements _might_ have children, even though it's invalid.
// We want to allow this to be parsed.
// Parse children.
closer := StripType(parse.All(parse.String("</"), parse.String(ot.Name), parse.Rune('>')))
tnp := newTemplateNodeParser(closer, fmt.Sprintf("<%s>: close tag", ot.Name))
nodes, _, err := tnp.Parse(pi)
if err != nil {
notFoundErr, isNotFoundError := err.(UntilNotFoundError)
if r.IsVoidElement() && isNotFoundError {
// Void elements shouldn't have children, or a close tag, so this is expected.
// When the template is reformatted, we won't reach here, because <hr> will be converted to <hr/>.
// Return the element as we have it.
pi.Seek(endOfOpenTag)
return addTrailingSpaceAndValidate(start, r, pi)
}
if isNotFoundError {
err = notFoundErr.ParseError
}
Expand All @@ -443,6 +462,10 @@ func (elementParser) Parse(pi *parse.Input) (n Node, ok bool, err error) {
}

func addTrailingSpaceAndValidate(start parse.Position, e Element, pi *parse.Input) (n Node, ok bool, err error) {
// Elide any void close tags.
if _, _, err = voidElementCloser.Parse(pi); err != nil {
return e, false, err
}
// Add trailing space.
ws, _, err := parse.Whitespace.Parse(pi)
if err != nil {
Expand Down
40 changes: 21 additions & 19 deletions parser/v2/elementparser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,20 @@ if test {
}
}

func TestVoidElementCloserParser(t *testing.T) {
t.Run("all void elements are parsed", func(t *testing.T) {
for _, input := range voidElementCloseTags {
_, ok, err := voidElementCloser.Parse(parse.NewInput(input))
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if !ok {
t.Fatalf("failed to parse %q", input)
}
}
})
}

func TestElementParser(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -662,15 +676,9 @@ func TestElementParser(t *testing.T) {
From: Position{Index: 1, Line: 0, Col: 1},
To: Position{Index: 6, Line: 0, Col: 6},
},
Children: []Node{
Text{
Value: "Text",
Range: Range{
From: Position{Index: 7, Line: 0, Col: 7},
To: Position{Index: 11, Line: 0, Col: 11},
},
},
},
// <input> is a void element, so text is not a child of the input.
// </input> is ignored.
Children: nil,
},
},
{
Expand Down Expand Up @@ -736,23 +744,17 @@ func TestElementParser(t *testing.T) {
},
},
{
name: "element: void nesting others is OK (br/hr)",
name: "element: void nesting is ignored",
input: `<br><hr></br>`,
expected: Element{
Name: "br",
NameRange: Range{
From: Position{Index: 1, Line: 0, Col: 1},
To: Position{Index: 3, Line: 0, Col: 3},
},
Children: []Node{
Element{
Name: "hr",
NameRange: Range{
From: Position{Index: 5, Line: 0, Col: 5},
To: Position{Index: 7, Line: 0, Col: 7},
},
},
},
// <br> is a void element, so <hr> is not a child of the <br>.
// </br> is ignored.
Children: nil,
},
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ templ input(value, validation string) {
<img></img>
<input>
<input></input>
<input>Text
<input>Text</input>
<keygen>
<keygen></keygen>
Expand Down Expand Up @@ -59,7 +60,8 @@ templ input(value, validation string) {
<img/>
<input/>
<input/>
<input>Text</input>
<input/>Text
<input/>Text
<keygen/>
<keygen/>
<link/>
Expand Down
16 changes: 16 additions & 0 deletions parser/v2/templateparser.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ type templateNodeParser[TUntil any] struct {

var rawElements = parse.Any(styleElement, scriptElement)

var templateNodeSkipParsers = []parse.Parser[Node]{
voidElementCloser, // </br>, </img> etc. - should be ignored.
}

var templateNodeParsers = []parse.Parser[Node]{
docType, // <!DOCTYPE html>
htmlComment, // <!--
Expand All @@ -80,6 +84,7 @@ var templateNodeParsers = []parse.Parser[Node]{
}

func (p templateNodeParser[T]) Parse(pi *parse.Input) (op Nodes, ok bool, err error) {
outer:
for {
// Check if we've reached the end.
if p.until != nil {
Expand All @@ -94,6 +99,17 @@ func (p templateNodeParser[T]) Parse(pi *parse.Input) (op Nodes, ok bool, err er
}
}

// Skip any nodes that we don't care about.
for _, p := range templateNodeSkipParsers {
_, matched, err := p.Parse(pi)
if err != nil {
return Nodes{}, false, err
}
if matched {
continue outer
}
}

// Attempt to parse a node.
// Loop through the parsers and try to parse a node.
var matched bool
Expand Down
38 changes: 38 additions & 0 deletions parser/v2/templateparser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -806,6 +806,44 @@ func TestTemplateParser(t *testing.T) {
},
},
},
{
name: "template: void element closers are ignored",
input: `templ Name() {
<br></br><br>
}`,
expected: HTMLTemplate{
Range: Range{
From: Position{Index: 0, Line: 0, Col: 0},
To: Position{Index: 31, Line: 2, Col: 1},
},
Expression: Expression{
Value: "Name()",
Range: Range{
From: Position{Index: 6, Line: 0, Col: 6},
To: Position{Index: 12, Line: 0, Col: 12},
},
},
Children: []Node{
Whitespace{Value: "\t"},
Element{
Name: "br",
NameRange: Range{
From: Position{Index: 17, Line: 1, Col: 2},
To: Position{Index: 19, Line: 1, Col: 4},
},
TrailingSpace: SpaceNone,
},
Element{
Name: "br",
NameRange: Range{
From: Position{Index: 26, Line: 1, Col: 11},
To: Position{Index: 28, Line: 1, Col: 13},
},
TrailingSpace: SpaceVertical,
},
},
},
},
}
for _, tt := range tests {
tt := tt
Expand Down
5 changes: 5 additions & 0 deletions parser/v2/textparser.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ var textParser = parse.Func(func(pi *parse.Input) (n Node, ok bool, err error) {
}
t.Range = NewRange(from, pi.Position())

// Elide any void element closing tags.
if _, _, err = voidElementCloser.Parse(pi); err != nil {
return
}

// Parse trailing whitespace.
ws, _, err := parse.Whitespace.Parse(pi)
if err != nil {
Expand Down

0 comments on commit ae99146

Please sign in to comment.