Inconsistent use of history leads to redundant rendering

[karger]

It appears that the standard design for views is a "reconstruct" method that gets called when they need to be regenerated based on changes to the underlying collection. This reconstruct method is called through two different pathways:

1. via an "onItemsChanged" event that is fired whenever the underlying collection changes
2. via an "importState" method that is fired by the history mechanism whenever the history state changes.

Right now, it seems that a change to the collection via a facet interaction causes both a direct change in the collection and a change to the history state, so the reconstruct method gets called twice for each collection change. This makes exhibit twice as slow, and also introduces some bugs due to races between the two rerenders. For example, in the tabular view, changing the collection resets the paginated page to zero (important if the number of items shrinks so you don't see an empty page) but importState does not. When importState runs first, you end up with a view showing no results because you're on the wrong page.

[karger]

Further investigation suggests a fundamental conceptual flaw with the way history is currently implemented. Roughly speaking, exhibit consists of a set of (mostly) independent components (views, facets) that each offer a view of an underlying collection of items. Each component has a state (sort selection, filter selection, etc.) that determines how it presents the collection.. Changes to the component's state (e.g., decision to sort a different way) cause a re-rendering of the component. Changes to the underlying collection of items also cause a re-rendering.

A history mechanism has been implemented over this conceptual design; it aims to remember each change of state so the user can return to one. To support such history navigation it is necessary for each component to be able to import and export its state for use in the history. Because support for importState is required, an apparently elegant decision was made to use only importState to manage state change for components. In other words, when a user clicks on a widget to affect the state of that component, the widget generates a state export; this in turn leads the history mechanism to trigger a stateChange event, which causes all widgets (including the one the user just clicked) to import the "new" state and re-render themselves according to it.

While apparently elegant, this approach suffers from some problems. At root, this is because the "current set of items" is not recorded as part of the state of any component. Instead, the current set of items is a function of the states of all the components (facets) taken together. When the user interacts with a facet and causes a change to that facet's state, the facet updates the current-items collection accordingly; this triggers an onItemsChanged event; all components listen for onItemsChanged and re-render when the event occurs. This indirect method is how an interaction with one component can change what's displayed in another, even though the components have no apparent direct interaction.

A lesser problem is performance. Because every facet change triggers an onItemsChanged event, a history movement (e.g., a user jumping several steps back in their history) which changes multiple facets will trigger multiple onItemsChanged events and thus multiple re-renderings of every single component. This is a rare case, since usually a user just uses forward and back history movements that change only one facet. However, it makes it very expensive (quadratic) to, for example, load a bookmark that sets the state of multiple facets.

A more serious problem is semantic. There are cases where an onItemsChanged event can trigger a change in state for a component. A particular example is pagination: a (sensible) decision was made that when a facet is modified (to filter the items differently) any paginated view should be reset to page 0. Sometimes changing pagination is unavoidable, e.g. when the newly filtered set of items is too few to reach the current page. But even if not, it seems intuitive that the user will not want to start seeing their newly filtered list from the middle (and may be confused if they don't notice they're at the middle).

But this raises a complicated issue: a change to a facet F, which is effected by invoking a stateChange that leads to importState calls by all the components, may cause all components including some particular component X to update their states and re-render, then cause an onItemsChanged event from F, which will cause component X to change its state (e.g. pagination). According to the discussion so far, this should trigger another stateChange call. But that will cause every component to re-render again. So at the very least, we're getting a lot of wasteful duplicate rendering causes by a single component's state change (the common case). If we do a big history jump which causes many facets to change state, this problem is compounded (quadratically).

In fact, the present implementation behaves differently: a change to pagination is handled directly without triggering a pagination event. On the one hand this is good; it avoids (some of) the redundant renderings. It also means we don't need to figure out how to handle a stateChange that is triggered recursively during another statechange.

But this creates a semantic bug. Because the change to pagination is not exported as part of the current state, it won't be remembered. Instead, the current state reflects the pagination before it gets reset. So, returning to this history from the future can produce inconsistent results with what the user is seeing now.

There's no apparent local fix for these problems. Because all re-rendering are handled through a stateChange event, interacting with just a single component causes every widget to think about updating. A stateDiffers method is used to let a component decide it doesn't need to bother re-rendering, but in fact these stateDiffers computations are wasted when we know which component has just been updated by the user.

More significantly, it's clear that we need to get pagination resets into the history so the history becomes semantically correct. Doing so is a challenge because the new state is stored before being passed to individual components' importStates. We can't just generate a new state in the history list to reflect the updated pagination because that would put a state in the history list that the user didn't generate: one with old pagination and one with new.

One option might be to "hold" the new state we generate and collect changes to it (from recursively generated pushState calls) until it "settles down", at which point we could store it. However, storing a new state is what generates the stateChange event that currently triggers updates; we would need to update all components without triggering the stateChange and, equally important, avoid updating components as a result of the stateChange that happens when we finally store the new state. We'll need to worry about race conditions if a user executes a history move while we're in the middle of state updates.

Another option would be to not generate the new state at all until it's ready: we could let individual components update directly as a result of onItemsChanged events, and only afterwards construct the new state. This has some of the same issues as the previous approach, but may be more efficient since it doesn't bother creating and destroying states that aren't needed.

Both of these options are essentially creating a second update pathway from the stateChange pathway, which is less elegant and may also lead to bugs if someone forgets to ensure that both pathways (one triggered by a direct user interaction with a widget, the other triggered by a user's history navigation) work correctly.

[karger]

Pagination is a particularly illuminating case here. Consider the case where a user is moving "forward" and "backward" through the history. If the history involves a series of pagination changes, then the movement in history should cause that pagination to evolve in the expected way, e.g. "forward" moves forward one page in the pagination. But what should be done if the user "jumps" to a distant history entry which has a different collection but where the page is nonzero. By consistency with the first example, it would seem we should generate the page specified in the history. But as discussed in the previous comment, there is also great sense in going back to page 0 when items change, so as not to confused the user by placing them in the middle of a results list. Which of these two contradictory principles should be applied here?

And how should it be implemented? Right now, in tabular view, an onItemsChanged event causes the page to reset to zero; we may need to detect that the onItemsChanged event is being triggered by a history jump and, because of that, not reset the page to 0.