bug: Stale event handlers with Range

[mastercactapus]

It seems that when using app.Range event handlers remain stale, even when the underlying data updates.

Example app demonstrating problem

package main

import (
	"log"
	"net/http"

	"github.com/maxence-charriere/go-app/v10/pkg/app"
)

type hello struct {
	app.Compo

	names []string
}

func NewHello() *hello {
	return &hello{
		names: []string{"foo", "bar", "baz", "qux"},
	}
}

func (h *hello) Render() app.UI {
	return app.Div().Body(
		app.Range(h.names).Slice(func(i int) app.UI {
			name := h.names[i]
			return app.Button().Text(name).OnClick(func(ctx app.Context, e app.Event) {
				app.Log("clicked", name)
			})
		}),
		app.Hr(),
		app.Button().Text("Shift").OnClick(func(ctx app.Context, e app.Event) {
			h.names = append(h.names[1:], h.names[0])
			ctx.Update()
		}),
	)
}

func main() {
	// Components routing:
	app.Route("/", func() app.Composer { return NewHello() })
	app.RunWhenOnBrowser()

	// HTTP routing:
	http.Handle("/", &app.Handler{
		Domain:      "bugreport",
		Name:        "Range Event Bug",
		Description: "An Hello World! example",
	})

	if err := http.ListenAndServe(":8080", nil); err != nil {
		log.Fatal(err)
	}
}

Steps to reproduce:

1. Compile and run the example application
2. Click foo and see clicked foo in the console
3. Click Shift and observe that the button text shifts
4. Click the bar button and see it still says clicked foo in the console

The button below will "shift" the slice and the buttons update as expected. However, it appears the original click handlers are used, rather than the new ones.

I expected that the click handlers would update along with everything else.

A workaround is to embed data in the DOM and pull it back out in the event handler, but is tricky with structs or other complex types that can't be serialized easily.

[oderwat]

Change your code and add an event scope:

...
return app.Button().Text(name).OnClick(func(ctx app.Context, e app.Event) {
	app.Log("clicked", name)
}, app.EventScope(i))
...

What actually happens is something very different from what you think that happens. Basically you only have one event Handler there. When you do not only add stuff you can't use simply i anymore. Then you can for example add something like 'ulids', labels. Here the name would work or use the memory address of the element fmt.Sprintf("%p", name). Anything that makes this event handler unique to all the others.

You can also remove ctx.Update(). That is only needed if you are making changes outside your event handlers.

[mastercactapus]

Gotcha. When it didn't behave as I expected, I started adding log statements, and what threw me off was that the function got called with the new values, but I expected it to use the latest closure rather than the first.

I looked for something like Key() (from experience with React) as I thought I needed to set it on the button itself to make that re-create, rather than looking into the options of OnClick itself.

I'm just starting to try out this library, so I expected to overlook things. That said, I learned about Range here: https://go-app.dev/declarative-syntax#range

It might be a good place to add a Note on event handlers and/or a note in the handlers like OnClick since those were the first places I thought to look.

[oderwat]

The options are "my fault". I requested some additional functionality and this hides the more obvious , scope) parameter that was there before.

We developed multiple rather large products with Go-App in the last three years, and it is mostly working very well.

Here are some of the most important things from my perspective:

• The DOM node and event handlers are "reused". This can have some weird effects, and you need to use something like our mountpoint, to actually get the update you expect.
• The "scope" given is resolved using %v. This often magically works. But it may dump a lot of data (&something). This is why I gave the example with the fmt.Sprintf("%p",&something).
• Exported fields of the component, can't be changed from inside the component scope. This is by design as they pass updates from the parent while rendering the new state.
• It may be a bit opaque, but when you program with the JS interoperability (app.Window()), it can be very helpful to use console.log() instead of app.Log(). This is because you will get the inspection features of the browser console. It is also nice to get some caller tracing. Check the dbg package from the above link. We have a lot of dbg.Log() in many places, this is enough to show what happens, because it shows who the callers are.
• You need to implement a timer based update handler if you want auto-updates checks. This is different than in the past.

[mastercactapus]

The DOM node and event handlers are "reused".

I think this is the key thing to point out (maybe added to the Range doc) that in my case would have saved me some time. If that makes sense to everyone else I'm happy to add a note in there.

Exported fields of the component, can't be changed from inside the component scope.
This feels a bit "magical" -- when you say "can't" is that enforced by throwing an exception, or a "it won't update the way you expect if you do it" sort of thing?

Ty for the tips; I did run into having trouble getting updates to happen when I expected them, but I don't remember if that was a public/private thing or what. In the end I switched to triggering an event/action to "refresh" specific things.

[oderwat]

About the exported (Uppercase) fields: It "overwrites" the updates silently. I think this is actually the most common problem for new users. Some people do not even "export" the fields for the component reuse, but just because they "think" that they need to be exported. Kind of like in reflections. And this is exactly the case. So the better way is to have exported fields twice and copy the value from the "external" to the "internal" state in "OnInit()" or "OnMount()".

P.S.: I do not like the "event" system. To me this is just "what you are used to when using java-script". They are also not type save. We actually use simple callback functions and even global state (muhahahaaa 666). Usually sharing a package globals. There is also a "hidden" guarantee that your event code is not running concurrently, because the browser has only one "GUI" thread (until this would change some day).