Repository: maragudk/gomponents (go)

Issue:
Make Group renderable
Maybe all the rendering logic can be moved from `El` into a shared function easily?

Pull request:
Make Groups renderable
This change makes the result of `Group` renderable directly, instead of panicking, with the important caveat that root-level attributes are _ignored_. I don't think this will give problems in practice, as the main use case for rendering `Group` is basically to return root-level elements to the client using something like HTMX.

I tried adding a `Fragment`, but it was weird and confusing having two functions (`Group` and `Fragment`) do essentially the same thing, the only difference being whether the argument was a slice of `Node`s or varargs. 

Fixes #162

Patch:
diff --git a/gomponents.go b/gomponents.go
index 952e32c..ffe8483 100644
--- a/gomponents.go
+++ b/gomponents.go
@@ -72,10 +72,16 @@ func (n NodeFunc) String() string {
 // If an element is a void element, non-attribute children nodes are ignored.
 // Use this if no convenience creator exists in the html package.
 func El(name string, children ...Node) Node {
-	return NodeFunc(func(w2 io.Writer) error {
-		w := &statefulWriter{w: w2}
+	return NodeFunc(func(w io.Writer) error {
+		return render(w, &name, children...)
+	})
+}
+
+func render(w2 io.Writer, name *string, children ...Node) error {
+	w := &statefulWriter{w: w2}
 
-		w.Write([]byte("<" + name))
+	if name != nil {
+		w.Write([]byte("<" + *name))
 
 		for _, c := range children {
 			renderChild(w, c, AttributeType)
@@ -83,17 +89,20 @@ func El(name string, children ...Node) Node {
 
 		w.Write([]byte(">"))
 
-		if isVoidElement(name) {
+		if isVoidElement(*name) {
 			return w.err
 		}
+	}
 
-		for _, c := range children {
-			renderChild(w, c, ElementType)
-		}
+	for _, c := range children {
+		renderChild(w, c, ElementType)
+	}
 
-		w.Write([]byte("</" + name + ">"))
-		return w.err
-	})
+	if name != nil {
+		w.Write([]byte("</" + *name + ">"))
+	}
+
+	return w.err
 }
 
 // renderChild c to the given writer w if the node type is t.
@@ -102,6 +111,8 @@ func renderChild(w *statefulWriter, c Node, t NodeType) {
 		return
 	}
 
+	// Rendering groups like this is still important even though a group can render itself,
+	// since otherwise attributes will sometimes be ignored.
 	if g, ok := c.(group); ok {
 		for _, groupC := range g.children {
 			renderChild(w, groupC, t)
@@ -241,17 +252,19 @@ type group struct {
 
 // String satisfies [fmt.Stringer].
 func (g group) String() string {
-	panic("cannot render group directly")
+	var b strings.Builder
+	_ = g.Render(&b)
+	return b.String()
 }
 
 // Render satisfies [Node].
-func (g group) Render(io.Writer) error {
-	panic("cannot render group directly")
+func (g group) Render(w io.Writer) error {
+	return render(w, nil, g.children...)
 }
 
-// Group multiple Nodes into one Node. Useful for concatenation of Nodes in variadic functions.
-// The resulting Node cannot Render directly, trying it will panic.
-// Render must happen through a parent element created with El or a helper.
+// Group a slice of Nodes into one Node. Useful for grouping the result of [Map] into one [Node].
+// A [Group] can render directly, but if any of the direct children are [AttributeType], they will be ignored,
+// to not produce invalid HTML.
 func Group(children []Node) Node {
 	return group{children: children}
 }


Test patch:
diff --git a/gomponents_test.go b/gomponents_test.go
index c0ae7a7..f3e81b0 100644
--- a/gomponents_test.go
+++ b/gomponents_test.go
@@ -234,35 +234,40 @@ func TestGroup(t *testing.T) {
 		assert.Equal(t, `<div class="foo"><img><br id="hat"><hr></div>`, e)
 	})
 
-	t.Run("panics on direct render", func(t *testing.T) {
-		e := g.Group(nil)
-		panicked := false
-		defer func() {
-			if err := recover(); err != nil {
-				panicked = true
-			}
-		}()
-		_ = e.Render(nil)
-		if !panicked {
-			t.FailNow()
-		}
+	t.Run("ignores attributes at the first level", func(t *testing.T) {
+		children := []g.Node{g.Attr("class", "hat"), g.El("div"), g.El("span")}
+		e := g.Group(children)
+		assert.Equal(t, "<div></div><span></span>", e)
 	})
 
-	t.Run("panics on direct string", func(t *testing.T) {
-		e := g.Group(nil).(fmt.Stringer)
-		panicked := false
-		defer func() {
-			if err := recover(); err != nil {
-				panicked = true
-			}
-		}()
-		_ = e.String()
-		if !panicked {
+	t.Run("does not ignore attributes at the second level", func(t *testing.T) {
+		children := []g.Node{g.El("div", g.Attr("class", "hat")), g.El("span")}
+		e := g.Group(children)
+		assert.Equal(t, `<div class="hat"></div><span></span>`, e)
+	})
+
+	t.Run("can render a group child node including attributes", func(t *testing.T) {
+		children := []g.Node{g.Attr("id", "hat"), g.El("div"), g.El("span")}
+		e := g.El("div", g.Group(children))
+		assert.Equal(t, `<div id="hat"><div></div><span></span></div>`, e)
+	})
+
+	t.Run("implements fmt.Stringer", func(t *testing.T) {
+		children := []g.Node{g.El("div"), g.El("span")}
+		e := g.Group(children)
+		if e, ok := e.(fmt.Stringer); !ok || e.String() != "<div></div><span></span>" {
 			t.FailNow()
 		}
 	})
 }
 
+func ExampleGroup() {
+	children := []g.Node{g.El("div"), g.El("span")}
+	e := g.Group(children)
+	_ = e.Render(os.Stdout)
+	// Output: <div></div><span></span>
+}
+
 func TestIf(t *testing.T) {
 	t.Run("returns node if condition is true", func(t *testing.T) {
 		n := g.El("div", g.If(true, g.El("span")))