Investigate WinRT improvements for updating items, async flow, refiltering items after updates

[joadoumie]

Description of the new feature / enhancement

We need to continue to improve the current state of the world of our WinRT usage. A lot of questions have come up regarding async await patterns + the idea of being able to 'yield' current in PT Run.

This work item includes investigating the Shmueli.WinRT implementation to see what kind of benefits we may be able to get for passing around types.

Scenario when this would be used?

For all things WinRT with extensions.

Supporting information

"Just copying the hacker news http logic, but would be nice to allow extensions to use proper async code... Maybe ties into if we do filtering and are polling extensions less often in the new model, maybe we have a callback pattern for results/updates, so the extension can just spin off a thread to do a search or longer operation, if needed vs. having to just return data right away?"

The discussion revolved around the possibility of extensions returning items with a group string, allowing the UI to handle the grouping instead of extensions having to structure their data. This could be achieved with a public async Task<IEnumberable> Getitems() method, or possibly an IAsyncOperation for WinRT. The extension author could then use yield return to transpose results from another data model to the item.

[zadjii]

This sounds like a combination of a couple different asks:

• Something about use Shmueli.WinRT (tracked in #77)
• Should the return types of the WinRT methods be IAsyncAction<T> vs just T
  • I was under the impression that WinRT's IAsync across processes was Considered Harmful.
  • And I assumed that even a synchronous method implemented out-of-proc could be async in the extension process. (or it doesn't have to be! it's up to the extension). It would just be the responsibility of the hosting app (the WinRT client in this case) to make sure to treat the call as async (it's x-proc)
• Then there's a discussion of how we structure "sections".
  • I'm totally fine switching that to being an optional property on the IListItem itself, rather than a top-level ISection of IListItems. It was kinda hard to implement sections like this.
• The prototype also didn't properly support changes to the list of items yet (ItemsChanged as in the spec IIRC), so that definitely needs to be done

Should we separate some of this out?

[joadoumie]

Yeah this was the top level item that needed to be broken out -- seems like you've done that above, so I'll go ahead and create the additional items and link them here (there is already one for Shmueli).

Based on feedback from Hackathon and folks building extensions -- it'd be interesting if we could make things a bit better for devs by:

• Allowing them to do actual async code @michael-hawker can you help explain more about what you meant by this?
• Using Enumerable vs array/list --> this would allow us to potentially yield results. @plante-msft was looking into file search and it just felt slow because each time you filtered the list of items to return you had to return an array and construct a new array more or less.

[michael-hawker]

It was about using the newer C# IAsyncEnumerable pattern to help synchronize between the extensions and the app to incrementally return results. But talking to Sergio, there's not equivalent WinRT, so that's probably a non-starter?

[joadoumie]

Probably naive question - is that something we can make better in WinRT if it's an important paradigm?

[zadjii-msft]

is that something we can make better in WinRT if it's an important paradigm?

Probably not, certainly not in the timelines we're thinking about.

FWIW I think the best notes I had on the topic are around here: https://github.com/zadjii-msft/PowerToys/blob/main/src/modules/cmdpal/doc/initial-sdk-spec/initial-sdk-spec.md#updating-the-list

[!WARNING] We chose this API surface for a few reasons:

• IObservableCollection, which has quite a good mechanism for specifying exactly which parts of the collection changed, isn't exposed through WinRT
• IObservableVector doesn't work well across process boundaries
• In general, all the collection WinRT objects are Considered Harmful in cross-proc scenarios

But we want both static and dynamic lists to be able to update the results in real-time. Example: A process list that updates in real-time. We want to be able to add and remove items from the list as they start and stop.

---

To mitigate how dumb all that is, the plan I came up with Hart was:

• For a IDynamicListPage, CmdPal will call IListPage.SearchText(string) whenever the search text changes.
  • An extension doesn't need to do anything with that. Like, for example, the Yahoo Fantasy football app doesn't actually start searching for a player till you type three letters. The first two calls to SearchText are just no-ops
• Whenever an extension wants to change its own list of results, it raises an ItemsChanged
  • [A] the extension should store its list of items at this point.
• CmdPal responds to that by calling GetItems on the extension
• the extension replies to GetItems with the items it stashed at [A]

That way, the replies to GetItems should remain quick

(this also doesn't look spec'd very well)

[zadjii-msft]

That however does not deal at all with anything "paginated" though. Like, the HN extension just returns the first 24 items, with no way to indicate "I could have more items for you if you want".

That ^^^^ I had mentally punted out of v0, but maybe we shouldn't

[michael-hawker]

I don't know if it's projected in WinRT, but there is the ISupportIncrementalLoading interface too.

I think that's the crux is how much does the UI pre-filter what it has vs. when it goes to the extension and what does that look like and how we minimize the creation of new/duplicate items:

• in the extension itself (i.e. it keeps its own list between calls - part of this would be examples we create to show best practice here)
• in the UI and the coordination between the items from the extension and what's used by the UI
  • are we able to leverage the objects directly? or have to tie to other objects, etc...
  • During the hackathon, this was made difficult by the nested hierarchy of sections of items vs. deferring the grouping to the UI (though that would assume all the items are of the same 'type').

[zadjii-msft]

Proposed changes to Lists

Okay some feedback from the hackathon was that we didn't exactly support extensions that wanted a list of items that changed over time. Additionally, the IDynamicListPage.GetItems was a little... bad. You had to reply to the chenge in the query immediately in the GetItems call (again, because there was no way to change the list of items after you returned it).

To remedy, I'm proposing the following changes to the API:

+ interface INotifyItemsChanged {
+     event Windows.Foundation.TypedEventHandler<Object, ItemsChangedEventArgs> ItemsChanged;
+ }
+ runtimeclass ItemsChangedEventArgs {
+     Int32 TotalItems { get; };
+ }

-interface IListPage requires IPage {
+interface IListPage requires IPage, INotifyItemsChanged {
    // DevPal will be responsible for filtering the list of items, unless the 
    // class implements IDynamicListPage

    String SearchText { get; };
    String PlaceholderText { get; };
    Boolean ShowDetails{ get; };
    IFilters Filters { get; };
    IGridProperties GridProperties { get; };
+   Boolean HasMoreItems { get; };

    IListItem[] GetItems(); 
+   void LoadMoreItems();
}

interface IDynamicListPage requires IListPage {
-    IListItem[] GetItems(String query); // DevPal will do no filtering of these items
+    // DevPal will do no filtering of these items
+    void SearchText { set; };    
}

To summarize:

• We're explicitly adding an ItemsChanged event to lists
• Lists can now state that they have more items than what they have returned so far[^1]

So, for something like a dynamic page (let's imagine, the GitHub issues search)

1. CmdPal loads the ListPage from the extension.
2. It's a IDynamicListPage, so the command palette knows not to do any host-side filtering.
3. CmdPal reads the SearchText from the ListPage
   • it returns is:issue is:open as initial text
4. CmdPal reads the HasMoreItems from the ListPage
   • it returns true as initial text
5. CmdPal calls GetItems()
   • the extension returns the first 25 items that match the query.
6. User scrolls the page to the bottom
   • CmdPal calls GetMore on the ListPage, to let it know it should start fetching more results
7. The extension raises a ItemsChanged(40), to indicate that it now has 40 items
   • CmdPal calls GetItems, which the command returns the whole list of items
     • CmdPal does an in-place update of the existing items - ignoring the unchanged ones, and appending the new ones to the end of the list
   • The extension probably also raises a PropChanged("HasMoreItems") here, but we'll skip that for simplicity
8. The user types foo.
   • CmdPal calls page.SearchText("is:issue is:open foo"), to let the extension know the query has changed
9. The extension does the background query to match
10. The extension raises an ItemsChanged(5), to indicate there are 5 results
11. CmdPal calls GetItems to fetch the items.

So that kinda covers both Pagination and making async dynamic query updates simpler. Or if you wanted to be simpler, you don't need to set HasMoreItems. Say you just had the PingExtension, that just does a ping -t 8.8.8.8. Every second, it writes the most recent line of output to a new ListItem. In that case, the author can just raise an ItemsChanged each second, and return the updated list each time.

[^1]: HasMoreItems / LoadMoreItems is almost exactly ISupportIncrementalLoading, but without 1: the async aspect, 2: forcing us to pull in XAML as a dependency, and 3: having possibly branched inheritance trees, which x-proc winrt hates. We can still use ISupportIncrementalLoading on our side, wrapping up a list that implements that.

[michael-hawker]

@zadjii-msft looking like a good step forward. All for trying to own filtering more in the UI layer, makes basic scenario simpler for extension developers. Couple of comments/questions:

1. In your examples you've called out the SearchText like a method, but then the interface has a property? Did you mean it to be a function?

void SearchText { set; };

vs.

page.SearchText("is:issue is:open foo")

2. We could have a method that extension can call to essentially clear out the current results? i.e. just wondering if we want to define the possible behaviors/scenarios between the app and the extension a bit separately vs. as one long example. That way we can understand if we have all the pieces we need where we're either drilling down to finer results or the extension needing to 'restart' to return a new list of items based on the changed query.

3. For the following remark on how we update the UI:

CmdPal does an in-place update of the existing items - ignoring the unchanged ones, and appending the new ones to the end of the list

If we want to support updating existing ones in-place, this seems to preclude that they'd be record types for items and instead classes, eh? (Is there an interface here or do we just have a basic object that holds the raw data)? So, if it is something where you can update the text of an existing item, then I imagine we'll have some sort of Id per item? Otherwise, how do we expect the UI to know which items returned correspond to the prior items it received? If they're re-serialized we won't have a direct reference to the instance to compare, right?

[zadjii-msft]

In your examples you've called out the SearchText like a method, but then the interface has a property? Did you mean it to be a function?

You know, that comes from my background usually writing C++WinRT, where properties are implemented as methods.

We could have a method that extension can call to essentially clear out the current results

I don't see why an extension couldn't just raise ItemsChanged, then return [] in the subsequent call it'll get for GetItems. Maybe I'm missing something on this scenario?

(I'll reply to 3 tomorrow)

[michael-hawker]

Thanks @zadjii-msft, I forgot the base event was there too. So, that's good.

Then I think the two scenarios I'm trying to distinguish with would be the difference in requesting more items because we're scrolling and that just being the collection of new items vs. the search changing and the extension wanting to obliterate the existing list and just return new results. Right now, they kind of look the same, right? So, just curious if we want an explicit remove/clear vs. update/add? (Probably ties into whatever thoughts you had on 3 as well.)

[zadjii-msft]

Okay so point 3:

What I'm imagining was heavily inspired by the way Terminal's Command Palette filters items. There, when the filter changes, we take the list of items for the updated filter, and "merge" it with the items we're already displaying. We found doing it that way makes the ListView update insanely smoother. Admittedly, Terminal only has strings to check for equality, while we'd have big-ol objects.

What I'm imagining here, is that when CmdPal fetches items in GetItems, the host will cache the important information into a local object / record / something. (in the current sample, WindowsCommandPalette.ListItemViewModel reads the properties from the IListItem into itself, so that if we wanted to read the list item's properties, it's not an IPC call).

If we can make sure that those host-local "wrappers" for an IListItem can be checked for equality across calls to GetItems, then I think the extension doesn't need to "clear" the list. Like, if an extension returns just [ObjectA] in the first call, then [ObjectA, ObjectB], we shouldn't need to build a new ListItem wrapper for ObjectA, and insert that new one. The list already has A in it.

Similarly, if the extension then returns [ObjectC, ObjectD, ObjectB], we as the host should be smart enough to discard A from the list, insert C and D at the start, and leave B alone.

Ultimately, I think on the host side, we can keep things seamless to the extension authors. If they just append to the end of the items returned, then we'll just add more items to our listview. If they blow away all the old objects and return new ones, we'll clear the list and display the new ones. If they just insert items into the middle of the results - so will we.

I'm assuming that even though the IListItem objects are out-of-proc, we can check for equality on the host side trivially. If we can't then we'd need to think about some sort of Title+Subtitle+icon+tags hash.

I think this makes sense in my head, but I can tell that this probably isn't the best explanation. I also have very little depth on the best way to implement stuff in C#. My C++ days might be further complicating my thoughts here.

[michael-hawker]

Sounds good @zadjii-msft, I think we're on the same page here 🙂 with how things should wrap within the host. There'll definitely be some ListItemViewModel that contains the IListItem reference (that was also the benefit of flattening the section, as then we can more easily have the 1:1 mapping between the extension, host, and the UI layer without more indirection.

I think the crux will be when we get the results from GetItems and its [ObjectA, ObjectB] and we already have ObjectA wrapped in the host, how easy is it for us to detect/compare that ObjectA that we have wrapped is the same ObjectA returned by the GetItems call that represents that same item. It probably is just a simple override to the GetHashCode method such that if an item has the same Title+Subtitle, etc... then it's the same.

We could even exploit this such that the ListItemViewModel and ListItem itself compare equally such that we wouldn't have to do unboxing as much between them when trying to find if it exists in the host or not yet. And then only wrap the new ones at the end if we find out they're indeed new.