From ee48c93f6a67b871380129ca251a19f62d9c8864 Mon Sep 17 00:00:00 2001 From: Immo Landwerth Date: Fri, 2 Aug 2024 15:23:39 -0700 Subject: [PATCH 01/12] Add initial draft for "Enabling Batch Events for ObservableCollection" --- INDEX.md | 1 + accepted/2024/observable_collection.md | 173 +++++++++++++++++++++++++ 2 files changed, 174 insertions(+) create mode 100644 accepted/2024/observable_collection.md diff --git a/INDEX.md b/INDEX.md index 9aa18f133..d7941bd45 100644 --- a/INDEX.md +++ b/INDEX.md @@ -86,6 +86,7 @@ Use update-index to regenerate it: | 2023 | [Multi-threading on a browser](accepted/2023/wasm-browser-threads.md) | [Pavel Savara](https://github.com/pavelsavara) | | 2023 | [net8.0-browser TFM for applications running in the browser](accepted/2023/net8.0-browser-tfm.md) | [Javier Calvarro](https://github.com/javiercn) | | 2024 | [.NET Standard Targeting Recommendations](accepted/2024/net-standard-recommendation.md) | [Immo Landwerth](https://github.com/terrajobst), [Viktor Hofer](https://github.com/ViktorHofer) | +| 2024 | [Enabling Batch Events for ObservableCollection](accepted/2024/observable_collection.md) | [Immo Landwerth](https://github.com/terrajobst), [Eirik Tsarpalis](https://github.com/eiriktsarpalis) | ## Drafts diff --git a/accepted/2024/observable_collection.md b/accepted/2024/observable_collection.md new file mode 100644 index 000000000..458c45dfc --- /dev/null +++ b/accepted/2024/observable_collection.md @@ -0,0 +1,173 @@ +# Enabling Batch Events for ObservableCollection + +**Owner** [Immo Landwerth](https://github.com/terrajobst) | [Eirik Tsarpalis](https://github.com/eiriktsarpalis) + +The .NET platform has infrastructure to enable *view models* that is, an object +model that UI can be bound to such that when the data changes the UI refreshes +automatically. + +One of the core building blocks for this is `INotifyCollectionChanged`, with +it's canonical implementation of `ObservableCollection`. + +While the shape of `INotifyCollectionChanged.CollectionChanged` always supported +expressing batched notifications, `ObservableCollection` only raises +notifications for individual items (except for the `Reset` event, which tells UI +to update everything). + +In fact, `ObservableCollection` has no APIs like `AddRange` which could allow +bulk operations. In the past we tried to add those but then we learned that some +of the .NET platform pieces took a dependency on the fact that +`ObservableCollection` raises events for individual items and crash when the +event args represent more than one item, specifically WPF. + +From a UI standpoint, bulk operations are desirable because they allow for UI +controls to be more performant when the data changes. Other UI frameworks that +support view models, such as Avalonia, Uno, and MAUI have expressed interest in +seeing this change. In fact, the [tracking issue][issue] is eight years old and +has over four hundred comments, with several folks expressing frustration that +this issue hasn't been addressed yet. + +The goal of this document is to explore ways to address this request. + +## Requirements + +### Goals + +* Enable bulk operations for `Add`, `Remove`, `Insert`, and `Replace` that + result in a single notification for view models. + +### Non-Goals + +* Support bulk notifications for discontinuous ranges in the collection + +## Stakeholders and Reviewers + +* Platform components + - Libraries team + - WinForms team + - WPF team + - MAUI team +* External UI platforms + - Avalonia + - Uno + +## Design + +### API Changes + +We don't need to make any changes to `INotifyCollectionChanged` as +`NotifyCollectionChangedEventArgs` can already represent ranges: + +```C# +public partial class NotifyCollectionChangedEventArgs : EventArgs +{ + // Constructors omitted + public NotifyCollectionChangedAction Action { get; } + public IList? NewItems { get; } + public IList? OldItems { get; } + public int NewStartingIndex { get; } + public int OldStartingIndex { get; } +} +``` + +> [!NOTE] +> Please note that the API shape only supports batched notifications if the +> affected range is contiguous, for example, inserting multiple items or +> removing a contiguous range. It doesn't, for example, support removing +> multiple discontiguous items. + +`ObservableCollection` needs to expose new APIs for performing bulk +operations that will result in bulk notifications: + +```C# +namespace System.Collections.ObjectModel; + +public class ObservableCollection +{ + public void AddRange(IEnumerable collection); + public void InsertRange(int index, IEnumerable collection); + public void RemoveRange(int index, int count); + public void ReplaceRange(int index, int count, IEnumerable collection); + + protected virtual void InsertItemsRange(int index, IEnumerable collection); + protected virtual void RemoveItemsRange(int index, int count); + protected virtual void ReplaceItemsRange(int index, int count, IEnumerable collection); +} +``` + +## How it breaks consumers + +`ObservableCollection` exists since .NET Framework 3.0, which was shipped in +2006, 18 years ago. While the eventing structure in principle supports raising +events for more than one item, the collection was never capable of raising such +events. As a result, consumers of `ObservableCollection`, specifically WPF, +have baked in assumptions that these can never happen. As a result, when tried +to make these changes a ton of code in WPF broke, i.e. it crashed with +exceptions. + +## Options to avoid breaking consumers + +* **Do nothing**. We could just do nothing and simply let customers report bugs + for controls that don't support bulk notifications. Strictly speaking, this + isn't a behavioral change as existing code won't start raising bulk events + unless the user is calling the new methods on `ObservableCollection`. The + guidance would be "use bulk operations if the data bound UI supports it, + otherwise don't". We would treat the lack of WPF support as a feature request + and let it run its course. + - Pro: Simplest API, easiest to implement, unblocks the ecosystem. + - Con: Violates our goal to not ship functionality we know won't work with + core in-box functionality (specifically WPF). + +* **Global switch**. We could add an `AppContext` switch which disables bulk + updates process-wide. + - Pro: Easy to implement, reduced complexity for the API. + - Con: limiting if some UI components support bulk updates and some don't. + +* **Per instance switch**. We could have a property on `ObservableCollection` + that turns off bulk notifications. + - Pro: Allows some part of the view model to use bulk notifications when the + UI supports it while turning it off for the parts that don't. + - Con: Still limiting when the same collection is bound to multiple UI + components and some support bulk notifications and some don't. Also + complicates the core API. + +* **Event adaptor**. We could provide an implementation of + `INotifyCollectionChanged` that wraps an existing `INotifyCollectionChanged` + and translates bulk operations to individual events. Authors of view models + could expose two properties, one with the actual collection and another one + that returns the wrapper. In data binding, they use the property with bulk + notifications if the UI supports it and the other if it doesn't. + - Pro: Core API is simple while providing an affordance to the user to solve + the problem without requiring changes from the UI framework. + - Con: More complex state, might not play well with WPF's `CollectionView`. + +* **Event enumerator**. We could expose a method on + `NotifyCollectionChangedEventArgs` that returns an + `IEnumerable`. For single item events it + returns itself, otherwise it creates a new event arg per item. This requires + UI controls to make code changes if they don't support bulk notifications, but + the change is fairly targeted. + - Pro: Simple API + - Con: Still breaks consumers, just provides an easier path to adapt to the + new behavior without having to full support bulk notifications. + +* **Handling it at the CollectionView level**. WPF seems to have an indirection + between the data-bound UI controls and the observable collections via the + collection view. Maybe we could expose a property on `CollectionView` that + controls whether the collection view will translate bulk events to individual + events. + - Pro: Simpler API for the base, with an opt-out switch at the level of WPF. + - Con: Needs more investigation to determine feasibility. + +* **Separate API**. We could add a new interface + `IBatchedNotifyCollectionChanged` interface that `ObservableCollection` + also implements. The interface adds new even `BulkCollectionChanged`. + - Pro: Consumers opt-into bulk events by subscribing to a different event. + This allows a single instance of `ObservableCollection` to be bound to + multiple UI elements and leverage bulk updates when it's supported while + falling back when it's not. + - Con: Complicates that API; requires all consumers to explicitly opt-in, even + if happen to already support bulk notifications. Will likely take longer to + be supported by UI frameworks. + +[issue]: https://github.com/dotnet/runtime/issues/18087 \ No newline at end of file From a75e6b95402965df73df33ef8061db936ec3daa3 Mon Sep 17 00:00:00 2001 From: Immo Landwerth Date: Wed, 7 Aug 2024 16:04:11 -0700 Subject: [PATCH 02/12] Update con --- accepted/2024/observable_collection.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/accepted/2024/observable_collection.md b/accepted/2024/observable_collection.md index 458c45dfc..e00ab20e4 100644 --- a/accepted/2024/observable_collection.md +++ b/accepted/2024/observable_collection.md @@ -157,7 +157,7 @@ exceptions. controls whether the collection view will translate bulk events to individual events. - Pro: Simpler API for the base, with an opt-out switch at the level of WPF. - - Con: Needs more investigation to determine feasibility. + - Con: Needs more investigation to determine feasibility, would only be helpful for WPF * **Separate API**. We could add a new interface `IBatchedNotifyCollectionChanged` interface that `ObservableCollection` From 089e5f61e9ea6794effd9742378027343a682dab Mon Sep 17 00:00:00 2001 From: Immo Landwerth Date: Wed, 7 Aug 2024 16:05:57 -0700 Subject: [PATCH 03/12] Include WinUI team Co-authored-by: Michael Hawker MSFT (XAML Llama) <24302614+michael-hawker@users.noreply.github.com> --- accepted/2024/observable_collection.md | 1 + 1 file changed, 1 insertion(+) diff --git a/accepted/2024/observable_collection.md b/accepted/2024/observable_collection.md index e00ab20e4..c8de668cd 100644 --- a/accepted/2024/observable_collection.md +++ b/accepted/2024/observable_collection.md @@ -46,6 +46,7 @@ The goal of this document is to explore ways to address this request. - Libraries team - WinForms team - WPF team + - WinUI team - MAUI team * External UI platforms - Avalonia From 3450d45007327c1ebd18aa3595f0d6207b209c0a Mon Sep 17 00:00:00 2001 From: Immo Landwerth Date: Wed, 7 Aug 2024 16:15:01 -0700 Subject: [PATCH 04/12] Clarify that even enumerator doesn't seem feasible --- accepted/2024/observable_collection.md | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/accepted/2024/observable_collection.md b/accepted/2024/observable_collection.md index c8de668cd..3545920d8 100644 --- a/accepted/2024/observable_collection.md +++ b/accepted/2024/observable_collection.md @@ -142,15 +142,20 @@ exceptions. the problem without requiring changes from the UI framework. - Con: More complex state, might not play well with WPF's `CollectionView`. -* **Event enumerator**. We could expose a method on +* ~~**Event enumerator**. We could expose a method on `NotifyCollectionChangedEventArgs` that returns an `IEnumerable`. For single item events it returns itself, otherwise it creates a new event arg per item. This requires UI controls to make code changes if they don't support bulk notifications, but - the change is fairly targeted. - - Pro: Simple API - - Con: Still breaks consumers, just provides an easier path to adapt to the - new behavior without having to full support bulk notifications. + the change is fairly targeted.~~ + - ~~Pro: Simple API~~ + - ~~Con: Still breaks consumers, just provides an easier path to adapt to the + new behavior without having to fully support bulk notifications.~~ + - > [!NOTE] + > Doesn't seem feasible. Handlers of the event likely assume the collection + > was just changed to include the one modification. Just translating the + > event would mean they get a series of events but the handler sees the + > final collection in all invocations. * **Handling it at the CollectionView level**. WPF seems to have an indirection between the data-bound UI controls and the observable collections via the From 7c22a95ccbe2d0c175682c36d201d88c0a05e49b Mon Sep 17 00:00:00 2001 From: Immo Landwerth Date: Wed, 7 Aug 2024 16:15:56 -0700 Subject: [PATCH 05/12] Clarify that new interface could support new semantics --- accepted/2024/observable_collection.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/accepted/2024/observable_collection.md b/accepted/2024/observable_collection.md index 3545920d8..381b3a102 100644 --- a/accepted/2024/observable_collection.md +++ b/accepted/2024/observable_collection.md @@ -171,7 +171,8 @@ exceptions. - Pro: Consumers opt-into bulk events by subscribing to a different event. This allows a single instance of `ObservableCollection` to be bound to multiple UI elements and leverage bulk updates when it's supported while - falling back when it's not. + falling back when it's not. This would also allow change of semantics to + allow for non-contiguous updates, for instance. - Con: Complicates that API; requires all consumers to explicitly opt-in, even if happen to already support bulk notifications. Will likely take longer to be supported by UI frameworks. From 4d4f3ed72aef9584b15a15324359773ea89cbcd6 Mon Sep 17 00:00:00 2001 From: Immo Landwerth Date: Wed, 7 Aug 2024 16:16:13 -0700 Subject: [PATCH 06/12] Clarify that new interface would likely require new type in WinUI --- accepted/2024/observable_collection.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/accepted/2024/observable_collection.md b/accepted/2024/observable_collection.md index 381b3a102..09f4dbfdf 100644 --- a/accepted/2024/observable_collection.md +++ b/accepted/2024/observable_collection.md @@ -174,7 +174,8 @@ exceptions. falling back when it's not. This would also allow change of semantics to allow for non-contiguous updates, for instance. - Con: Complicates that API; requires all consumers to explicitly opt-in, even - if happen to already support bulk notifications. Will likely take longer to - be supported by UI frameworks. + if happen to already support bulk notifications. Would also likely require + adding a corresponding interface to WinUI that .NET can project to. Will + likely take longer to be supported by UI frameworks. [issue]: https://github.com/dotnet/runtime/issues/18087 \ No newline at end of file From 99e93c772bd4e87c3c5edb066e44f673b6364d1b Mon Sep 17 00:00:00 2001 From: Immo Landwerth Date: Wed, 7 Aug 2024 16:23:00 -0700 Subject: [PATCH 07/12] Also mark Event Adapter as likely unfeasible --- accepted/2024/observable_collection.md | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/accepted/2024/observable_collection.md b/accepted/2024/observable_collection.md index 09f4dbfdf..9f938e20e 100644 --- a/accepted/2024/observable_collection.md +++ b/accepted/2024/observable_collection.md @@ -141,16 +141,21 @@ exceptions. - Pro: Core API is simple while providing an affordance to the user to solve the problem without requiring changes from the UI framework. - Con: More complex state, might not play well with WPF's `CollectionView`. + - > [!NOTE] + > Doesn't seem feasible. Handlers of the event likely assume the collection + > was just changed to include the one modification. Just translating the + > event would mean they get a series of events but the handler sees the + > final collection in all invocations. -* ~~**Event enumerator**. We could expose a method on +* **Event enumerator**. We could expose a method on `NotifyCollectionChangedEventArgs` that returns an `IEnumerable`. For single item events it returns itself, otherwise it creates a new event arg per item. This requires UI controls to make code changes if they don't support bulk notifications, but - the change is fairly targeted.~~ - - ~~Pro: Simple API~~ - - ~~Con: Still breaks consumers, just provides an easier path to adapt to the - new behavior without having to fully support bulk notifications.~~ + the change is fairly targeted. + - Pro: Simple API + - Con: Still breaks consumers, just provides an easier path to adapt to the + new behavior without having to fully support bulk notifications. - > [!NOTE] > Doesn't seem feasible. Handlers of the event likely assume the collection > was just changed to include the one modification. Just translating the From 44a6450edbff9f911b02e86f394c9d91b957eb3b Mon Sep 17 00:00:00 2001 From: Immo Landwerth Date: Wed, 7 Aug 2024 16:31:04 -0700 Subject: [PATCH 08/12] Clarify why separate interface isn't sufficient --- accepted/2024/observable_collection.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/accepted/2024/observable_collection.md b/accepted/2024/observable_collection.md index 9f938e20e..b64a6b259 100644 --- a/accepted/2024/observable_collection.md +++ b/accepted/2024/observable_collection.md @@ -182,5 +182,10 @@ exceptions. if happen to already support bulk notifications. Would also likely require adding a corresponding interface to WinUI that .NET can project to. Will likely take longer to be supported by UI frameworks. + - > [!NOTE] + > For the same reason event adapters aren't viable just having a new + > interface is probably not sufficient. Rather, we'd need a different + > implementation like `BulkObservableCollection`. However, for WinUI + > we likely also still need an interface too. [issue]: https://github.com/dotnet/runtime/issues/18087 \ No newline at end of file From 3323a17c79737c9030b72db3fb8340ca64dc2095 Mon Sep 17 00:00:00 2001 From: Immo Landwerth Date: Wed, 7 Aug 2024 16:31:27 -0700 Subject: [PATCH 09/12] Add summary of what seems feasible at this point --- accepted/2024/observable_collection.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/accepted/2024/observable_collection.md b/accepted/2024/observable_collection.md index b64a6b259..2772289cb 100644 --- a/accepted/2024/observable_collection.md +++ b/accepted/2024/observable_collection.md @@ -108,6 +108,16 @@ exceptions. ## Options to avoid breaking consumers +The most likely candidate for accommodating consumers is this: + +1. Global switch to turn off bulk updates +2. Per instance switch to turn off bulk updates + +We could decide to ship (1) first and see whether that's sufficient and add (2) +if deemed necessary. + +Below are a list of all options that were identified, with their + * **Do nothing**. We could just do nothing and simply let customers report bugs for controls that don't support bulk notifications. Strictly speaking, this isn't a behavioral change as existing code won't start raising bulk events From 9114e0c45f61a4f1bea41f2e0fd7e5c2992c60a7 Mon Sep 17 00:00:00 2001 From: Immo Landwerth Date: Wed, 4 Dec 2024 09:27:07 -0800 Subject: [PATCH 10/12] Let's see whether removing bullet points fixes the note boxes --- accepted/2024/observable_collection.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/accepted/2024/observable_collection.md b/accepted/2024/observable_collection.md index 2772289cb..7c613692d 100644 --- a/accepted/2024/observable_collection.md +++ b/accepted/2024/observable_collection.md @@ -151,7 +151,7 @@ Below are a list of all options that were identified, with their - Pro: Core API is simple while providing an affordance to the user to solve the problem without requiring changes from the UI framework. - Con: More complex state, might not play well with WPF's `CollectionView`. - - > [!NOTE] + > [!NOTE] > Doesn't seem feasible. Handlers of the event likely assume the collection > was just changed to include the one modification. Just translating the > event would mean they get a series of events but the handler sees the @@ -166,7 +166,7 @@ Below are a list of all options that were identified, with their - Pro: Simple API - Con: Still breaks consumers, just provides an easier path to adapt to the new behavior without having to fully support bulk notifications. - - > [!NOTE] + > [!NOTE] > Doesn't seem feasible. Handlers of the event likely assume the collection > was just changed to include the one modification. Just translating the > event would mean they get a series of events but the handler sees the @@ -192,7 +192,7 @@ Below are a list of all options that were identified, with their if happen to already support bulk notifications. Would also likely require adding a corresponding interface to WinUI that .NET can project to. Will likely take longer to be supported by UI frameworks. - - > [!NOTE] + > [!NOTE] > For the same reason event adapters aren't viable just having a new > interface is probably not sufficient. Rather, we'd need a different > implementation like `BulkObservableCollection`. However, for WinUI From 3e94baed7016bf4d51815568e81523e5179abfba Mon Sep 17 00:00:00 2001 From: Immo Landwerth Date: Wed, 4 Dec 2024 09:28:27 -0800 Subject: [PATCH 11/12] Add UWP team in stakeholders & reviewers --- accepted/2024/observable_collection.md | 1 + 1 file changed, 1 insertion(+) diff --git a/accepted/2024/observable_collection.md b/accepted/2024/observable_collection.md index 7c613692d..7ee0c479a 100644 --- a/accepted/2024/observable_collection.md +++ b/accepted/2024/observable_collection.md @@ -47,6 +47,7 @@ The goal of this document is to explore ways to address this request. - WinForms team - WPF team - WinUI team + - UWP team - MAUI team * External UI platforms - Avalonia From c076aa11cba3bbaa4e2081ec247304bc4511e3e9 Mon Sep 17 00:00:00 2001 From: Immo Landwerth Date: Wed, 4 Dec 2024 10:25:36 -0800 Subject: [PATCH 12/12] Fix note blocks --- accepted/2024/observable_collection.md | 33 ++++++++++++++------------ 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/accepted/2024/observable_collection.md b/accepted/2024/observable_collection.md index 7ee0c479a..56b0e8b77 100644 --- a/accepted/2024/observable_collection.md +++ b/accepted/2024/observable_collection.md @@ -152,11 +152,12 @@ Below are a list of all options that were identified, with their - Pro: Core API is simple while providing an affordance to the user to solve the problem without requiring changes from the UI framework. - Con: More complex state, might not play well with WPF's `CollectionView`. - > [!NOTE] - > Doesn't seem feasible. Handlers of the event likely assume the collection - > was just changed to include the one modification. Just translating the - > event would mean they get a series of events but the handler sees the - > final collection in all invocations. + +> [!NOTE] +> Doesn't seem feasible. Handlers of the event likely assume the collection +> was just changed to include the one modification. Just translating the +> event would mean they get a series of events but the handler sees the +> final collection in all invocations. * **Event enumerator**. We could expose a method on `NotifyCollectionChangedEventArgs` that returns an @@ -167,11 +168,12 @@ Below are a list of all options that were identified, with their - Pro: Simple API - Con: Still breaks consumers, just provides an easier path to adapt to the new behavior without having to fully support bulk notifications. - > [!NOTE] - > Doesn't seem feasible. Handlers of the event likely assume the collection - > was just changed to include the one modification. Just translating the - > event would mean they get a series of events but the handler sees the - > final collection in all invocations. + +> [!NOTE] +> Doesn't seem feasible. Handlers of the event likely assume the collection +> was just changed to include the one modification. Just translating the +> event would mean they get a series of events but the handler sees the +> final collection in all invocations. * **Handling it at the CollectionView level**. WPF seems to have an indirection between the data-bound UI controls and the observable collections via the @@ -193,10 +195,11 @@ Below are a list of all options that were identified, with their if happen to already support bulk notifications. Would also likely require adding a corresponding interface to WinUI that .NET can project to. Will likely take longer to be supported by UI frameworks. - > [!NOTE] - > For the same reason event adapters aren't viable just having a new - > interface is probably not sufficient. Rather, we'd need a different - > implementation like `BulkObservableCollection`. However, for WinUI - > we likely also still need an interface too. + +> [!NOTE] +> For the same reason event adapters aren't viable just having a new +> interface is probably not sufficient. Rather, we'd need a different +> implementation like `BulkObservableCollection`. However, for WinUI +> we likely also still need an interface too. [issue]: https://github.com/dotnet/runtime/issues/18087 \ No newline at end of file