Repeater static values vs dynamic refresh

[JustinGeorgi]

I'm moving the conversation on the forms here after further testing:

Quick recap:
Issue #1804 pointed out that oh-repeaters using a calculated value for the array source could be a significant drain on processing in larger systems due to constant api calls when the component is redrawn.

PR #1904 addressed this issue by calculating source once on component creation and caching that array for all future redraws of the repeater and its children.

Issue #1918 (and related forum topics) pointed out that this broke the repeater for several user who relied on the ability to refresh repeater information without a full page refresh. This issue included an example where the in array of the repeater was dynamically defined but the repeater contents would not change even when the input array changed.

PR #1919 addressed this issue by creating a carve out in the caching method for the repeater's in array definition. This did not fix the non-refresh issue for most users, especially users as discussed in the above linked forum topic.

The refresh issue can be addressed by users by, instead of refreshing the repeater directly, refreshing one of the repeater's parents. This solution, however, is suboptimal because that destroys the repeater and redraws it from scratch which results in a significant visual change in the UI when elements shift with the repeater's removal and shift back when it is redrawn.

Now:
After further testing I think there is an argument for going back to the previous computed source method that refreshes the repeater in response to object and css changes, but to add a new repeater property (e.g., static) that caches the array and then prevents further updates.

As I was looking at the changes in PR 1919 that allowed for the in property to override the cached source array I realized that there a many more reasonable situations parallel to the demo widget used in 1918 that apply as well. And it's likely that broad adoption of OH4 with this change will break a lot of widgets.

For example, let's think about a common enough simple widget that uses an array to list the equipment in a location. Easy, that's just a itemsInGroup source with an itemGroup property. But, now let's say the user wants three buttons which change that group between Kitchen, Dining room, and Front Hall. That's also easy, itemGroup can be set to an expression with the variable value that's determined by the buttons. The problem is, with this new system, that repeater won't refresh when any of the button are pushed. A change to the variable will cause the repeater to be re-evaluated, but the cached array will be used and there will be no change in the repeater output. So, now we have to create a second carve out in the cache method for the itemsInGroup kind of arrays.

The problem is we can do this for each type of source. It's trivial to change the example above to a widget that lists the all Speaker equipment (an itemsWithTags source type) but with buttons that select other equipment types (again, a very reasonable kind of widget). So, now we need to consider a carve out for itemsWithTags.

Based on this thought process, I've come to the conclusion that it is more sensible to return the repeater closer to it's original form while allowing users to opt in to caching the source array.

1. This is less of a breaking change
2. Because it uses a property to define whether to cache the source it can even be dynamically defined so that users have real-time control over whether to use caching or not.
3. The modification is very light and I have already tested what I believe to be a fully working version.

Before submitting a PR with the change, however, I wanted to open a direct discussion about it, because it does fully revert the changes in 1904 and 1919 by returning to the asynComputed method for source

Edit:
As discussed at the end of the forum topic, I did some testing as well with binding the key property properly to the repeater. This has the same undesirable visual impact as refreshing a repeater parent element because the repeater is destroyed and redrawn.

In contrast, when the repeater source array is just recalculated as the result of an object change, the repeater's elements are maintained and only the children change resulting in a smoother visual transition.

[florian-h05]

I‘d do it that way:

1. Rename load method to something along loadPromise and make it return a promise.
2. Add a new boolean parameter cacheSource.
3. Re-introduce asyncComputed source, which uses loadPromise to load the data from REST API if cacheSource is not true.
4. If cacheSource is true, load the data on widget creation and return the cached data from the async computed source.

However I‘m open to have a look at any proposals in PRs ;-)

[JustinGeorgi]

OK, Pr is in. I went with the full previous source definition because it seemed to me, at that point that the separate load method was redundant (if I missed some other improvement that provides, let me know and I can go back and try it with the load method).