Code-quality review: documentation-only improvements

broadcast.go

@@ -139,6 +139,22 @@ func sortedDirtTags(dirty map[any]int) []any {
 	return dirt
 }
 
+// distributeDirt drains the accumulated dirty tags and appends them to every live
+// Request's pending-dirt list for the next update pass.
+//
+// It snapshots the Request set under jw.mu, releases it, then appends to each Request
+// without holding jw.mu. Two consequences are deliberate and harmless:
+//
+//   - The snapshot includes pending (not-yet-running) Requests whose process loop has
+//     not started, so their todoDirt buffers until they connect or are recycled
+//     (bounded by the request timeout). This is intentional: a value mutated in the
+//     render-to-connect window is then reflected on the first update pass.
+//   - A Request can be recycled and reused for a different connection between the
+//     snapshot and the append. appendDirtyTags and clearLocked both take rq.mu, so
+//     there is no data race, and stale tags landing in a reborn Request resolve to
+//     nothing in [Request.makeUpdateList] against its freshly emptied tagMap (at worst
+//     a redundant re-render). Unlike destKey/cancelIfCurrent this path needs no
+//     key-identity guard because applying dirt is idempotent.
 func (jw *Jaws) distributeDirt() int {
 	var reqs []*Request
 	var dirt []any

contracts.go

@@ -9,17 +9,19 @@ import (
 // Container is implemented by UI values that render a dynamic list of child
 // [UI] values.
 type Container interface {
-	// JawsContains must return a slice of comparable [UI] objects (they are used
-	// as map keys; see [UI] for the comparability requirement). The slice contents
-	// must not be modified after returning it.
+	// JawsContains returns the current child [UI] values contained by elem.
+	//
+	// The returned [UI] values must be comparable, since they are used as map keys
+	// (see [UI] for the comparability requirement), and the slice contents must not
+	// be modified after returning it.
 	JawsContains(elem *Element) (contents []UI)
 }
 
 // InitHandler allows initializing UI getters and setters before their use.
 //
 // You can of course initialize them in the call from the template engine,
-// but at that point you don't have access to the [Element], [Element.Context]
-// or [Element.Session].
+// but at that point you don't have access to the [Element], [Request.Context]
+// or [Request.Session].
 type InitHandler interface {
 	JawsInit(elem *Element) (err error)
 }
@@ -35,6 +37,7 @@ type Logger interface {
 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]).
 	JawsRender(elem *Element, w io.Writer, params []any) error
 }
 
@@ -62,6 +65,7 @@ type UI interface {
 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]).
 	JawsUpdate(elem *Element)
 }
 

element.go

@@ -44,8 +44,9 @@ func (elem *Element) String() string {
 
 // appendHandlers is the single internal chokepoint for mutating elem.handlers.
 //
-// handlers is read lock-free on the event goroutine (see callEventHandlers), so
-// it must only be appended to while the Element is being rendered, before any
+// handlers is read lock-free on the event goroutine (via [CallEventHandlers], which
+// calls the internal callEventHandlers), so it must only be appended to while the
+// Element is being rendered, before any
 // event for it can fire. Once frozen, late mutations are a bug: reportMisuse
 // panics in debug builds and logs in production, and the mutation is dropped
 // rather than racing the lock-free read.
@@ -160,7 +161,13 @@ func (elem *Element) JawsUpdate() {
 
 // queue enqueues a wire message of the given type and data for this element on
 // its Request, tagged with the element's Jid. It is a no-op once the element has
-// been deleted. Call only during JawsRender or JawsUpdate processing.
+// been deleted.
+//
+// It is intended to be called while the element is rendering or updating; the
+// message is appended to the Request's muQueue-guarded outbound queue and flushed on
+// the next processing-loop pass. Calling it from an event handler is safe but defers
+// delivery to that pass, so the event-driven path is to mark the element dirty (see
+// [Request.Dirty]), which schedules a [Updater.JawsUpdate].
 func (elem *Element) queue(wht what.What, data string) {
 	if !elem.deleted.Load() {
 		elem.Request.queue(wire.WsMsg{
@@ -177,47 +184,65 @@ func (elem *Element) queue(wht what.What, data string) {
 // The value parameter must be the unescaped logical attribute value. It is sent
 // to the browser DOM and used as the value argument to setAttribute().
 //
-// Call this only during JawsRender() or JawsUpdate() processing.
+// Call this while the [Element] is rendering or updating. The change is queued and
+// sent on the next processing pass; to change the [Element] in response to a browser
+// event, mark it dirty with [Request.Dirty] instead, since calling this from an event
+// handler only defers the change to the next pass.
 func (elem *Element) SetAttr(attr, value string) {
 	elem.queue(what.SAttr, attr+"\n"+value)
 }
 
 // RemoveAttr queues sending a request to remove an attribute
 // to the browser for the [Element].
 //
-// Call this only during JawsRender() or JawsUpdate() processing.
+// Call this while the [Element] is rendering or updating. The change is queued and
+// sent on the next processing pass; to change the [Element] in response to a browser
+// event, mark it dirty with [Request.Dirty] instead, since calling this from an event
+// handler only defers the change to the next pass.
 func (elem *Element) RemoveAttr(attr string) {
 	elem.queue(what.RAttr, attr)
 }
 
 // SetClass queues sending a class
 // to the browser for the [Element].
 //
-// Call this only during JawsRender() or JawsUpdate() processing.
+// Call this while the [Element] is rendering or updating. The change is queued and
+// sent on the next processing pass; to change the [Element] in response to a browser
+// event, mark it dirty with [Request.Dirty] instead, since calling this from an event
+// handler only defers the change to the next pass.
 func (elem *Element) SetClass(cls string) {
 	elem.queue(what.SClass, cls)
 }
 
 // RemoveClass queues sending a request to remove a class
 // to the browser for the [Element].
 //
-// Call this only during JawsRender() or JawsUpdate() processing.
+// Call this while the [Element] is rendering or updating. The change is queued and
+// sent on the next processing pass; to change the [Element] in response to a browser
+// event, mark it dirty with [Request.Dirty] instead, since calling this from an event
+// handler only defers the change to the next pass.
 func (elem *Element) RemoveClass(cls string) {
 	elem.queue(what.RClass, cls)
 }
 
 // SetInner queues sending new inner HTML content
 // to the browser for the [Element].
 //
-// Call this only during JawsRender() or JawsUpdate() processing.
+// Call this while the [Element] is rendering or updating. The change is queued and
+// sent on the next processing pass; to change the [Element] in response to a browser
+// event, mark it dirty with [Request.Dirty] instead, since calling this from an event
+// handler only defers the change to the next pass.
 func (elem *Element) SetInner(innerHTML template.HTML) {
 	elem.queue(what.Inner, string(innerHTML))
 }
 
 // SetValue queues sending a new current input value in textual form
 // to the browser for the [Element].
 //
-// Call this only during JawsRender() or JawsUpdate() processing.
+// Call this while the [Element] is rendering or updating. The change is queued and
+// sent on the next processing pass; to change the [Element] in response to a browser
+// event, mark it dirty with [Request.Dirty] instead, since calling this from an event
+// handler only defers the change to the next pass.
 func (elem *Element) SetValue(value string) {
 	elem.queue(what.Value, value)
 }
@@ -228,7 +253,10 @@ func (elem *Element) SetValue(value string) {
 // is a programming error: debug builds panic and production builds report it via
 // [Jaws.MustLog] and skip the replacement.
 //
-// Call this only during JawsRender() or JawsUpdate() processing.
+// Call this while the [Element] is rendering or updating. The change is queued and
+// sent on the next processing pass; to change the [Element] in response to a browser
+// event, mark it dirty with [Request.Dirty] instead, since calling this from an event
+// handler only defers the change to the next pass.
 func (elem *Element) Replace(htmlCode template.HTML) {
 	if !elem.deleted.Load() {
 		var b []byte
@@ -246,14 +274,20 @@ func (elem *Element) Replace(htmlCode template.HTML) {
 
 // Append appends a new HTML element as a child to the current one.
 //
-// Call this only during JawsRender() or JawsUpdate() processing.
+// Call this while the [Element] is rendering or updating. The change is queued and
+// sent on the next processing pass; to change the [Element] in response to a browser
+// event, mark it dirty with [Request.Dirty] instead, since calling this from an event
+// handler only defers the change to the next pass.
 func (elem *Element) Append(htmlCode template.HTML) {
 	elem.queue(what.Append, string(htmlCode))
 }
 
 // Order reorders the HTML elements.
 //
-// Call this only during JawsRender() or JawsUpdate() processing.
+// Call this while the [Element] is rendering or updating. The change is queued and
+// sent on the next processing pass; to change the [Element] in response to a browser
+// event, mark it dirty with [Request.Dirty] instead, since calling this from an event
+// handler only defers the change to the next pass.
 func (elem *Element) Order(jidList []jid.Jid) {
 	if !elem.deleted.Load() && len(jidList) > 0 {
 		var b []byte
@@ -270,7 +304,10 @@ func (elem *Element) Order(jidList []jid.Jid) {
 // Remove requests that the HTML child with the given HTML ID of this [Element]
 // is removed from the [Request] and its HTML element from the browser.
 //
-// Call this only during JawsRender() or JawsUpdate() processing.
+// Call this while the [Element] is rendering or updating. The change is queued and
+// sent on the next processing pass; to change the [Element] in response to a browser
+// event, mark it dirty with [Request.Dirty] instead, since calling this from an event
+// handler only defers the change to the next pass.
 func (elem *Element) Remove(htmlID string) {
 	elem.queue(what.Remove, htmlID)
 }

errvalueunchanged.go

@@ -2,7 +2,11 @@ package jaws
 
 import "errors"
 
-// ErrValueUnchanged can be returned from JawsSet methods
-// to indicate that while there was no error, the underlying value
-// was already the desired value.
+// ErrValueUnchanged reports a successful no-op set: there was no error, but the
+// underlying value already equaled the desired value.
+//
+// Setter-style implementations (the JawsSet / JawsSetPath methods in
+// github.com/linkdata/jaws/lib/ui and github.com/linkdata/jaws/jawstree) return it,
+// and callers test for it with [errors.Is]. It lives in this package so all
+// implementations share one error identity.
 var ErrValueUnchanged = errors.New("value unchanged")

eventhandler.go

@@ -8,12 +8,20 @@ import (
 	"github.com/linkdata/jaws/lib/what"
 )
 
-// ErrEventHandlerPanic is returned when an event handler panics.
+// ErrEventHandlerPanic is returned by [CallEventHandlers] when a user event handler
+// panics.
+//
+// Match it with [errors.Is]. When the recovered panic value is itself an error it is
+// available via Unwrap (and thus [errors.As] / [errors.Is]); a non-error panic value
+// appears only in the formatted message.
 var ErrEventHandlerPanic errEventHandlerPanic
 
 type errEventHandlerPanic struct {
+	// Type is the [Element]'s UI object type. Handlers registered on the Element are
+	// tried before the UI object, so the type that actually panicked may differ from
+	// this when a registered handler is the culprit.
 	Type  reflect.Type
-	Value any
+	Value any // the recovered panic value
 }
 
 func (e errEventHandlerPanic) Error() string {

jaws.go

@@ -378,6 +378,9 @@ func MakeID() string {
 //
 // Automatic timeout handling is performed by [Jaws.ServeWithTimeout]. The default
 // [Jaws.Serve] helper uses a 10-second timeout.
+//
+// It panics if the system CSPRNG ([crypto/rand]) fails while generating the request
+// key, which does not happen on supported platforms.
 func (jw *Jaws) NewRequest(r *http.Request) (rq *Request) {
 	remoteIP := jw.clientIP(r)
 

request.go

@@ -42,6 +42,12 @@ type ConnectFn = func(rq *Request) error
 //
 // Note that we have to store the context inside the struct because there is no call chain
 // between the Request being created and it being used once the WebSocket is created.
+//
+// Unlike [Session], whose methods are nil-safe, Request methods are not safe to call on a
+// nil *Request: a Request is always obtained from [Jaws.NewRequest] or [Jaws.UseRequest]
+// and is never legitimately nil. The nil-receiver guards on [Request.Log] and
+// [Request.MustLog] exist only so a nil Request can be rendered into error text, not as a
+// public nil-safe contract.
 type Request struct {
 	Jaws       *Jaws                   // (read-only) the JaWS instance the Request belongs to
 	JawsKey    key.Key                 // (read-only) random key identifying this Request in the WebSocket URI and the broadcast/tail target; read under mu by the identity check (destKey, wantMessage) and the render path (JawsKeyString, HeadHTML)
@@ -225,6 +231,12 @@ func (rq *Request) clearLocked() *Request {
 	rq.remoteIP = netip.Addr{}
 	for _, e := range rq.elems {
 		if e != nil {
+			// Nil the GC-reachable fields and set the deleted guard, which makes any
+			// retained *Element inert (see [Element.Deleted]). The Request back-pointer
+			// and frozen flag are deliberately left as-is: Elements are allocated fresh
+			// per newElementLocked and never pooled, so a stale frozen value can never
+			// be observed by a reused Element. Any future move to pool Elements must
+			// also reset frozen, since it gates the lock-free handler read.
 			e.handlers = nil
 			e.ui = nil
 			e.deleted.Store(true)
@@ -402,15 +414,18 @@ func (rq *Request) Initial() (r *http.Request) {
 	return
 }
 
-// Get is shorthand for Session().Get and returns the session value associated with the key, or nil.
-// If no session is associated with the [Request], it returns nil.
+// Get is shorthand for [Session.Get].
+//
+// It returns the session value associated with key, or nil if no session is associated
+// with the [Request].
 func (rq *Request) Get(key string) any {
 	return rq.Session().Get(key)
 }
 
-// Set is shorthand for Session().Set and sets a session value to be associated with the key.
-// If value is nil, the key is removed from the session.
-// Does nothing if no session is associated with the [Request].
+// Set is shorthand for [Session.Set].
+//
+// It associates value with key in the session; a nil value removes the key. It does
+// nothing if no session is associated with the [Request].
 func (rq *Request) Set(key string, value any) {
 	rq.Session().Set(key, value)
 }
@@ -532,7 +547,8 @@ func (rq *Request) Alert(level, msg string) {
 	}
 }
 
-// AlertError calls [Request.Alert] if the given error is not nil.
+// AlertError logs err via [Jaws.Log] and, if it is non-nil, also shows it to the
+// current request as a danger-level [Request.Alert].
 func (rq *Request) AlertError(err error) {
 	if rq.Jaws.Log(err) != nil {
 		rq.Alert("danger", err.Error())
@@ -675,6 +691,10 @@ func (rq *Request) HasTag(elem *Element, tagValue any) (yes bool) {
 // appendDirtyTags queues already-expanded tags onto this request's pending-dirt
 // list. The Serve loop's update tick later drains the list (see makeUpdateList)
 // and re-renders the affected elements. Takes rq.mu.
+//
+// It may run on a Request that was recycled and reused after the caller's dirt
+// snapshot was taken; that is race-free (clearLocked also takes rq.mu) and harmless,
+// as explained on distributeDirt.
 func (rq *Request) appendDirtyTags(tags []any) {
 	rq.mu.Lock()
 	rq.todoDirt = append(rq.todoDirt, tags...)

session.go

@@ -344,6 +344,9 @@ func (jw *Jaws) GetSession(r *http.Request) (sess *Session) {
 // As a side effect, the session cookie is also added to r itself, so the new
 // [Session] is visible to [Jaws.GetSession] and [Jaws.NewRequest] for the
 // remainder of the same HTTP request.
+//
+// It panics if the system CSPRNG ([crypto/rand]) fails while generating the session
+// ID, which does not happen on supported platforms.
 func (jw *Jaws) NewSession(w http.ResponseWriter, r *http.Request) (sess *Session) {
 	if r != nil {
 		if oldSess := jw.GetSession(r); oldSess != nil {