Refactor routing / routed state management

[lukavdplas]

This issue is intended to track something we discussed in our last meeting and that I'm currently working on.

Deep routing for queries is a bit of a mess. The query parameters of the route are supposed to maintain two-way synchronisation with the state of the UI. There are currently two ways in which this is implemented (depending on the component):

• The ParamDirective is a superclass for components. The idea is that components handle state updates by sending updates to the Router, and then listen to changes in the route. This is a clean way to achieve two-way binding, but tying everything to components makes it difficult to model interdepencies between different parts of the query / results. It also makes it difficult to manipulate the data without using that routed component, which complicates unit testing.
• For the reasons described above, the QueryModel, as a more complex model, is not really built on the ParamDirective class, but the implementation is not very clean. The idea is that the model handle's its state internally, and the component that creates it (the SearchComponent) makes sure the route is kept up to date. If the route is changed in a way that doesn't match the current state of the model, the model is re-instantiated from the route. This carries a risk that if the conversion between the route and the model is incorrect, the application can get caught in a loop.

So the main issues are (in no particular order):

1. inconsistent implementation
2. difficulty in managing connected parameters
3. difficulty in unit testing
4. two-way binding that can cause cyclical updates

I've been working on a solution that should help with issues 2-4. If it is applied across the application, point 1 would be addressed as well.

[lukavdplas]

Current status of this issue:

After https://github.com/UUDigitalHumanitieslab/I-analyzer/pull/1467 is merged, the core of the application is based on the Store / StoreSync classes to handle routing.

The refactor will be complete if we also convert:

• the word models query
• visualisation modules

For visualisations, this can be done by basing visualisations on the Results class but may also be done by rewriting the component so it inherits StoreSync. The former option is more "proper" but also takes longer.

[jgonggrijp]

Perhaps this is a superfluous comment, but you could consider the route a special type of view/controller. As such, ideally it would communicate changes by editing a model and only update itself in response to changes in a model. This would eliminate any direct ties between routes and components and avoid cyclical updates. Maybe this what you were planning to do already, or similar to it.

[lukavdplas]

Yes, I went with that approach in an earlier version, but the issue was that routes can be updated independently (through browser navigation) and it's hard to detect whether changes in the route were triggered by the model or not (especially in a fool-proof way). You end up listening to any router events and responding to those in the model, but also triggering router events when the model changes, so there is a real risk of cyclical updates.

I find this tricky in MVC - it doesn't really seem to avoid cyclical data flow when the view and controller are the same thing - which is actually true for a lot of modern UI language, but, in this case, also for routes.

The current approach instead conceptualises routes as a "backend" to the model. The same model class can be instantiated with different backends so this maintains flexibility, but you get a clean unidirectional dataflow, where changes from controllers result in the model sending an update message to the backend, and the model presents an observable based on the backend state to view components.

[jgonggrijp]

Please clarify, how does that eliminate the cyclical updates? Whether you call the route a backend or a view, isn't it both a source and a target of events in both cases?

MVC generally avoids these cycles by only issuing events when there are actual changes. For example, consider that we start with a change in the model. The route updates accordingly. Assuming for a moment that the routing mechanism has no way to prevent an event from being triggered, this will cause a response in the model again. However, since this does not change the actual value of the model, the model does not retrigger a change event. The chain stops.

Similarly if the route changes first, for example because the user pressed the back button: the model changes accordingly. It will obviously trigger an event because any views observing the model need to update. The router will detect this event as well and respond. However, since this does not change the route, the router will not emit a second event and the chain stops.

(I know that in Backbone, routers can distinguish between external and internal updates, and they only trigger events on external updates unless you explicitly ask them to also do it on an internal update. I don't know Angular's routing mechanism very well, but even if it does not have a similar provision, the above general mechanism should still stop cyclical updates.)

[lukavdplas]

Re. cyclical updates, the earlier implementation that conceptualised routes as views/controllers ended up with a flow like this:

1. a controller signals an update to the model
2. the model updates its internal state
3. the router (as a view) receives the updated state from the model and starts navigation
4. the router finishes navigation and (as a controller) signals the new path to the model
5. if the route doesn't look like the model expects it to, go back to step 2

I think that mostly matches your description. Step 5 was where you would compare the router state to the model state and only respond to real changes. But if a model's methods to convert between its internal state and the route are implemented incorrectly, you can get caught in a loop where steps 2-5 keep repeating.

The current implementation functions roughly like this:

1. a controller signals an update to the model
2. the model signals an update to the router, and the router starts navigation
3. the router finishes navigation and signals the new path to the model
4. the model's observable state is defined as a reflection of the router, so it will now show the new value to observers (e.g. views)

Mistakes in the implementation of models can obviously still result in bugs, but there is no possibility of a feedback loop.

I don't think this approach generalises as well to other applications but it works for how I-analyzer uses deep routing.

[jgonggrijp]

Both of the flows that you describe are fairly MVC-ish, except that in both cases, you seem to frame it as if the model is concerned with the state of the route. Maybe that is just my interpretation. In any case, this is how I would approach it:

1. A controller tells the model what state it should have.
2. The model compares the target state to its actual state. If they are the same, the flow stops here.
3. Otherwise, the model triggers an event with its updated state.
4. The router (as a view) responds to the event with a pushState, which updates the URL in the address bar. At the same time, other views probably also respond to the update.
5. The route change finds its way back to the model, either by the router acting as a controller or through some other pathway. The new route translates to a state that the model should have.
6. We are back at step 2; the model compares the target state to its actual state. Since the model was already at the state that corresponds to the current route, the flow stops here.

You are right that a loop can still happen if the comparison in steps 2 and 6 is implemented incorrectly, but that is just a plain bug that can happen with any design. I would not consider that a flaw in the MVC division of labor.

In the second flow, in which you described the current implementation, the model and the router have a shared concern. I'm not saying that as a judgment, just an observation. If this setup works, I don't think there is a need to change it.