Skip to content

Commit af19bbf

Browse files
committed
update atom. deep copy on SetFn. warning for data mutation
1 parent b6f12b3 commit af19bbf

2 files changed

Lines changed: 55 additions & 3 deletions

File tree

tsunami/app/atom.go

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package app
55

66
import (
77
"log"
8+
"reflect"
89
"runtime"
910

1011
"github.com/wavetermdev/waveterm/tsunami/engine"
@@ -21,6 +22,42 @@ func logInvalidAtomSet(atomName string) {
2122
}
2223
}
2324

25+
// sameRef returns true if oldVal and newVal share the same underlying reference
26+
// (pointer, map, or slice). Nil values return false.
27+
func sameRef[T any](oldVal, newVal T) bool {
28+
vOld := reflect.ValueOf(oldVal)
29+
vNew := reflect.ValueOf(newVal)
30+
31+
if !vOld.IsValid() || !vNew.IsValid() {
32+
return false
33+
}
34+
35+
switch vNew.Kind() {
36+
case reflect.Ptr:
37+
// direct comparison works for *T
38+
return any(oldVal) == any(newVal)
39+
40+
case reflect.Map, reflect.Slice:
41+
if vOld.Kind() != vNew.Kind() || vOld.IsZero() || vNew.IsZero() {
42+
return false
43+
}
44+
return vOld.Pointer() == vNew.Pointer()
45+
}
46+
47+
// primitives, structs, etc. → not a reference type
48+
return false
49+
}
50+
51+
// logMutationWarning logs a warning when mutation is detected
52+
func logMutationWarning(atomName string) {
53+
_, file, line, ok := runtime.Caller(2)
54+
if ok {
55+
log.Printf("WARNING: atom '%s' appears to be mutated instead of copied at %s:%d - use app.DeepCopy to create a copy before mutating", atomName, file, line)
56+
} else {
57+
log.Printf("WARNING: atom '%s' appears to be mutated instead of copied - use app.DeepCopy to create a copy before mutating", atomName)
58+
}
59+
}
60+
2461
// Atom[T] represents a typed atom implementation
2562
type Atom[T any] struct {
2663
name string
@@ -32,7 +69,9 @@ func (a Atom[T]) AtomName() string {
3269
return a.name
3370
}
3471

35-
// Get returns the current value of the atom
72+
// Get returns the current value of the atom. When called during component render,
73+
// it automatically registers the component as a dependency for this atom, ensuring
74+
// the component re-renders when the atom value changes.
3675
func (a Atom[T]) Get() T {
3776
vc := engine.GetGlobalContext()
3877
if vc != nil {
@@ -44,20 +83,34 @@ func (a Atom[T]) Get() T {
4483
return typedVal
4584
}
4685

47-
// Set updates the atom's value
86+
// Set updates the atom's value to the provided new value and triggers re-rendering
87+
// of any components that depend on this atom. This method cannot be called during
88+
// render cycles - use effects or event handlers instead.
4889
func (a Atom[T]) Set(newVal T) {
4990
vc := engine.GetGlobalContext()
5091
if vc != nil {
5192
logInvalidAtomSet(a.name)
5293
return
5394
}
95+
96+
// Check for potential mutation bugs with reference types
97+
currentVal := a.client.Root.GetAtomVal(a.name)
98+
currentTyped := util.GetTypedAtomValue[T](currentVal, a.name)
99+
if sameRef(currentTyped, newVal) {
100+
logMutationWarning(a.name)
101+
}
102+
54103
if err := a.client.Root.SetAtomVal(a.name, newVal); err != nil {
55104
log.Printf("Failed to set atom value for %s: %v", a.name, err)
56105
return
57106
}
58107
a.client.Root.AtomAddRenderWork(a.name)
59108
}
60109

110+
// SetFn updates the atom's value by applying the provided function to the current value.
111+
// The function receives a copy of the current atom value, which can be safely mutated
112+
// without affecting the original data. The return value from the function becomes the
113+
// new atom value. This method cannot be called during render cycles.
61114
func (a Atom[T]) SetFn(fn func(T) T) {
62115
vc := engine.GetGlobalContext()
63116
if vc != nil {

tsunami/demo/recharts/app.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,6 @@ var App = app.DefineComponent("App",
209209
return
210210
}
211211
chartDataAtom.SetFn(func(currentData []DataPoint) []DataPoint {
212-
currentData = app.DeepCopy(currentData)
213212
newData := append(currentData, generateNewDataPoint(currentData))
214213
if len(newData) > 20 {
215214
newData = newData[1:]

0 commit comments

Comments
 (0)