Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Todo MVC example has misleading usage of p.Items #306

Open
Bradbev opened this issue Apr 22, 2023 · 2 comments
Open

Todo MVC example has misleading usage of p.Items #306

Bradbev opened this issue Apr 22, 2023 · 2 comments

Comments

@Bradbev
Copy link

Bradbev commented Apr 22, 2023

Hi folks. I'm learning Vecty by following the Todo example. In the example.go file there is this block of code

store.Listeners.Add(p, func() {
	p.Items = store.Items
	vecty.Rerender(p)
})

However, p.Items is never used the the app - PageView directly accesses the store. I'd personally prefer if the example worked as expected, but I've been unable to find a clean way to trigger the update.
If the store is loaded & the refresh triggered prior to vecty.RenderBody then a panic is thrown.

I've found two ways to work around this issue without needing my list display component to directly access the store.

  1. Upon first store load, delay dispatching the update by sleeping inside a goroutine for a short time. I think because of the batch singleton this should be reliable - Sleep(0) works for me for example. But feels like maybe one day it'll just stop working because of underlying threading models.
  2. Add an isFirstRender bool to the example.go block above and move the store load to just before RenderBody. This works, but generally feels like it won't scale well with many components.

I'm going to use #1 for now because it scales better and should be pretty reliable.

Side question - how active is Vecty? It seems quiet - which is great if most things generally work :)

Thanks!
Brad

@Bradbev
Copy link
Author

Bradbev commented Apr 22, 2023

Ah, perhaps the cleanest way is to just not use RenderBody in this case.

	//	vecty.RenderBody(p)
	err := vecty.RenderInto("body", p)
	if err != nil {
		panic(err)
	}
	store.LoadLocalStorage()
	select {} // run Go forever

This way needs no delaying etc. I wonder if this usecase is common enough that RenderBody could grow a "renderdone" callback before it blocks forever?

@soypat
Copy link
Collaborator

soypat commented Apr 23, 2023

Careful- Sleep(0) is documented as a call that should return immediately. Use time.Sleep(1) or runtime.Gosched() to guarantee a sleep of sorts.

As far as the correctness of the todomvc example goes, I remember reading it a while ago and thinking it could be done simpler. I think they're due for a rewrite.

As for the quietness- yes, it's been pretty quiet around here. Vecty itself works and as far as I can tell most if not all bugs I've run into have been from my own doing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants