Proposal for go-app v10 - Your Suggestions and Deprecations Input

[maxence-charriere]

Hello go-app community,

I hope you're all doing well. As I start working on the next major version of go-app (v10), I want to involve you in the decision-making process by inviting your suggestions and thoughts on new features and potential changes.

What I'm Seeking

I truly value your insights. If you've been holding onto ideas or have suggestions to improve go-app, now is the perfect time to share them. Please take a moment to comment below with your thoughts on what you'd like to see in v10.

Proposed Changes

For this upcoming version, I have a couple of modifications in mind that I'd like to discuss with you:

1. Deprecation of app.If(v bool, v ...app.UI) Call:
   Currently, the app.If(v boo, v ...app.UIl) call generates unnecessary memory allocations. To address this, I'm considering replacing it with app.If(v bool, func() app.UI). My goal is to retain the familiar code semantics while boosting efficiency. I'm eager to know your perspective on this change and whether it aligns with your needs.

2. Transition of String Argument Functions to Format Version, Including app.Text and Other Text Methods:
   Another proposal is to transition functions that currently take a string argument to a format version. This would encompass methods like app.Text. The adjusted function signature would be something like func(format string, v ...any). Initially, I plan to use fmt.Sprintf, but my ultimate aim is to introduce a more optimized function for the same results. Your insights on this transition would be incredibly valuable.

3. Replacement of app.Route(path string, v app.Compo) Functions with Factory Function app.Route(path string, func app.UI), Removing Duplicates:
   The usage of app.Route(path string, v app.Compo) functions has raised confusion among many users. In order to streamline and enhance clarity, I'm considering a replacement that involves taking a factory function as an argument, something like app.Route(path string, func app.UI). This adjustment aims to simplify the process and improve usability while ensuring a consistent approach across routes. As part of this transition, duplicates such as app.RouteFunc(path string) will be removed to maintain a clean and efficient API. Your thoughts on this transition are greatly appreciated.

4. Uniformization and Simplification of app.Handler Syntax:
   The evolution of app.Handler has led to a growing complexity in both syntax and underlying mechanisms. To achieve API uniformity and simplify this aspect of go-app, I'm contemplating adopting a syntax similar to components. This change aims to enhance consistency while providing a more straightforward approach to using app.Handler.

5. Documentation Relocation:
   The influx of documentation-related requests has introduced noise to this repository. To address this and ensure a cleaner environment for issue discussions, I'm considering moving the documentation to a separate repository or adopting a GitHub Wiki-like approach. This change aims to improve the user experience and keep our discussions focused on enhancements and issues.

Your Voice Matters

Your feedback drives the direction of go-app. Please share your thoughts on these proposed changes, offer your opinions, and suggest any alternative ideas you might have.

Thank you for being an integral part of the go-app community. Your contributions and insights fuel our progress. Let's collaborate to make v10 the most remarkable version yet!

I'm eagerly looking forward to hearing from you.

Warm regards,
Maxence

[oderwat]

Hey Maxence,

here are my views:

1. It is obvious, that I do not like the current If(). I also think it is not just about allocations, but about code that may get executed "for nothing".
   BTW: "Deprecation of app.If(v bool) Call" should probably "Deprecation of app.If(v bool, elems ...UI) Call" in your text. I was initially irritated that there is a "preparation" if. Which could mean something like app.If(condition).Then(func()).Else(func()).EndIf()
2. I do not like having "Text()" using a format string because you need to guard it behind the varargs count or end up with format specifiers not printed correctly (escaping them would be a horror). I think I prefer having Text(string), Textf(format, args) without any magic. I may like having "Text(... string)" instead and had it concatenate the texts using a string builder or just strings.Join(). For fmt.Sprintf() I wonder if you can do better than the standard library and still support everything possible.
3. Yes, but I still think that some comments and more examples would make this clear enough for the current release too.
4. I would need to see some example code. I have no problem with the current implementation, though.
5. The reasoning is not clear to me. Just tagging issues would make clear what is just about documentation, and users will probably still mix all of it inside this repository. You then need to ask them to go somewhere else, and if you will introduce even more noise. I think "go-app.dev" is not an ideal way for documentation either (I understand that it is a showcase for Go-App, but then it should do some more stuff, like Search/Commenting and so on). I also have yet to experience a good working "GitHub wiki". I may be wrong, but usually the wiki is not really "maintained" and stuff ends up mixed all over the place. Likewise, I think a good way of documentation are additional README's inside the repository at package level or an example hierarchy, and of course, Go docs. This lets people hand you PR's for extra documentation and more examples. Like for example here.

I have another change that I would like: Ignore nil arguments in Body() app.Div().Body(c.header(),c.updateInfo(),c.content(),c.footer()) would then ignore if c.updateInfo() returned nil. Currently, I have to build a slice beforehand.

I actually do not even think that most of my wishes need another major release.

[maxence-charriere]

Hey @oderwat, thanks for the answer.
I ll take time to answer this with more details, just a note that every Body methods use FilterUIElems which already ignore nil values.

[oderwat]

FilterUIElems which already ignore nil values

I have to re-check. I thought I got errors with that.

[maxence-charriere]

@oderwat thank you for sharing your insights.

I do not like having "Text()" using a format string because you need to guard it behind the varargs count or end up with format specifiers not printed correctly (escaping them would be a horror). I think I prefer having Text(string), Textf(format, args) without any magic. I may like having "Text(... string)" instead and had it concatenate the texts using a string builder or just strings.Join(). For fmt.Sprintf() I wonder if you can do better than the standard library and still support everything possible.

I understand the concern. For context, in my codebase for Murlok.io, a significant portion of the string arguments are formatted with fmt.Sprintf. I've noticed that this tends to clutter the code and hampers readability.

There are hundreds of calls in the current setup that could benefit from format args. Introducing an F version for all these calls, such as Text(string) and Textf(string, ...any), would substantially increase the call stack. From a design perspective, it could introduce redundancy and potentially complicate function names. I believe standardizing to format args provides a more streamlined approach.

It's important to note that having a format string by default doesn't prevent the usage of the first argument as a non-format string. For instance, you can still perform concatenations like "str1" + "str2".

Additionally, the present implementation of Text(v any) employs various methods to format the argument, with fmt.Sprintf("%v") being the default fallback. This kind of forces a format onto the user, which isn't always desirable.

[oderwat]

@maxence-charriere I am fine without the "magic" and add my fmt.Sprintf() or a shorter stub version for that. Curious about me having no "bad memories" I did a quick check with the currently largest "little" project and found 4 Sprintf() in the roughly 5000 lines of code that is the PWA components. There is only one used with .Text() while the others are actually unrelated. But I also have only 56 calls to Text() overall.

it's important to note that having a format string by default doesn't prevent the usage of the first argument as a non-format string. For instance, you can still perform concatenations like "str1" + "str2".

But this is only possible if your check is there is more than one argument in the call, and all of my code will run slower.

I also do not know if it actually changes the code the compiler produces with functions that accept variable arguments. I read some (old?) information that it has to be allocated to the heap, and even if not, it should take up space and make everything slower for those that do not want or need to use the format string. I did some quick and lazy experiments, and it seems at least to allocate the only/first variable in different memory regions. But even with //go:noinline pragma it was not a big difference in speed. But I would actually suggest that you do some kind of benchmark to see what happens. Especially, having a lot more GC activity would be awful for the frontend in the browser. It should be possible to check the WASM disassembly for the code. If one cares.

[mlctrez]

Hey Maxence,

My thoughts:

1. Same as @oderwat on this one.
2. I would lean towards keeping the default .Text(v any) method and only add .Textf(format string, a ...any) as in the v9.8.0 release.
   2a. You mention other Text methods - would this be more than what's in the v9.8.0 release?
3. Same as @oderwat on this one.
4. Same as @oderwat on this one.
   4a. I do see a point that providing app.Handler defaults is problematic with the current code base. I would also be interested in seeing what a new implementation looks like.
5. I can see your point with the recent PRs being opened in the main repo a desire to move that documentation elsewhere. I also agree with @oderwat that user's are going to open PRs on the main repo even if there is another one that is more appropriate for how do I do this questions.
   5a. I would lean toward refreshing go-app-demo to the current release and provide enough examples there. I believe this would cut down quite a bit on the does not work as expected PRs. Also, configuring some issue templates could help in getting valid bug/feature enhancements for the main repo. With 90+ issues open on the main repo, it might be a good idea to setup automatic closing on those that are just hanging out there with no activity.

As @oderwat mentioned, I don't see any of these changes resulting in a new major version ( with the exception of 4 if the api changes quite a bit ).

Matt

[maxence-charriere]

The proposed changes to If will break compatibility on their own. I'll provide an update later regarding the adjustments to app.Handler.

Regarding the documentation, I'd like to clarify a few points:

• The current documentation consists of the Go-generated references alongside the markdown files.
• It's important to me that we retain these markdown files within the go-app repository.

Additionally, I've been contemplating a project for some time that essentially replicates what the current documentation website does but makes it accessible for wider community use. Consequently, any issues related to documentation in this repository will be primarily centered on content.

[rtrzebinski]

1. I'd love the option to inject dependencies when defining app.Routes, for instance:

app.Route(components.PathHome, &components.Home{})

Could have a dependency injected like this:

app.Route(components.PathHome, &components.Home{service})

Currently it always lands as an empty struct.

2. I agree documentation could be improved, I personally see it even more important for the project than new features - feature wise it feels like it does a lot already, but it's often hard to find out how to do some basic things.

Specifically I'd like to see:

• a way to submit documentation changes as a PR
• a useful search feature that is now lacking

Unsure about moving it to a separated repo though, perhaps better to use /docs folder and build something searchable with a Github action?

[maxence-charriere]

Hello everyone,

I've dedicated significant effort towards refining v10 recently, particularly focusing on the refactoring of the engine code. Here are the key updates:

• Deprecation of OnInit within the Initializer interface: The primary function of OnInit was to facilitate initialization prior to the first-time rendering of a component. Given that OnMount now also triggers right before the first rendering of the component, the Initializer interface has become obsolete.

• Deprecation of PreRenderer interface: After working with it for some time, I've found it to be an overly complex segment of the codebase. This has now been removed. For specific tuning regarding where code is executed, simply use if app.IsServer or if app.IsClient.

• Modification to Context.ObserveState: It now requires the receiver as its second argument, leading to the removal of the observer's Value method.

• Updates to Context.SetState: It no longer accommodates options. Instead, it now returns a State object that includes methods to persist, set a timeout for, and broadcast the state. This modification was intended to enhance the discoverability of options. For instance: ctx.SetState("test", 42).Persist().

Your feedback would be invaluable. Feel free to check out and try these changes on https://github.com/maxence-charriere/go-app/tree/node-manager

[maxence-charriere]

@rtrzebinski #875 (comment)
This is feasible since Handle now takes a function that creates a component.

[oderwat]

@maxence-charriere LGTM. We are in the middle of a large project, so I do not have too much time at hand. But I will try to get the branch running on the new version. Do you have some ETA for when v10 could be released?

[maxence-charriere]

I estimate it will take another month or two. Currently, Murlok.io is utilizing it, but my priority is to ensure that it introduces a minimal number of breaking changes for users like you. I believe the work on the engine is nearing completion. Additionally, I'm looking into reworking the HTTP handler and will also be reviewing and potentially integrating some of the pending PRs.

[mlctrez]

@maxence-charriere I'm testing this out now on a fairly large project and a go-app library used by that project.

Migration to the new observe state code is trivial so far and improves readability. 👍

I'm stuck on how to trigger component updates since this method was removed:

go-app/pkg/app/component.go

Lines 197 to 202 in 4af7476

	// Update triggers a component appearance update. It should be called when a
	// field used to render the component has been modified. Updates are always
	// performed on the UI goroutine.
	func (c *Compo) Update() {
	c.dispatch(func(Context) {})
	}

I see the new update manager in the V10 code and how it is used internally.

go-app/pkg/app/update.go

Lines 3 to 8 in 19e4353

	// updateManager manages scheduled updates for UI components. It ensures that
	// components are updated in an order respecting their depth in the UI
	// hierarchy.
	type updateManager struct {
	pending []map[Composer]struct{}
	}

What would be the correct way to trigger an update in V10?

Thanks!

[oderwat]

@maxence-charriere We also do a lot of out of event handler component updates (like consumers that receive messages from NATS). I also still need the preventUpdate() because we do debounce a lot of events. I did not check your new code at all (busy with writing python code PTs for memGPT lol).

[mlctrez]

@oderwat

PreventUpdate is still there, just changed a bit to use the new update manager

go-app/pkg/app/context.go

Lines 224 to 228 in b060dac

	// PreventUpdate halts updates for the enclosing component, respecting any
	// implemented UpdateNotifier behavior.
	func (ctx Context) PreventUpdate() {
	ctx.foreachUpdatableComponent(ctx.sourceElement, ctx.removeComponentUpdate)
	}

[mlctrez]

@maxence-charriere The lastURLVisited lines in addHistory and replaceHistory in pkg/app/js_wasm.go might have been missed during refactoring. It looks like the engine code handles tracking the last url now.

go-app/pkg/app/js_wasm.go

Lines 274 to 286 in 3d07156

	func (w *browserWindow) addHistory(u *url.URL) {
	u.Scheme = w.URL().Scheme
	u.Host = w.URL().Host
	w.Get("history").Call("pushState", nil, "", u.String())
	lastURLVisited = u
	}
	func (w *browserWindow) replaceHistory(u *url.URL) {
	u.Scheme = w.URL().Scheme
	u.Host = w.URL().Host
	w.Get("history").Call("replaceState", nil, "", u.String())
	lastURLVisited = u
	}

[mlctrez]

Before refactoring, I missed that OnMount is now called on server side renders.

Given that OnMount now also triggers right before the first rendering of the component, the Initializer interface has become obsolete.

For specific tuning regarding where code is executed, simply use if app.IsServer or if app.IsClient.

So to upgrade from V9 -> V10, existing code will need to implement the guard you mentioned in any OnMount:

func (a *Audio) OnMount(ctx app.Context) {
	if app.IsServer {
		return
	}
	// existing V9 component mounted code
}

I refactored my app and go-app library to add the guard conditions in OnMount. When rendering server side html, the client only OnMount code is skipped.

However, the behavior of JSValue() appears to be broken in V10.
When running on the client in V9, JSValue() returned the JavaScript value of the dom element.
With V10 this appears to only return nil.

I tracked it down to the Compo.rootElement being nil when OnMount is invoked. Line 156 is always true.

go-app/pkg/app/component.go

Lines 155 to 160 in d8cf3a5

	func (c *Compo) JSValue() Value {
	if c.rootElement == nil {
	return ValueOf(nil)
	}
	return c.rootElement.JSValue()
	}

OnMount is called on line 232:

go-app/pkg/app/node.go

Lines 231 to 233 in e89c91b

	if mounter, ok := v.(Mounter); ok {
	mounter.OnMount(ctx)
	}

But Compo.rootElement is not set until line 249:

go-app/pkg/app/node.go

Lines 248 to 250 in e89c91b

	root = root.setParent(v)
	v = v.setRoot(root)

This might be as simple as moving the setRoot up a dozen or so lines, but I don't know the implications of this.

Note

Edit: for what it is worth, moving node.go lines 231-233 after v.setRoot fixed the issue with JSValue() returning nil, but that defeats OnMount replacing PreRender and OnInit as it is called too late in the process.

[maxence-charriere]

#875 (comment)

The primary purpose of this is to determine whether it's a fragment navigation and to avoid unnecessary page re-renders. This behavior has been in place since before, as seen here => https://github.com/maxence-charriere/go-app/blob/master/pkg/app/app.go#L311.

#875 (comment)

Thank you for pointing this out. The current design results in OnMount being called before the js value is set. As a result, one would need to either defer or dispatch to access the js value. I'm debating whether it's better to keep it this way (and document the behavior) or revert to the previous OnInit setup. What do you think?

[oderwat]

revert to the previous OnInit setup

I think that is better. We need "OnInit" to run once while mount/dismount components multiple times. I also do not want to have every OnMount needing to check it runs on the server.

For us, it was all fine as it was before. Using preRender for what the bots see, OnInit to set stuff up before Render is called for the first time. OnMount to attach stuff to the node.

Why are you changing these things? What is the advantage over how it worked before? To me, it looks a bit like changing stuff, for the purpose of changing stuff, and then we have to refactor everything until it works as it worked before the changes.

Please consider that there are some users that write quite large PWA's that primarily focus on getting installed. There is no SEO or "link to some route" needed at all. In fact the SEO shows totally different information and all routes are going back to one handler which handles all state of the application.

P.S.: I did not try the new changes myself, but from reading what @mlctrez wrote, the whole changes stop LGTM.

[maxence-charriere]

Regarding pre-rendering, I noticed that my codebase started to accumulate duplicate calls. It became apparent that both the server and client often require the same calls, and using IsServer or IsClient makes it clearer where specific actions take place, enhancing code readability.

As for OnInit, I'm contemplating reverting to its original behavior over the new OnMount. The goal isn't change for change's sake, but rather to simplify the API and minimize the number of operations involved. The OnInit approach seems to offer a clearer and more streamlined interface.

[mlctrez]

#875 (comment)

The primary purpose of this is to determine whether it's a fragment navigation and to avoid unnecessary page re-renders.

Sorry, I was not clear on what I was trying to convey here.

The wasm build fails to compile because lastURLVisited is undefined.

In the V10 branch, this variable block was removed ( code below is from V9 )

go-app/pkg/app/app.go

Lines 40 to 46 in 4af7476

	var (
	rootPrefix string
	isInternalURL func(string) bool
	appUpdateAvailable bool
	lastURLVisited *url.URL
	resizeTimer *time.Timer
	)

[oderwat]

We do not even have the same code for front-end routes and back end routes. So I guess it would never call our front-end OnMount() on the server. I guess we are looking at two complete different use cases.

As OnInit() originally was a PR by me, I wanted it for a reason and that is still the case. As usual, I think that the documentation needs to be more clear about what is actually happening. I tell my coders that it is always: OnInit (called once) -> Render() -> OnMount() (called again after dismount) -> Render() and that Render has no context and should not change the model. With that we usually get this out of the way very quickly.

[maxence-charriere]

I put back OnPreRender and OnInit. @mlctrez can you check if everything works?

[mlctrez]

I reverted the conditional if app.IsServer checks in OnMount and everything works in my app and library. The Compo.JsValue() in OnMount returns a non nil which matches the V9 behavior.

There is some delay being introduced in an OnClick on one of the UI elements occasionally. I need to see if that is something that was a V9 -> V10 change or something in my app or environment. I'm building out a test application to see if I can re-create this delay. It appears that it might be related to the new updateManger and the UI thread. Strangely, adding logging statements in prevents the delay from occurring. So far I have not been able to reproduce it in the test application.

My app and library ( and other go-app source that I've written ) does not make much use of OnPreRender and OnInit so I'm afraid I don't have much to test with there. I added these to the test application and from what I can tell they are working as expected and behave the same as the V9 code. There are two calls to Render() , one before and one after Mount() but this also occurs in V9.

@maxence-charriere This comment was edited with some updates from today.

Another update:

I was able to reproduce the issue in the test application after bringing in some additional libraries that the other application used. At first the issue could not be reproduced until I matched the nats.io library version. nats.io v1.20.x does not appear to cause the issue but nats.io v1.30 does.

Strangely enough as mentioned before, inserting a single fmt.Println after invoking the nats.io call resolved the issue. Starting a new goroutine that does nothing after invoking the nats.io call also resolves the issue. So any attempts to gather information about what goroutine might be blocking another goroutine result in the issue going away. 😢

I don't think that V10 needs to do anything different, but I do think that the upgrade docs for V9 -> V10 should mention this change in behavior. My solution was to wrap the nats call and the resulting context.NewAction call in a context.Defer

[maxence-charriere]

I've implemented several updates on the v10 branch.

Here's a summary of the latest changes:

• Removed the testing dispatchers. The NewTestEngine() function has been introduced as a replacement for the previous testing dispatchers.

@mlctrez, I've made some adjustments regarding the delay issue you mentioned. Could you please give it another try and let me know how it goes?

[mlctrez]

@maxence-charriere I tested again against the new V10 branch and the delay is still present.

I'll look into the nats.io source code some more and see if I can track down how it is uses goroutines, selects, channels, etc. What I suspect is happening is that there is contention for the JS UI thread between go-app and the nats.io code.

The nats.io code is communicating using a web socket and the delay that I'm seeing when clicking on a component ends exactly when pings are sent across the web socket.

[oderwat]

@mlctrez we start all NATS consumers in their own go routines, and we also use debouncer code (which also makes them async) for most event handlers.

@maxence-charriere I sadly still can't test the new go-app code as we are short before releasing our biggest go-app application into the beta-tester hell. So far it all worked out quite good. I guess we can check what happens to our code at the end of November.

[mlctrez]

@maxence-charriere I finally found what was different between V9 and V10 that caused my UI delay.

In V10, the frames ticker is reset to 1 Hour when the ticker fires.

go-app/pkg/app/engine.go

Lines 250 to 255 in 8e43917

	case <-frames.C:
	e.processFrame()
	frames.Reset(iddleFrameDuration)
	currentFrameDuration = iddleFrameDuration

In V9, the frames timer is doubled each time the dispatch length is 0 and the ticker fires:

go-app/pkg/app/engine.go

Lines 341 to 350 in 4af7476

	case <-frames.C:
	e.handleFrame()
	if len(e.dispatches) == 0 {
	if currentFrameDuration < time.Hour {
	currentFrameDuration *= 2
	}
	frames.Reset(currentFrameDuration)
	}

I think that my code was somehow dependent on the frames code being executed more frequently.

Again, I don't think this is something that should be changed for V10, since it is probably a rare case and adding a ctx.Async() in the right location solves the delay issue. My code makes use of actions, state, and websockets, so there still may be something there and if it pops up again I'll let you know.

[oderwat]

Some things I would like to see in go-app:

1. We implemented "online/offline" handler callbacks and wondered if this would not be a nice addition for go-app itself.
2. I would love to be able to trigger an update check for the serviceworker through the app. This way one could save one reload when trying to force an update. Currently the serviceworker needs to get installed and then a reload is needed. But to force the app to check for an update you need to reload it beforehand. So you reload into the same app, the service worker picks up the update and then reloads into the new app. If one could send a message to the app that there is a new version in another channel (trivial for us having a websocket open) on could tell the webworker to search for and update and it would load that and trigger the normal updater. Hence you only get one reload. This way one can leave the automatic check disabled or have it very slow.
3. Having "OnUpdate" work as I thought it would work. I once tried to actually get to know if the dom is getting updated and thought an "OnUpdate" Handler would be called after a components DOM is getting updated (or before?). But I did not get any calls to that Handler?
4. An official way to queue something as Dispatch() without having a context. Currently we store the context of in "OnMount()" to the component and use that one to call Dispatch() from random Go routines. It seems to work but I have a bad feeling about that for some time. Is this actually supported and if not, what are alternatives. I try to avoid any "state handler" and prefer callbacks/handlers.
5. Making the state handlers typesave (using generics / ascii85 + gob encoder instead of json?)
6. Some kind of panic recovery. Currently a panic seems to stop everything including the service worker (right?). It seems that because of that there will never be an check for an update until you can initiate a reload of the app, which is not easy to do when the app is installed and the user is "joe random", We currently add "recover()" into all major async routines like the nats consumers or time loops. Maybe there could be a "recover()" in "engine.start()" that can for example restart the app(?) and set a flag in "app.Paniked" (or whatever) to indicate that the app crashed and was recovered?
7. A official way to extend the service worker (I think we had this somewhere before).

[mlctrez]

@maxence-charriere

After replacing goappSetupAutoUpdate with goappTryUpdate , this line needs to be removed :

go-app/pkg/app/gen/app.js

Line 33 in cd97cdc

goappSetupAutoUpdate(registration);

[maxence-charriere]

@mlctrez fixed

[maxence-charriere]

I think we need a way to add the "purpose: maskable" to the icons objects in the webmanifes

@oderwat , replaced the apple touch icon with the maskable one.
=> #938

[maxence-charriere]

your even handlers are missing a way to set the passive option

@oderwat I made a PR for this: #942

Usage:

// Event with scope
app.Div().OnScroll(func(cox app.Context, e app.Event){}, app.EventScope("1"))

// Passive event
app.Div().OnScroll(func(cox app.Context, e app.Event){}, app.PassiveEvent())

// Multiple options
app.Div().OnScroll(func(cox app.Context, e app.Event){},
    app.PassiveEvent(),
    app.EventScope("1"),
)

[oderwat]

@maxence-charriere this will need a lot of changes in our code and I do not like the idea to base multiple packages on a PR commit hash. I think it is a good way into the future to declare the event options using a concrete type. I would like that this is getting merged before we rewrite so much packages, just to rewrite them again, when it gets merged.

[oderwat]

Besides that I tested #942 in a smaller project, and it does what we need and is also fairly easy to updated and understand. I like that solution.

[oderwat]

BTW. I verified that "push notifications" (really "webpush") works with iOS 17.3.1 and the new (EU) 17.4 on the iPhone. I will test iPad later. It would be really cool if we could get the distinctions between "WebPush" and "Notifications" and implement a standard way for recognizing pushed notifications as I described above. See also #303 which was probably not seen by many people. @krustowski and I seem to have had the same journey to get it work.

[oderwat]

@maxence-charriere I wonder why TryUpdateApp() is on context. This makes using it problematic, because if you want to have the old timer based variant, it needs to be added inside a component. This seems unnecessary as it just uses app.Window() and can run anytime. I would suggest changing it to app.TryUpdate().

[maxence-charriere]

app.TryUpdate()

@oderwat Good idea, this has been modied

[maxence-charriere]

Do you folks use state broadcast?
Look like it prevent page caching and affect lighthouse performance test.

Planning to either deprecate it or disable it by default.
What do you think?

[oderwat]

Do you folks use state broadcast?

Not with our current apps. But we will most likely need inter tab communication (with local storage events or with event broadcast from the service worker) in the future. Our apps do not support forward/backward navigation anyway.

I think it would be okay, to have it implemented but disabled by default.

[oderwat]

@maxence-charriere another "thing"

We use "dark node" for some apps. So the default loading looks "wrong". I solve this by adding a raw header:

		RawHeaders: []string{`<style>
    .goapp-app-info {
        background-color: #000 !important;
        color: white !important;
    }
</style>`},

I think this should be configurable in app.Handler. Is that feasible?

[maxence-charriere]

you should be able to solve that with any css.

[oderwat]

you should be able to solve that with any css.

Not just "any". I believe it has to be embedded in the page and use "!important", so it overrides your hard coded media query app.css styles. I just wonder if app.css really needs to be hard coded and added instead of being created when the server starts, so that the content can be modified in an easier way. For example: Not every app wants to support different color schemes and your standard code uses the scheme with the media query while loading. I am not even sure, if we can override a media query based CSS with the !important, if our light scheme would have another background. This may end in the requirement to write a http-handler to catch "app.css" requests to replace them with something else (we did something like this with the manifest too in the past).

Something similar about the rotating Logo. Sure, one can do this, but I already had clients telling me that this looks unprofessional and lame. It would be nice if stuff like this could be changed more easily.

P.S.: It does work like I posted in the comment before. I just do not find it very "nice" to override "my" own CSS (considering that any external person would wonder why I write CSS in "my" code just to override it all the time)

[maxence-charriere]

this is how I made it on Murlok.io:

.goapp-app-info {
  color: var(--text-color);
  background: var(--default-background);
}

Does not seems to be !important. Maybe the difference is that I included that in a css file.
If you put that in raw headers, app.css is loaded after so it get overridden.

[oderwat]

@maxence-charriere Yes. It works as you say. I think there is this stupid bug(?) with Safari, that does not update the CSS used for the display when the app gets an update that changes CSS. It loads the new app but uses the former CSS. You need to hard kill the application and start it again. This is partly reproducible in Safari in OSX. I think that this was the problem we had, why it was not working the "easy" way when we tried this months ago.

[oderwat]

@maxence-charriere I think that we also need app.EventNotPassive() setting the option to 'false', to suppress warnings with such handlers that actually want to use prevent Default (we use both). So this either needs to be added or the former one needs to get a bool argument.

[maxence-charriere]

I don’t understand the need since it is non passive by default. Since you should not call prevent default with the passive option, a same handler should not be used for passive and non passive.

[oderwat]

I don’t understand the need since it is non passive by default

Do you specify { passive: false } as default? If not, this seems to be what Chrome checks for to not show the debug message if you use prevent Default in such a handler.

Very last comment here: https://stackoverflow.com/questions/44060131/zone-js-violation-warnings-on-console-in-angular-project-only-on-chrome/44339214#44339214

[maxence-charriere]

@oderwat from what I read,
Prevent default should not be called when an event handler is set to passive.

when there is no passive option, no passive field is set so the default value is whatever the browser set (which should be false).

Chrome seems to be unhappy if you set a non passive handler with wheel, or if you call prevent default in a passive handler.

I think that issue should be handled in your code by making sure to not use prevent default in a passive handler.

[oderwat]

@maxence-charriere I think that there was something changed in Chrome which makes this obsolete. It all works for us without the problem that was mentioned in that StackOverflow article. I forgot to update / remove that request.

[oderwat]

@maxence-charriere I am not sure what is going on, but we now have a lot of times (while developing) that we get the message "installing app worker" but not the "app worker ... is activated" following. The app does not call the OnAppUpdate() so we need to reload by hand. It then shows the "app worker ... is activated" and it still does not call the "OnAppUpdate()" so we need to do another manual reload. After this, the new version is active. Do you have an idea what may go wrong?

[maxence-charriere]

@oderwat, I noticed that on Murlok.io as well. I haven't changed anything, but I observed it occurs when the ads fail to load some elements.

Do you see any errors or warnings in the browser console?

[maxence-charriere]

@oderwat, I've just implemented a fix that addresses the issue of the service worker being activated before the WASM is loaded. Please let me know if this resolves the problem.

[oderwat]

@maxence-charriere I had some time to check the fix with different projects and "updater" setups, and it seems to solve the problems we saw before. Thank you!

[maxence-charriere]

I'm planning to move forward and release version 10. This release is long overdue! 😀

A quick note on push notifications: I attempted to migrate them to the service worker, but unfortunately, I wasn’t successful.

If there's anything specific you think needs to be addressed before the v10 release, please let me know.

[maxence-charriere]

https://github.com/maxence-charriere/go-app/releases/tag/v10.0.0

[gepengscu]

why private app.Value addEventListener method ?

[maxence-charriere]

@gepengscu A javascript value is not necessary a DOM element.