Routing, initial state not set

[thscott]

Hi there. I've been playing about with this example, and have noticed that the initial browser hash is cleared on page load. This seems to be because the call to initialise state https://github.com/adamhaile/surplus-todomvc/blob/9fcd69fc31fe447e2f92de72026bc7ad05fbf811/src/router.ts#L28-L29 happens after https://github.com/adamhaile/surplus-todomvc/blob/9fcd69fc31fe447e2f92de72026bc7ad05fbf811/src/router.ts#L7-L14 has already run. Moving the call to the top of the function fixes the issue.

I'm also curious as to why S.sample(setStateFromHash); is used rather than just setStateFromHash();. (I'm sure there's good reason that I'm failing to grasp here)

[adamhaile]

Yep, you're exactly right. Flushing our state to the hash before we've read the incoming state in is a mistake. I'll push up a fix. Thanks!

The S.sample() call is to avoid the router invalidating itself when ctrl.filter() changes. It's good practice to avoid unnecessary dependencies in constructors. If a constructor reads a signal, you're effectively saying that it depends on that signal, so if the signal changes, it should be thrown away and reconstructed.

In the current app, the router is constructed at top level, so there's no one listening to any signals that the router might read during construction. But if the app was refactored -- say we switched to a scenario where we only constructed the router after some bootstrap state was set up -- then we might be building it inside a computation. That would mean that without S.sample() that computation would pick up a dependency on ctrl.filter(): computation calls ToDosRouter(), ToDosRouter() calls setStateFromHash(), setStateFromHash() calls ctrl.filter(). So when ctrl.filter() changed, the computation would re-run, discarding and creating a new router in the process.

All that said, reading the code now I think there are better ways to handle this:

• if setStateFromHash() needs to read ctrl.filter(), it should probably sample it rather than read it directly, so that it can be called from any code, top-level or inside a computation, without creating unnecessary dependencies.

• the only reason setStateFromHash() is reading ctrl.filter() is to avoid triggering a change event if it's already consistent with the hash. There's now a standard library function, S.value(), for handling this exact scenario, so there's no need to be reading ctrl.filter() at all.

[thscott]

Thanks for the in-depth reply, that clears up a few things.

Regarding the router as it now stands, setting the hash causes setStateFromHash() to be called, which sets the ctrl.filter value. This change triggers the computation. This computation's only function is to set the hash, but this has already been set, so the function changes nothing. In fact with the way the app works now, the computation could be removed entirely.

In the case that you do want to be able to go the other way as well (change the filter value and have the hash change), is there a way to organise it so that a needless function isn't called?

In general is there a way for these sort of cyclical dependencies, S(() => hash(filter()); S(() => filter(hash()); where we're trying to keep values in sync, to not result in both computations running on any change?

[adamhaile]

You're right that, in the current app, the computation isn't necessary, since the app triggers state changes via <a href="#..."> tags instead of setting the filter() data signal.

But that might not always be the case. If we removed the computation that sets the hash, and future code changes filter() directly, then the two would go out of sync. App state and hash state would be different state, with a rule that sometimes synced them and sometimes not, depending on which side changed last.

Having code that syncs from both directions means that we can effectively think of the two as representing the same state. Change the hash, the signals change; change the signals, the hash changes. We don't (and never) need to worry about where the change came from. In the lingo of declarative programming, we're not thinking in terms of 'how' and 'when' but 'is': the hash is the signals, the signals are the hash.

The general strategy for keeping two things in sync like this is to have two functions, one that propagates A->B, the other B->A. The important bit is that both functions must be idempotent, including in regards to events (which is why S.value() is important here).

So yeah, change goes A->B->A, or B->A->B, with the last set being effectively a no-op. Since it doesn't trigger any change events, it takes virtually no time. It also serves as a check that the two sides are indeed symmetric, and we didn't go A->B->A'.

You could try to write a system that just goes A->B or B->A, but realize that you're introducing another piece of state, "where did the change come from?" And with it, an intervening piece to honor that state, so change goes A->(was change from A?)->B, B->(was change from B?)->A.

[thscott]

Thanks for that. It makes sense, was just wondering if avoiding the no-ops was possible, but yeah it makes the system simpler to not worry about it.

On a totally unrelated note (which I didn't think warranted an issue of it's own), the minimalist todo application from the Surplus readme (as typescript) has a type error in my editor for the opening div,

Type '{ children: (Element | SArray<Element>)[]; }' is not assignable to type 'HTMLAttributes<HTMLDivElement>'.
  Types of property 'children' are incompatible.
    Type '(Element | SArray<Element>)[]' is not assignable to type 'Children'.
      Type '(Element | SArray<Element>)[]' is not assignable to type '() => Child[]'.
        Type '(Element | SArray<Element>)[]' provides no match for the signature '(): Child[]'.

due to mixing Element and SArray types at the same level. It compiles and works just fine (as it should seeing as TypeScript isn't doing anything with the JSX), but would be interested if there's a workaround besides casting to any, or something I'm missing.