fix: keep ui.Template a value, holding its state on the Element

.agents/skills/jaws/SKILL.md

@@ -46,7 +46,7 @@ JaWS is an immediate-mode, server-driven UI framework, not an MVC framework.
   nil `UI` interface is a no-op. Surviving such a call is up to the concrete type, not
   a requirement: a widget that dereferences its fields panics, and none of the
   standard `lib/ui` widgets document nil-receiver tolerance. Do not pass a nil pointer
-  of a type that does not; use its zero value (e.g. `&ui.Template{}`) instead.
+  of a type that does not; use its zero value (e.g. `ui.Template{}`) instead.
 - Every JaWS `UI` value is request-scoped. Once used by one Request, never use
   that value with another Request; construct fresh widgets per request. The
   widgets may still refer to shared, synchronized application state, binders,
@@ -104,9 +104,24 @@ These are the two usual building blocks for widget handlers passed to `$.Button`
 
 - `ui.Template` expands `Dot` into tags via `tag.TagExpand` (package `github.com/linkdata/jaws/lib/tag`, imported as `tag`); the root dot is part of identity/tag behavior.
 - `ui.Template` is for partial templates only; full document/page templates should be rendered through `ui.Handler`.
-- Prefer comparable root dots (pointers or small comparable structs).
-- If root dot is non-comparable, implement `JawsGetTag(tag.Context) any` and return a comparable tag.
-- Do not use plain `string`, numeric, `bool`, `template.HTML`, or `template.HTMLAttr` as tags; `tag.TagExpand` rejects them.
+- A nil-interface Template `Dot` is valid and contributes no tag; a typed nil follows its
+  dynamic type's comparability and expansion behavior.
+- The root dot **must** be comparable at runtime and equal to itself: `ui.NewTemplate`
+  returns a value, so the dot is part of the widget the container widgets use as a map key.
+  A slice, map, func or NaN-bearing dot makes the widget unusable as a container child.
+- Implementing `JawsGetTag(tag.Context) any` does **not** fix a non-comparable dot — it
+  resolves the *tag*, not the widget's comparability. A non-comparable dot is unsupported.
+  Always use the Template itself as a value; taking its address is unsupported because it
+  changes container reuse to pointer identity. `ui.Handler` is the arbitrary-dot exception.
+- `tag.TagExpand` rejects exactly these as tags: `string`, `bool`, `int`/`int8`/`int16`/`int32`/`int64`,
+  `uint`/`uint8`/`uint16`/`uint32`/`uint64`, `float32`/`float64`, `template.HTML`, `template.HTMLAttr`,
+  `jid.Jid` and `key.Key`. It is a switch on exact types, so aliases of a rejected type are rejected,
+  while `uintptr` and the complex types are not on the rejection list. Other defined types
+  (`type RowID string`) are not rejected merely because their underlying predeclared type is on it —
+  they still have to be comparable and equal to themselves, and one implementing `tag.TagGetter` is
+  expanded instead.
+- This applies to a Template's `Dot` too, since rendering expands it: a comparable, reflexive `string`
+  dot still fails at render with `illegal tag type string`.
 - If you need string-like semantic tags, use `tag.Tag("...")` or a comparable typed struct/pointer.
 
 ## `$.Template(...)` signature and parameter semantics
@@ -156,13 +171,24 @@ For clickable content rendering:
 
 - Keep HTML structure in templates; avoid manual HTML string assembly in Go.
 - `ui.Template.JawsUpdate` re-renders the template data into the generated wrapper.
-- `ui.NewTemplate` returns a `*ui.Template`, which tracks the Elements its execution
-  creates and therefore backs one live Element; construct a fresh one per render
-  (`$.Template` already does). A successful update unregisters every Element the
-  previous execution created through the writer it was given — the widget helpers,
-  `$.Register`, `$.RadioGroup` and nested `$.Template` alike — since `SetInner` replaces
-  the DOM holding them. Ownership is recorded at creation, so an Element that never
-  rendered is reclaimed as well.
+- `ui.NewTemplate` returns a plain `ui.Template` **value** that may back multiple live
+  Elements because it keeps the Elements its execution creates in each rendering Element's
+  state slot rather than on itself. Do not take its address. A
+  successful update unregisters every Element the previous execution created through the
+  writer it was given — the widget helpers, `$.Register`, `$.RadioGroup` and nested
+  `$.Template` alike — since `SetInner` replaces the DOM holding them. Ownership is recorded
+  at creation, so an Element that never rendered is reclaimed as well.
+- Because the value is stateless, a container's `JawsContains` may rebuild equal children on
+  every call and their Elements are still reused — that equality *is* the reuse key.
+- The state slot is claimed while rendering, which constrains composition:
+  - at most one Template may render a given Element;
+  - a composite UI must use Template values equal under `==` for rendering and updating
+    an Element; using unequal values is unsupported;
+  - a **wrapped** Template updates only an Element rendered by an equal Template value, so
+    it is not usable as a `$.Register` updater;
+  - an **unwrapped** Template is usable there — its updates are a documented no-op — but only
+    `$.Register` (the `RequestWriter` helper) also delivers its click/input/context-menu
+    handlers; a bare `ui.Register`/`ui.NewRegister` value promotes no handler methods.
 - Call `$.RadioGroup` from the template that renders the group: its Elements belong to
   the template whose body called it, not to the wrapper their markup lands in.
 - HTML getter paths must not mutate domain state, but they may call element update methods (`SetClass`, `RemoveClass`, `SetAttr`, `RemoveAttr`, etc.) on the passed-in `*Element` to co-ordinate wrapper class/attribute changes with the inner-HTML refresh. No custom `JawsUpdate` is needed for that case — the queued wrapper updates flush alongside the `SetInner` from `HTMLInner.JawsUpdate`.

contracts.go

@@ -44,6 +44,12 @@ type Renderer interface {
 	// JawsRender is called once per [Element] when rendering the initial webpage.
 	// Do not call this yourself unless it is from within another JawsRender implementation.
 	// The engine does not invoke this once the [Element] is deleted (see [Element.Deleted]).
+	//
+	// When delegating, note that a renderer may claim the Element's widget state slot
+	// (see [SetElementState]) and that only one of them can: a delegate whose own
+	// renderer claims the slot — [github.com/linkdata/jaws/lib/ui.Template] does —
+	// fails with [ErrElementStateClaimed] if the delegating renderer, or an earlier
+	// delegate, already claimed it.
 	JawsRender(elem *Element, w io.Writer, params []any) error
 }
 
@@ -61,8 +67,10 @@ type TemplateLookuper interface {
 // Within its owning Request, a UI value must back at most one live Element
 // unless its concrete type documents support for multiple live Elements. Such
 // a type must not retain state on the shared UI value that can differ between
-// those Elements. To render the same application state more than once, construct
-// distinct UI values that share getters, setters, handlers or tags.
+// those Elements — [SetElementState] gives it somewhere else to keep such state,
+// keyed to the Element rather than to the widget, but opting in remains the concrete
+// type's decision to document. To render the same application state more than once,
+// construct distinct UI values that share getters, setters, handlers or tags.
 // An Element stops being live when it is deleted or its owning Request lifecycle
 // ends.
 //
@@ -100,6 +108,9 @@ type Updater interface {
 	// JawsUpdate is called for an [Element] that has been marked dirty to update its HTML.
 	// Do not call this yourself unless it is from within another JawsUpdate implementation.
 	// The engine does not invoke this once the [Element] is deleted (see [Element.Deleted]).
+	// A UI implementation that delegates rendering and updating must delegate both calls
+	// to the same UI widget. Rendering elem through one widget and updating it through
+	// another is unsupported.
 	JawsUpdate(elem *Element)
 }
 

element.go

@@ -39,9 +39,13 @@ type Element struct {
 	// All builds enforce this: appendHandlers drops late mutations (debug builds
 	// panic).
 	handlers []any
-	jid      jid.Jid     // JaWS ID, unique to this Element within its Request
-	deleted  atomic.Bool // true once the Element has been removed from its Request
-	frozen   atomic.Bool // set when handlers are sealed (JawsRender returns or Freeze called); guards handler mutators in all builds
+	// data is the widget state slot, claimed by the widget rendering this Element and
+	// reached through [ElementState] and [SetElementState]. It is guarded by
+	// Request.mu; the stored value's own synchronization guards its contents.
+	data    any
+	jid     jid.Jid     // JaWS ID, unique to this Element within its Request
+	deleted atomic.Bool // true once the Element has been removed from its Request
+	frozen  atomic.Bool // set when handlers are sealed (JawsRender returns or Freeze called); guards handler mutators in all builds
 }
 
 // String returns a debug representation of elem: its UI type, Jid, and tags.

element_create_benchmark_test.go

@@ -0,0 +1,69 @@
+package jaws
+
+import (
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+// This file isolates the element-creation benchmark, which measures what every Element
+// pays for the widget state slot whether or not that Element ever uses one.
+
+// benchCreateUI is a stateless widget that never touches the state slot, so the measurement
+// is the per-Element cost rather than any Template bookkeeping. It documents support for
+// multiple live Elements because one value backs every Element the benchmark creates.
+type benchCreateUI struct{}
+
+func (benchCreateUI) JawsRender(elem *Element, w io.Writer, params []any) error {
+	_, err := io.WriteString(w, "<span>x</span>")
+	return err
+}
+
+func (benchCreateUI) JawsUpdate(elem *Element) {}
+
+// BenchmarkElementCreateBatch creates and renders a fixed batch of Elements per iteration,
+// then deletes them with the timer stopped.
+//
+// Batching is deliberate: b.StopTimer and b.StartTimer each call runtime.ReadMemStats, so
+// toggling around a single sub-microsecond creation would leave the timed section tiny,
+// calibration would pick an enormous b.N, and the excluded setup would run for minutes.
+// Amortising both calls over the batch keeps that honest, and deleting the batch keeps the
+// Request registry bounded instead of growing across iterations. The reported figure is per
+// batch of 64 Elements.
+//
+// The Jaws and Request are built before b.ResetTimer, and the final b.StopTimer excludes the
+// deferred Close: both would otherwise be divided into every reported figure, time and
+// allocations alike, making the result depend on b.N rather than on the Element.
+func BenchmarkElementCreateBatch(b *testing.B) {
+	b.ReportAllocs()
+	const batch = 64
+
+	jw, err := New()
+	if err != nil {
+		b.Fatal(err)
+	}
+	defer jw.Close()
+	rq := jw.NewRequest(httptest.NewRequest(http.MethodGet, "/", nil))
+	if rq == nil {
+		b.Fatal("nil request")
+	}
+	var ui benchCreateUI
+	elems := make([]*Element, 0, batch)
+
+	b.ResetTimer()
+	for range b.N {
+		elems = elems[:0]
+		for range batch {
+			elem := rq.NewElement(ui)
+			if err := elem.JawsRender(io.Discard, nil); err != nil {
+				b.Fatal(err)
+			}
+			elems = append(elems, elem)
+		}
+		b.StopTimer()
+		rq.DeleteElements(elems)
+		b.StartTimer()
+	}
+	b.StopTimer()
+}

elementstate.go

@@ -0,0 +1,74 @@
+package jaws
+
+import "errors"
+
+// ErrElementStateClaimed is returned by [SetElementState] when the [Element] already
+// has widget state, including state of the same type.
+var ErrElementStateClaimed = errors.New("jaws: element state already claimed")
+
+// ErrElementStateNil is returned by [SetElementState] when the state to store is a nil
+// interface, which cannot be distinguished from an unclaimed slot.
+var ErrElementStateNil = errors.New("jaws: element state must not be nil")
+
+// ElementState returns the widget state stored for elem, or nil if none was claimed.
+//
+// The state belongs to the [Element], not to the widget value that claimed it.
+// ElementState therefore does not identify its caller, but that storage detail does not
+// permit a different UI widget to update the Element; see [Updater]. Loading never
+// claims: a widget that finds no state did not claim this Element. Most widgets never
+// claim one, so a nil return says nothing about whether the Element was rendered.
+//
+// Safe for concurrent use; it takes the [Request] lock. Only the slot itself is
+// synchronized: whatever the stored value contains is guarded by that value's own
+// synchronization, not by this call.
+//
+// elem must be a non-nil Element obtained from [Request.NewElement]. ElementState does
+// not verify that provenance. It panics if elem or elem.Request is nil.
+func ElementState(elem *Element) (state any) {
+	rq := elem.Request
+	rq.mu.RLock()
+	state = elem.data
+	rq.mu.RUnlock()
+	return
+}
+
+// SetElementState claims elem's widget state slot, which a widget does while rendering
+// the Element so its updates and cleanup can find that state again.
+//
+// There is one slot per Element and it cannot be replaced, only claimed: a second claim
+// returns [ErrElementStateClaimed] and leaves the stored state untouched, even when the
+// new state has the same type. At most one widget may claim a given Element, so a widget
+// that renders an Element and delegates to another renderer on that same Element must
+// decide which of them claims it.
+//
+// A nil state returns [ErrElementStateNil] and stores nothing, since a nil interface is
+// how an unclaimed slot is represented; that check comes first, so a nil state is
+// rejected whatever the slot holds, and before elem is examined at all. A typed nil is a
+// non-nil interface and does claim the slot.
+//
+// Safe for concurrent use: concurrent claims on one Element are serialized by the
+// [Request] lock and exactly one wins, the rest reporting [ErrElementStateClaimed]. Only
+// the claim is synchronized; mutating the stored value afterwards is guarded by that
+// value's own synchronization, not by this call.
+//
+// When state is non-nil, elem must be a non-nil Element obtained from
+// [Request.NewElement]. SetElementState does not verify that provenance. It panics if
+// elem or elem.Request is nil; a nil state is rejected before elem is examined.
+func SetElementState(elem *Element, state any) error {
+	// The two functions are package-level rather than methods on Element because
+	// ui.With embeds both *Element and ui.RequestWriter, so any method returning a value
+	// is reachable from a template: {{$.Element.SetElementState $.Dot}} would let a
+	// template claim the slot out from under the renderer. html/template cannot call a
+	// package-level function.
+	if state == nil {
+		return ErrElementStateNil
+	}
+	rq := elem.Request
+	rq.mu.Lock()
+	defer rq.mu.Unlock()
+	if elem.data != nil {
+		return ErrElementStateClaimed
+	}
+	elem.data = state
+	return nil
+}

elementstate_test.go

@@ -0,0 +1,121 @@
+package jaws
+
+import (
+	"errors"
+	"sync"
+	"testing"
+)
+
+type testElementState struct{ name string }
+
+func TestElementState_ClaimOnce(t *testing.T) {
+	rq := newTestRequest(t)
+	defer rq.Close()
+
+	elem := rq.NewElement(&testUi{})
+	if got := ElementState(elem); got != nil {
+		t.Fatalf("unclaimed slot = %v, want nil", got)
+	}
+
+	first := &testElementState{name: "first"}
+	if err := SetElementState(elem, first); err != nil {
+		t.Fatalf("first claim: %v", err)
+	}
+	if got := ElementState(elem); got != first {
+		t.Fatalf("loaded %v, want the claimed state", got)
+	}
+
+	// A second claim fails and leaves the original in place, including one carrying the
+	// same dynamic type: same type does not mean same owner.
+	for _, state := range []any{&testElementState{name: "same type"}, "other type"} {
+		if err := SetElementState(elem, state); !errors.Is(err, ErrElementStateClaimed) {
+			t.Fatalf("second claim with %T = %v, want %v", state, err, ErrElementStateClaimed)
+		}
+	}
+	if got := ElementState(elem); got != first {
+		t.Fatalf("state after rejected claims = %v, want the original", got)
+	}
+
+	// Slots are per Element.
+	other := rq.NewElement(&testUi{})
+	if got := ElementState(other); got != nil {
+		t.Fatalf("second element's slot = %v, want nil", got)
+	}
+	if err := SetElementState(other, &testElementState{name: "second"}); err != nil {
+		t.Fatalf("claiming a different element: %v", err)
+	}
+	if ElementState(elem) != first {
+		t.Error("claiming another element disturbed the first element's state")
+	}
+}
+
+func TestElementState_NilHandling(t *testing.T) {
+	rq := newTestRequest(t)
+	defer rq.Close()
+
+	elem := rq.NewElement(&testUi{})
+
+	// A nil interface cannot be stored: it is indistinguishable from an unclaimed slot,
+	// so accepting it would report success while leaving the slot claimable.
+	if err := SetElementState(elem, nil); !errors.Is(err, ErrElementStateNil) {
+		t.Fatalf("nil claim = %v, want %v", err, ErrElementStateNil)
+	}
+	if got := ElementState(elem); got != nil {
+		t.Fatalf("slot after nil claim = %v, want still nil", got)
+	}
+
+	// A typed nil is a non-nil interface, so it does claim the slot.
+	var typedNil *testElementState
+	if err := SetElementState(elem, typedNil); err != nil {
+		t.Fatalf("typed-nil claim: %v", err)
+	}
+	if err := SetElementState(elem, &testElementState{}); !errors.Is(err, ErrElementStateClaimed) {
+		t.Fatalf("claim after typed nil = %v, want %v", err, ErrElementStateClaimed)
+	}
+
+	// The nil-argument check precedes the occupancy check, so a nil state against an
+	// occupied slot still reports ErrElementStateNil.
+	if err := SetElementState(elem, nil); !errors.Is(err, ErrElementStateNil) {
+		t.Fatalf("nil claim on occupied slot = %v, want %v", err, ErrElementStateNil)
+	}
+}
+
+func TestElementState_ConcurrentClaims(t *testing.T) {
+	rq := newTestRequest(t)
+	defer rq.Close()
+
+	elem := rq.NewElement(&testUi{})
+	const claimants = 8
+
+	var wg sync.WaitGroup
+	errs := make([]error, claimants)
+	states := make([]any, claimants)
+	start := make(chan struct{})
+	for i := range claimants {
+		states[i] = &testElementState{name: "claimant"}
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			<-start
+			errs[i] = SetElementState(elem, states[i])
+		}()
+	}
+	close(start)
+	wg.Wait()
+
+	var winners int
+	for i, err := range errs {
+		switch {
+		case err == nil:
+			winners++
+			if got := ElementState(elem); got != states[i] {
+				t.Errorf("claimant %d succeeded but the slot holds %v", i, got)
+			}
+		case !errors.Is(err, ErrElementStateClaimed):
+			t.Errorf("claimant %d = %v, want %v", i, err, ErrElementStateClaimed)
+		}
+	}
+	if winners != 1 {
+		t.Errorf("successful claims = %d, want exactly 1", winners)
+	}
+}