CommunityToolkit / Windows

Collection of controls for WinUI 2, WinUI 3, and Uno Platform developers. Simplifies and demonstrates common developer tasks building experiences for Windows with .NET.
https://aka.ms/windowstoolkitdocs
Other
601 stars 74 forks source link

Implement IncrementalLoadingCollection from IAsyncEnumerable #167

Open Licantrop0 opened 1 year ago

Licantrop0 commented 1 year ago

Overview

To implement an IncrementalLoadingCollection, you need:

  1. IIncrementalSource from the dotnet repo, CommunityToolkit.Common namespace
  2. IncrementalLoadingCollection from the WindowsCommunityToolkit repo, Microsoft.Toolkit.Uwp namespace.

The API design is quite dated and requires creating a source class implementing IIncrementalSource<TItem>, and creating IncrementalLoadingCollection<TSource, TItem>.

IIncrementalSource requires to implement a method returning Task<IEnumerable<TItem>> that is modernly expressed with IAsyncEnumerable<TItem>.

IAsyncEnumerable<TItem> allows cycling over the collection in a fully asynchronous way and it's the perfect fit for a modern IncrementalLoadingCollection. I called this new type AsyncLoadingCollection

API breakdown

public class AsyncLoadingCollection<T>(IAsyncEnumerable<T> source, uint itemsPerPage = 25) : ObservableCollection<T>, ISupportIncrementalLoading
{
    private IAsyncEnumerator<T>? _asyncEnumerator = source.GetAsyncEnumerator();
    private readonly SemaphoreSlim _mutex = new(1, 1);

    public bool HasMoreItems => _asyncEnumerator != null;

    public IAsyncOperation<LoadMoreItemsResult> LoadMoreItemsAsync(uint count = 0) =>
        LoadMoreItemsAsync(count == 0 ? itemsPerPage : count, default)
        .AsAsyncOperation();

    private async Task<LoadMoreItemsResult> LoadMoreItemsAsync(uint count, CancellationToken cancellationToken)
    {
        await _mutex.WaitAsync(cancellationToken);

        if (cancellationToken.IsCancellationRequested || !HasMoreItems)
            return new LoadMoreItemsResult(0);

        uint itemsLoaded = 0;
        var itemsToLoad = Math.Min(itemsPerPage, count);

        try
        {
            while (itemsLoaded < itemsToLoad)
            {
                if (await _asyncEnumerator!.MoveNextAsync(cancellationToken).ConfigureAwait(false))
                {
                    Add(_asyncEnumerator!.Current);
                    itemsLoaded++;
                }
                else
                {
                    // Dispose the enumerator when we're done
                    await _asyncEnumerator!.DisposeAsync();
                    _asyncEnumerator = null;
                    break;
                }
            }
        }
        catch (OperationCanceledException)
        {
            // The operation has been canceled using the Cancellation Token.
            await _asyncEnumerator!.DisposeAsync();
            _asyncEnumerator = null;
        }
        catch (Exception)
        {
            await _asyncEnumerator!.DisposeAsync();
            _asyncEnumerator = null;
            throw;
        }
        finally
        {
            _mutex.Release();
        }

        return new LoadMoreItemsResult(itemsLoaded);
    }
}

Usage example

public static async IAsyncEnumerable<string> GetSampleData()
{
    for (var i = 0; i < 100; i++)
    {
        await Task.Delay(i * 10);
        yield return $"string {i}";
    }
}

var myAsyncCollection = new AsyncLoadingCollection<string>(GetSampleData());

// this is needed for the initial load (viewControls like DataGrid don't call it automatically after DataContext change)
await myAsyncCollection.LoadMoreItemsAsync();

Then you can bind myAsyncCollection to a DataGrid as ItemsSource

Breaking change?

No, it's an addition to IncrementalLoadingCollection.

Alternatives

After few days of research, I couldn't find any other implementation of an IncrementalLoadingCollection using IAsyncEnumerable.

Sergio0694 commented 1 year ago

Hey there, thank you for the feature proposal! Unfortunately this is out of scope for the .NET Community Toolkit and just could not be added to it at all, as the proposed implementation has a hard dependency on several WinRT and WinUI 3 APIs (ISupportIncrementalLoading, LoadMoreItemsResult, IAsyncOperation<T>), which are just not available from purely .NET projects at all. We'd need a dependency on WinAppSDK and WinUI 3 to access those APIs, and that's unfortunately not something we can do in the .NET Community Toolkit, as being completely framework-agnostic is one of its core principles. This API seems best suited for a separate package outside of the .NET Community Toolkit. Perhaps the Windows Community Toolkit might be a good fit for a proposal like this? 🙂

ghost commented 1 year ago

Hello Licantrop0, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

michael-hawker commented 1 year ago

Thanks for the issue @Licantrop0, wonder if this makes sense to be it's own class, if there should be a common base class, or should there just be another constructor on the IncrementalLoadingCollection that takes the new interface?

Looks like we would need the https://www.nuget.org/packages/Microsoft.Bcl.AsyncInterfaces reference for UWP support, but otherwise, should be able to make it work universally.

Licantrop0 commented 1 year ago

Another constructor on IncrementalLoadingCollection would introduce some complexities (e.g. forking between _source and _asyncEnumerable all over the places) and the original class definition IncrementalLoadingCollection<TSource, IType> : ObservableCollection<IType>, ISupportIncrementalLoading where TSource : IIncrementalSource<IType> would not make much sense for the IAsyncEnumerable implementation.

A base class could be a good idea, as they both implement ISupportIncrementalLoading and they cater to the exact same scenario. However, considering the differences in the constructor and the way data is sourced, merging them into a single base might result in an architecture that's somewhat convoluted and harder to maintain, if the base contains a little more code than an Interface.

I can write a couple of proposals (extending the code above to implement all the other features like OnStartLoading, OnEndLoading, and OnError), and then we can vote for the best one.