fix(state): don't drop effects under render-phase convergence (v3.5.4)

CHANGELOG.md

@@ -1,5 +1,21 @@
 # Changelog
 
+## v3.5.4 - 2026-06-25
+
+### Fixed
+
+- **Effects were dropped when a component used render-phase updates (regression in
+  v3.5.3).** The render-phase convergence loop cleared the fiber's per-iteration
+  effect list before each re-run; combined with the deps-equality check (a
+  stable-deps effect does not re-register when its deps are unchanged), the effect
+  ended up registered in no iteration and never ran. The loop no longer clears the
+  effect list — effects accumulate across convergence iterations and are then
+  de-duplicated by cleanup-index (keeping the last, i.e. the final render's effect
+  per slot). Each effect now runs exactly once with the converged state, for both
+  stable and changing deps, and multiple effects run once each in declaration
+  order. Covered by `TestRenderPhaseEffectRunsOnceStableDeps` /
+  `...ChangingDeps` / `TestRenderPhaseMultipleEffectsEachOnce`.
+
 ## v3.5.3 - 2026-06-25
 
 ### Fixed

examples/public-examples-site/assets/docs/latest-release-notes.md

@@ -3,18 +3,19 @@
 This docs-site page mirrors the first section returned by
 `tools/changelogcheck.LatestEntry(CHANGELOG.md)`.
 
-## v3.5.3 - 2026-06-25
+## v3.5.4 - 2026-06-25
 
-The current latest changelog section is `v3.5.3 - 2026-06-25`. **Fixed:** a
-render-phase state update (a component calling its own `setState` during render —
-the derived-state pattern) left the DOM showing a value the component never
-actually rendered and never converged. `renderFunctionComponent` now detects a
-render-phase update (via a new `activeRenderFiber` flag, distinct from the ambient
-current fiber so it never trips on the SSR path or hook unit tests) and re-runs
-the component to convergence — bounded at 25 iterations with a diagnostic to guard
-against an unconditional-setState infinite loop — matching React's "keeps
-restarting until there are no more new updates." Found while bug-hunting GWC's
-state management against React. It is checked by the blocking `tools/changelogcheck`
-release gate before a versioned release can ship.
+The current latest changelog section is `v3.5.4 - 2026-06-25`. **Fixed:** a
+regression in v3.5.3 where effects were dropped when a component used render-phase
+updates. The convergence loop cleared the fiber's per-iteration effect list before
+each re-run; combined with the deps-equality check (a stable-deps effect does not
+re-register when its deps are unchanged), the effect ended up registered in no
+iteration and never ran. The loop no longer clears the effect list — effects
+accumulate across iterations and are then de-duplicated by cleanup-index (keeping
+the final render's effect per slot), so each effect runs exactly once with the
+converged state (for both stable and changing deps), and multiple effects run once
+each in declaration order. Found by stress-testing the v3.5.3 fix. It is checked by
+the blocking `tools/changelogcheck` release gate before a versioned release can
+ship.
 
 See the repository root `CHANGELOG.md` for the full entry body.

internal/runtime/hot_reload.go

@@ -911,8 +911,16 @@ func (parseRt *Runtime) renderFunctionComponent(parseFiber *Fiber) (*Element, bo
 		// this render (parseFiber.renderPhaseUpdate, set by the hook setter), re-run
 		// it with the new state until it stops updating — bounded like React's
 		// re-render limit — so the committed output reflects the final state instead
-		// of a half-rendered intermediate.
+		// of a half-rendered intermediate. The hook indices are reset per iteration
+		// but the deps/cleanups/effects arrays persist; effects accumulate across
+		// iterations (the deps-equality check makes a stable effect register only
+		// once, while a deps-changing effect registers per iteration), so they are
+		// de-duplicated by cleanup index afterwards to commit exactly the final
+		// render's effect per slot. NOTE: do not clear parseFiber.effects here —
+		// clearing it loses an effect whose deps did not change across iterations.
+		parseConverged := false
 		for parseRPIters := 0; parseFiber.renderPhaseUpdate; parseRPIters++ {
+			parseConverged = true
 			parseFiber.renderPhaseUpdate = false
 			if parseRPIters >= 25 {
 				ReportDiagnosticWithContext("runtime", DiagnosticError,
@@ -934,9 +942,6 @@ func (parseRt *Runtime) renderFunctionComponent(parseFiber *Fiber) (*Element, bo
 				parseFiber.hooks.cleanupIndex = 0
 				parseFiber.hooks.signature = parseFiber.hooks.signature[:0]
 			}
-			if parseFiber.effects != nil {
-				parseFiber.effects = parseFiber.effects[:0]
-			}
 			SetCurrentFiber(parseFiber)
 			parseRt.activeRenderFiber = parseFiber
 			func() {
@@ -954,6 +959,9 @@ func (parseRt *Runtime) renderFunctionComponent(parseFiber *Fiber) (*Element, bo
 				}
 			}()
 		}
+		if parseConverged {
+			parseFiber.effects = dedupRenderPhaseEffects(parseFiber.effects)
+		}
 
 		if parseAttempt == 0 && parseRestore != nil {
 			if !componentSnapshotSerializableCompatible(parseRestore, parseFiber) {
@@ -975,3 +983,33 @@ func (parseRt *Runtime) renderFunctionComponent(parseFiber *Fiber) (*Element, bo
 
 	return nil, false, nil
 }
+
+// dedupRenderPhaseEffects collapses the effects accumulated across render-phase
+// convergence iterations to exactly one per cleanup-index slot — the last one,
+// i.e. the effect produced by the final converged render — preserving
+// declaration (cleanup-index) order. Without this, an effect whose deps change as
+// state converges would be registered (and run) once per iteration.
+func dedupRenderPhaseEffects(parseEffects []Effect) []Effect {
+	if len(parseEffects) <= 1 {
+		return parseEffects
+	}
+	parseLastBySlot := make(map[int]Effect, len(parseEffects))
+	parseMaxSlot := -1
+	for _, parseEffect := range parseEffects {
+		parseLastBySlot[parseEffect.CleanupIndex] = parseEffect
+		if parseEffect.CleanupIndex > parseMaxSlot {
+			parseMaxSlot = parseEffect.CleanupIndex
+		}
+	}
+	if len(parseLastBySlot) == len(parseEffects) {
+		// No duplicates (every slot appeared once); keep the original order.
+		return parseEffects
+	}
+	parseOut := parseEffects[:0]
+	for parseSlot := 0; parseSlot <= parseMaxSlot; parseSlot++ {
+		if parseEffect, parseOk := parseLastBySlot[parseSlot]; parseOk {
+			parseOut = append(parseOut, parseEffect)
+		}
+	}
+	return parseOut
+}

ui/render_phase_effects_test.go

@@ -0,0 +1,101 @@
+//go:build !js || !wasm
+
+package ui_test
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/monstercameron/GoWebComponents/internal/platform/mockdom"
+	"github.com/monstercameron/GoWebComponents/internal/runtime"
+)
+
+// Regression test for v3.5.4: render-phase convergence (v3.5.3) must commit
+// exactly the final render's effects — running each effect once, with the
+// converged state — for both stable and changing deps. The v3.5.3 convergence
+// loop cleared the per-iteration effect list, which (combined with the
+// deps-equality check) dropped a stable-deps effect entirely.
+func TestRenderPhaseEffectRunsOnceStableDeps(t *testing.T) {
+	adapter := mockdom.NewMockDOMAdapter()
+	rt := runtime.NewRuntime(runtime.Config{DOMAdapter: adapter, Reset: true})
+	root := adapter.CreateElement("div")
+
+	parseEffectRuns := 0
+	var parseSaw []int
+	rt.RenderInto(root, runtime.CreateElement(func() *runtime.Element {
+		parseGet, parseSet := runtime.GoUseState(rt, 0)
+		if parseGet() < 3 {
+			parseSet(parseGet() + 1)
+		}
+		runtime.GoUseEffect(func() func() {
+			parseEffectRuns++
+			parseSaw = append(parseSaw, parseGet())
+			return nil
+		}, 0) // stable deps
+		return runtime.CreateElement("span", map[string]any{}, fmt.Sprint(parseGet()))
+	}, map[string]any{}))
+
+	if parseEffectRuns != 1 {
+		t.Errorf("effect runs = %d, want 1 (only the final converged render), saw=%v", parseEffectRuns, parseSaw)
+	}
+	if len(parseSaw) != 1 || parseSaw[0] != 3 {
+		t.Errorf("effect observed %v, want [3] (the converged state)", parseSaw)
+	}
+}
+
+func TestRenderPhaseEffectRunsOnceChangingDeps(t *testing.T) {
+	adapter := mockdom.NewMockDOMAdapter()
+	rt := runtime.NewRuntime(runtime.Config{DOMAdapter: adapter, Reset: true})
+	root := adapter.CreateElement("div")
+
+	parseEffectRuns := 0
+	var parseSaw []int
+	rt.RenderInto(root, runtime.CreateElement(func() *runtime.Element {
+		parseGet, parseSet := runtime.GoUseState(rt, 0)
+		if parseGet() < 3 {
+			parseSet(parseGet() + 1)
+		}
+		runtime.GoUseEffect(func() func() {
+			parseEffectRuns++
+			parseSaw = append(parseSaw, parseGet())
+			return nil
+		}, parseGet()) // deps change as state converges
+		return runtime.CreateElement("span", map[string]any{}, fmt.Sprint(parseGet()))
+	}, map[string]any{}))
+
+	if parseEffectRuns != 1 {
+		t.Errorf("changing-deps effect runs = %d, want 1 (deduped to the final render), saw=%v", parseEffectRuns, parseSaw)
+	}
+	if len(parseSaw) != 1 || parseSaw[0] != 3 {
+		t.Errorf("changing-deps effect observed %v, want [3]", parseSaw)
+	}
+}
+
+// Two effects in a render-phase-converging component: each runs once, in order,
+// with the converged state.
+func TestRenderPhaseMultipleEffectsEachOnce(t *testing.T) {
+	adapter := mockdom.NewMockDOMAdapter()
+	rt := runtime.NewRuntime(runtime.Config{DOMAdapter: adapter, Reset: true})
+	root := adapter.CreateElement("div")
+
+	var parseLog []string
+	rt.RenderInto(root, runtime.CreateElement(func() *runtime.Element {
+		parseGet, parseSet := runtime.GoUseState(rt, 0)
+		if parseGet() < 2 {
+			parseSet(parseGet() + 1)
+		}
+		runtime.GoUseEffect(func() func() {
+			parseLog = append(parseLog, fmt.Sprintf("a:%d", parseGet()))
+			return nil
+		}, 0)
+		runtime.GoUseEffect(func() func() {
+			parseLog = append(parseLog, fmt.Sprintf("b:%d", parseGet()))
+			return nil
+		}, 0)
+		return runtime.CreateElement("span", map[string]any{}, fmt.Sprint(parseGet()))
+	}, map[string]any{}))
+
+	if len(parseLog) != 2 || parseLog[0] != "a:2" || parseLog[1] != "b:2" {
+		t.Errorf("two effects log = %v, want [a:2 b:2] (each once, converged, in order)", parseLog)
+	}
+}