Skip to content

ui: Register and RadioGroup bypass RequestWriter.NewUI, so a Template does not track their Elements #222

Description

@linkdata

Summary

RequestWriter.Register and RequestWriter.RadioGroup create their Elements with
Request.NewElement instead of RequestWriter.NewUI, so they are the only widget
helpers a surrounding ui.Template does not track. A template that updates registers a
fresh set on every execution and unregisters none; reclaiming the previous set is left
entirely to the browser's removal acknowledgement, which can only name ids that reached
the DOM.

This is the gap deliberately deferred by #221 (which fixed #216 for every NewUI-backed
helper) and documented as an exception on Template, RequestWriter.Register,
RequestWriter.RadioGroup and in the lib/ui README.

Class / severity

Resource lifecycle — Medium. With a cooperating browser the common shape
(<div id="{{$.Register .Updater}}">) reaches a steady state, because applying the
wrapper's new inner content makes jawsRemoving report the old id and handleRemove
unregisters it. Growth is unbounded whenever the id never reaches the DOM:

  • {{$.Register .Updater}} whose returned Jid is discarded or written as text rather
    than used as an element id.
  • RadioElement.Label rendered without its Radio, which leaves the radio Element
    registered but unrendered (already noted on RadioElement).
  • An execution that fails after creating them, since Template.JawsUpdate discards the
    output and the markup never reaches the browser.

Separately, it makes server-side Element lifetime depend on the client for these two
helpers while every other helper is now deterministic server-side.

Contract and valid usage

RequestWriter.Register documents the <div id="{{$.Register .MyUpdater}}"> form, and
RadioGroup is meant to be ranged over in a template body. Calling either from a
template that later updates is supported usage.

Reproduction

package ui_test

import (
	"html/template"
	"testing"
	"time"

	"github.com/linkdata/jaws"
	"github.com/linkdata/jaws/jawstest"
	"github.com/linkdata/jaws/lib/ui"
	"github.com/linkdata/jaws/lib/what"
	"github.com/linkdata/jaws/lib/wire"
)

// registerDot is the template dot and the $.Register updater, so one tag covers
// the wrapper and the registered Element.
type registerDot struct{}

func (*registerDot) JawsUpdate(*jaws.Element) {}

func TestRegisterInsideUpdatedTemplateIsNotTracked(t *testing.T) {
	jw, err := jaws.New()
	if err != nil {
		t.Fatal(err)
	}
	t.Cleanup(jw.Close)

	lookuper := template.Must(template.New("templates").Parse(
		`{{define "parent"}}<div id="{{$.RequestWriter.Register $.Dot}}"></div>{{end}}`,
	))
	if err := jw.AddTemplateLookuper(lookuper); err != nil {
		t.Fatal(err)
	}
	go jw.Serve()

	tr := jawstest.NewTestRequest(jw, nil)
	t.Cleanup(func() {
		tr.Close()
		<-tr.DoneCh
	})
	<-tr.ReadyCh

	dot := &registerDot{}
	rw := ui.RequestWriter{Request: tr.Request, Writer: tr.Recorder}
	if err := rw.Template("div", "parent", dot); err != nil {
		t.Fatal(err)
	}
	if got := len(tr.GetElements(dot)); got != 2 {
		t.Fatalf("initial tagged elements = %d, want 2 (wrapper and registered)", got)
	}

	for round := 1; round <= 3; round++ {
		tr.BcastCh <- wire.Message{Dest: dot, What: what.Update}
		select {
		case <-tr.OutCh:
		case <-time.After(time.Second):
			t.Fatalf("round %d: timeout waiting for the wrapper update", round)
		}
		if got := len(tr.GetElements(dot)); got != 2 {
			t.Fatalf("tagged elements after %d update(s) = %d, want 2", round, got)
		}
	}
}

The count is 3 after the first update and grows by one per update. lib/ui has an
equivalent characterization test pinning the current behavior for both helpers,
TestTemplate_UpdateDoesNotTrackRegisterOrRadioGroup, which covers RadioGroup
(2 Elements per execution: the input and its label).

Root cause

RequestWriter.NewUI is the only place the unexported elementRendered hook fires
(lib/ui/requestwriter.go), and that hook is how a ui.Template records ownership.
RequestWriter.Register (lib/ui/register.go:54) and the lazy radio/label creation in
radioState.radioElem / RadioElement.Label (lib/ui/radiogroup.go:47,
lib/ui/radiogroup.go:84) call Request.NewElement directly and never notify.

Suggested fix

Notify the hook from both helpers, most likely through a small unexported
RequestWriter method wrapping the nil check, so ui.Template owns their Elements like
any other nested UI. Two details need deciding:

  • When to notify. NewUI notifies after a successful render. RadioGroup creates the
    radio Element lazily in radioElem() and renders it later in Radio(), and a Label
    without its Radio leaves it unrendered — the case that leaks today. Notifying at
    creation rather than after render covers it, but widens the hook's meaning from
    "rendered" to "created through this writer".
  • Hook errors. Register returns a jid.Jid, not an error, so it cannot propagate a
    hook failure the way NewUI does. The only implementation never fails, so either
    ignore it or report it through MustLog — but pick deliberately rather than by
    omission.

Both helpers' doc comments, the Template doc, the lib/ui README and
.agents/skills/jaws/SKILL.md state the current exception and need updating with the
fix. TestTemplate_UpdateDoesNotTrackRegisterOrRadioGroup asserts today's growth, so it
will fail and is the place to invert the expectation.

Confidence

Confirmed on the #221 branch (c5e2cd9), where the behavior is pinned by a test.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions