fix: close #222 — track the Elements Register and RadioGroup create

.agents/skills/jaws/SKILL.md

@@ -158,16 +158,13 @@ For clickable content rendering:
 - `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 the Elements the
-  previous execution created through `rw.NewUI` — the path every `RequestWriter`
-  widget helper except `$.Register` and `$.RadioGroup` takes — since `SetInner`
-  replaces the DOM holding them.
-- Those two helpers create Elements with `Request.NewElement`, so no template owns
-  them and their cleanup falls to the browser, which reports the ids it removed from
-  the DOM as the wrapper's new content is applied. An Element whose id never reaches
-  the DOM stays registered until the request ends (a discarded `$.Register` Jid, or an
-  execution that failed before delivering its markup), so prefer the `rw.NewUI`-backed
-  helpers inside a template that updates.
+  (`$.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.
+- 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`.
 - Use a custom `JawsUpdate` only when the widget's update logic diverges from "render the getter again" — e.g. to compare against a stored last-value and skip the update (as the input widgets do).
 - `Element.SetAttr/RemoveAttr/SetClass/RemoveClass/SetInner/SetValue/Append/Order/Remove/Replace` are update-time operations; call them only from render/update processing.

lib/ui/README.md

@@ -29,26 +29,22 @@ already happened; it does not roll back partial output, queued messages, or
 application side effects. The tracked elements the failed execution registered are
 unregistered, since nothing will update them.
 
-A template owns the elements created through `rw.NewUI(...)`, the path taken by
-every `RequestWriter` widget helper except the two below — `{{$.Span ...}}`,
-`{{$.Button ...}}`, a nested `{{$.Template ...}}`, and so on. A successful update
-unregisters the ones the previous render left behind, along with the DOM that
-`SetInner` replaces. On updates that `SetInner` is queued only after a complete
-successful render, so a failed update leaves the browser DOM unchanged — and with
-it the previous render's elements — while earlier server-side side effects from
-that attempted render may remain. Treat template execution errors as application
-bugs: validate data before rendering and keep template actions infallible once they
-start emitting output or nested UI.
-
-`{{$.Register ...}}` and `{{$.RadioGroup ...}}` are the exceptions: they create
-elements through `Request.NewElement` directly rather than `rw.NewUI`, so the
-template neither owns nor unregisters them. Their cleanup falls to the browser,
-which reports the JaWS ids it removed from the DOM as the wrapper's new content is
-applied, and the request unregisters those elements. An element whose id never
-reaches the DOM has nothing to report it and stays registered until the request
-ends — one from an execution that failed before its markup was delivered, or a
-`$.Register` whose returned Jid the template discards. Prefer the `rw.NewUI`-backed
-helpers inside a template that updates.
+A template owns every element created through the `RequestWriter` it is given —
+`{{$.Span ...}}`, `{{$.Button ...}}`, `{{$.Register ...}}`, `{{$.RadioGroup ...}}`,
+a nested `{{$.Template ...}}`, and so on. A successful update unregisters the ones
+the previous render left behind, along with the DOM that `SetInner` replaces.
+Ownership is recorded when an element is created rather than after it renders, so
+an element that never reached the browser is reclaimed too. On updates that
+`SetInner` is queued only after a complete successful render, so a failed update
+leaves the browser DOM unchanged — and with it the previous render's elements —
+while earlier server-side side effects from that attempted render may remain. Treat
+template execution errors as application bugs: validate data before rendering and
+keep template actions infallible once they start emitting output or nested UI.
+
+`$.RadioGroup` has one attribution condition: its radio and label elements belong to
+the template whose body called it, not to the wrapper their markup lands in. Call it
+from the template that renders the group; see `RequestWriter.RadioGroup` for what
+happens when the two differ.
 
 You can also use explicit constructors through:
 

lib/ui/radiogroup.go

@@ -15,8 +15,10 @@ import (
 // renders register no elements on the [jaws.Request]. Call each of Radio and
 // Label at most once. Render Label only when Radio is also rendered: Label emits a
 // for="..." referencing the radio's id, so a Label without its Radio points at an
-// input that is absent from the document (and leaves an unrendered radio Element
-// registered on the Request for the request's lifetime).
+// input that is absent from the document. The radio Element it created is still
+// unregistered by the [Template] that owns it (see [RequestWriter.RadioGroup]) when
+// that template next replaces its content. With no template owner it has no DOM node
+// for a removal to report either, so it stays registered until the [jaws.Request] ends.
 type RadioElement struct {
 	st *radioState
 }
@@ -44,7 +46,14 @@ type radioGroupState struct {
 // ordered before the label's regardless of which is rendered first.
 func (st *radioState) radioElem() *jaws.Element {
 	if st.radio == nil {
+		// Create and report the Element rather than going through
+		// RequestWriter.NewUI: Radio and Label return their HTML for the template to
+		// place instead of writing it to the writer, and the Element has to exist
+		// before that render because its Jid supplies the group's name= and the
+		// label's for=. Reporting it here, at creation, also means a radio that is
+		// never rendered (a Label without its Radio) is still owned and reclaimed.
 		st.radio = st.rw.Request.NewElement(NewRadio(st.nb))
+		st.rw.trackElement(st.radio)
 		if st.group.nameAttr == "" {
 			st.group.nameAttr = `name="` + st.radio.Jid().String() + `"`
 		}
@@ -81,7 +90,9 @@ func (re RadioElement) Radio(params ...any) template.HTML {
 func (re RadioElement) Label(params ...any) template.HTML {
 	radio := re.st.radioElem()
 	if re.st.label == nil {
+		// Created and reported like the radio Element; see radioElem.
 		re.st.label = re.st.rw.Request.NewElement(NewLabel(re.st.nb))
+		re.st.rw.trackElement(re.st.label)
 	}
 	var sb strings.Builder
 	forAttr := string(radio.Jid().AppendQuote([]byte("for=")))
@@ -97,13 +108,16 @@ func (re RadioElement) Label(params ...any) template.HTML {
 // rendered radio in the group shares a name derived from the first created
 // radio Element's request-scoped [jaws.Jid].
 //
-// The radio and label Elements are created through [jaws.Request.NewElement] rather
-// than [RequestWriter.NewUI], so a surrounding [Template] neither owns nor unregisters
-// them when the template re-renders. Cleanup falls to the browser, which reports the
-// JaWS ids it removed from the DOM as the surrounding wrapper's new content is applied;
-// a rendered radio or label carries its own id, so it is reported. One that never
-// reached the DOM stays registered until the [jaws.Request] ends, including the
-// unrendered radio Element a [RadioElement.Label] without its Radio leaves behind.
+// The radio and label Elements belong to the [Template] whose body called RadioGroup,
+// which unregisters them when it next replaces its content. Ownership follows that call
+// site rather than the wrapper the markup lands in, so passing [RadioElement] values
+// into a nested wrapped template through its dot leaves them owned by the outer
+// template: an update of the inner wrapper alone replaces their DOM without their owner
+// reclaiming them, leaving that to the browser's removal acknowledgement for the ids
+// that reached the DOM and to the outer template's next update for any that did not.
+// Re-rendering them in the inner template is not an alternative, since [RadioElement]
+// allows Radio and Label at most one render each. Call RadioGroup from the template
+// that renders the group to avoid the condition entirely.
 func (rw RequestWriter) RadioGroup(nba *named.BoolArray) (rel []RadioElement) {
 	group := &radioGroupState{}
 	nba.ReadLocked(func(nbl []*named.Bool) {

lib/ui/register.go

@@ -33,13 +33,14 @@ func (u Register) JawsRender(elem *jaws.Element, w io.Writer, params []any) erro
 // The updater's [jaws.Updater.JawsUpdate] method will be called immediately to
 // ensure the initial rendering is correct.
 //
-// Register creates its Element through [jaws.Request.NewElement] rather than
-// [RequestWriter.NewUI], so a surrounding [Template] neither owns nor unregisters it
-// when the template re-renders. Cleanup falls to the browser, which reports the JaWS
-// ids it removed from the DOM as the surrounding wrapper's new content is applied. Use
-// the returned [jid.Jid] as an element id for that to work: an Element whose id never
-// reaches the DOM stays registered until the [jaws.Request] ends, and every re-render
-// of the template adds another.
+// A surrounding [Template] owns the Element and unregisters it when the template next
+// replaces its content, so repeated updates do not accumulate registrations. With no
+// template owner — rendered through a [RequestWriter] a caller built itself — cleanup
+// falls to the ordinary DOM-removal handling: the browser reports the JaWS ids it
+// removes when an ancestor's content is replaced, and [jaws.Element.Remove] unregisters
+// a managed child outright. Only an Element no removal ever reports, such as one whose
+// returned [jid.Jid] never becomes an element id, necessarily stays registered until the
+// [jaws.Request] ends.
 //
 // Register does not call [jaws.Renderer.JawsRender]. The updater must therefore
 // be ready for JawsUpdate and event handling without render-time initialization.
@@ -51,7 +52,13 @@ func (u Register) JawsRender(elem *jaws.Element, w io.Writer, params []any) erro
 //
 //	<div id="{{$.Register .MyUpdater}}">...</div>
 func (rw RequestWriter) Register(updater jaws.Updater, params ...any) jid.Jid {
+	// Create and report the Element rather than going through RequestWriter.NewUI, so
+	// a surrounding Template owns it without anything being rendered: NewUI calls
+	// JawsRender, which appends a debug comment when Jaws.Debug is set, and the
+	// documented usage puts the returned Jid inside an attribute
+	// (<div id="{{$.Register .X}}">), where that comment would corrupt the markup.
 	elem := rw.NewElement(Register{Updater: updater})
+	rw.trackElement(elem)
 	elem.Tag(updater)
 	// The wrapping Register element's UI is not the updater, so events reach the
 	// updater only through the element's handler list, not the elem.UI() fallback.

lib/ui/requestwriter.go

@@ -11,15 +11,23 @@ import (
 type RequestWriter struct {
 	*jaws.Request
 	io.Writer
-	// elementRendered, when non-nil, is called by NewUI with each Element it
-	// successfully rendered, letting the widget that owns this writer track the
-	// Elements created through it. A returned error fails the NewUI call.
+	// elementCreated, when non-nil, is called with every Element created through
+	// this writer, letting the widget that owns the writer track them. It is called
+	// as soon as the Element exists, before it renders and whether or not it ever
+	// does, so an implementation must not assume rendered state.
 	//
 	// It is deliberately unexported: RequestWriter is embedded in [With], so an
 	// exported func field would be callable from any template as
-	// {{call $.ElementRendered $.Element}}, letting a template make an Element own
+	// {{call $.ElementCreated $.Element}}, letting a template make an Element own
 	// itself and have its own wrapper unregistered on the next update.
-	elementRendered func(elem *jaws.Element) (err error)
+	elementCreated func(elem *jaws.Element)
+}
+
+// trackElement reports a newly created Element to the writer's owner, if any.
+func (rw RequestWriter) trackElement(elem *jaws.Element) {
+	if rw.elementCreated != nil {
+		rw.elementCreated(elem)
+	}
 }
 
 // NewUI creates an element for ui and renders it to the underlying writer.
@@ -28,12 +36,12 @@ type RequestWriter struct {
 // requirements documented by [jaws.UI].
 func (rw RequestWriter) NewUI(ui jaws.UI, params ...any) (err error) {
 	elem := rw.NewElement(ui)
-	if err = elem.JawsRender(rw, params); err == nil {
-		if rw.elementRendered != nil {
-			err = rw.elementRendered(elem)
-		}
-	}
-	if err != nil {
+	// Report the Element before rendering it, so the owner's set is complete even for
+	// one that fails. That set may then hold an Element already unregistered below,
+	// which costs nothing: Request.DeleteElements skips elements it finds
+	// unregistered, and every rollback path deletes the whole set at once.
+	rw.trackElement(elem)
+	if err = elem.JawsRender(rw, params); err != nil {
 		// Unregister anything the failed Element already owns along with it, so no
 		// widget can strand a subtree by not rolling back itself.
 		deleteOwnedElements(rw.Request, []*jaws.Element{elem})

lib/ui/template.go

@@ -28,20 +28,19 @@ import (
 // helper. The referenced template must be a partial template, not a full HTML
 // document.
 //
-// The Elements a template creates through [RequestWriter.NewUI] — the path taken by
-// every RequestWriter widget helper except [RequestWriter.Register] and
-// [RequestWriter.RadioGroup], including a nested [RequestWriter.Template] — belong to
-// the Template that rendered them: when [Template.JawsUpdate] replaces the wrapper's
-// content, those Elements are unregistered along with the DOM that held them, and a
-// nested widget's own Elements go with it.
+// Every Element a template creates through the [RequestWriter] it is given belongs to
+// the Template that rendered it — the widget helpers, [RequestWriter.Register],
+// [RequestWriter.RadioGroup] and a nested [RequestWriter.Template] alike. When
+// [Template.JawsUpdate] replaces the wrapper's content, those Elements are
+// unregistered along with the DOM that held them, and a nested widget's own Elements
+// go with it.
 //
-// Those two helpers create their Elements through [jaws.Request.NewElement] instead,
-// so a Template neither tracks nor unregisters them. Their cleanup is left to the
-// browser, which reports the JaWS ids it removed from the DOM as the wrapper's new
-// content is applied, and the [jaws.Request] unregisters those Elements. An Element
-// whose id never reaches the DOM has nothing to report it and stays registered until
-// the Request ends: one from an execution that failed before its markup was
-// delivered, or from a Register call whose returned Jid the template discards.
+// Ownership is recorded when an Element is created rather than after it renders, so
+// one that never reaches the browser is reclaimed too: an Element whose render failed,
+// or a radio Element left unrendered by a [RadioElement.Label] without its
+// [RadioElement.Radio]. See [RequestWriter.RadioGroup] for the one attribution
+// condition, which applies when a group's markup ends up in a different wrapper than
+// the RadioGroup call.
 //
 // Template execution is best-effort rather than transactional. Template actions
 // and nested JaWS helpers run as the template executes, so an execution error
@@ -81,12 +80,12 @@ func (tmpl *Template) String() string {
 }
 
 // ownElement records child as created while tmpl's template executed. It is the
-// [RequestWriter] element-rendered hook installed by execute.
-func (tmpl *Template) ownElement(child *jaws.Element) (err error) {
+// [RequestWriter] element-created hook installed by execute, so it is called as soon
+// as the Element exists and makes no assumption about whether it rendered.
+func (tmpl *Template) ownElement(child *jaws.Element) {
 	tmpl.mu.Lock()
 	tmpl.owned = append(tmpl.owned, child)
 	tmpl.mu.Unlock()
-	return
 }
 
 // takeOwnedElements returns the Elements created by the most recent execution and
@@ -137,7 +136,7 @@ func (tmpl *Template) execute(elem *jaws.Element, w io.Writer, lookedUp *templat
 	// races on the shared io.Writer, and on the render as a whole.
 	err = lookedUp.Execute(w, With{
 		Element:       elem,
-		RequestWriter: RequestWriter{Request: elem.Request, Writer: w, elementRendered: tmpl.ownElement},
+		RequestWriter: RequestWriter{Request: elem.Request, Writer: w, elementCreated: tmpl.ownElement},
 		Dot:           tmpl.Dot,
 		Auth:          tmpl.auth(elem),
 	})