swift-cross-ui icon indicating copy to clipboard operation
swift-cross-ui copied to clipboard

Fix ForEach Collection change rendered View correctness

Open MiaKoring opened this issue 3 months ago • 1 comments

This pr fixes the issue of only the last items getting removed from the rendered content, even if they werent the ones changing and item insertion/reordering correctness like mentioned in https://github.com/stackotter/swift-cross-ui/issues/243

I added apple/swift-collections as a dependency, I’m using OrderedSet in the changes.

If a duplicate identifier is detected every node gets replaced with a new one, diffing is not possible.

For unique identifiers:

  • If it was included in the previous update, the node gets reused. It checks if there is an identifier it doesn’t know from the previous update anywhere before it, if there is, ongoing every existing node gets removed from the view graph and reinserted at its new place.

  • if it wasn’t included, a new node gets created and ongoing every existing node gets readded.

  • if its the first update every node gets added to the view graph

  • if there was no duplicate the nodes included in the previous update not included in the current one get removed from the view graph

  • if there were duplicate(s) every node gets replaced

Also I added some new initializers: for Identifiable, setting the identifier key path to .id if not specified otherwise for existing identifier a new one allowing customization of the id KeyPath

Sadly this PR needs to be a breaking change. I was forced to add labels to the elements property on some existing initializers. Otherwise the Swift Compiler wouldn’t select the right initializer (or even select one). disfavouredOverload didn’t help. While updating you can decide between adding the label (if needed) or a keyPath. Choosing the label option uses the same update method as before. With adding a keyPath for the Identifier you choose the new, improved update method. ForEach with Range or [Identifiable] automatically recieve the new method.

I added a new Example App, ForEachExample, showcasing deletion, insertion, appending and reordering.

I tested iOS, macOS, macCatalyst, GtkBackend on Linux and WinUIBackend on Windows successfully.

While it works with non-unique Identifiers I strongly recommend using unique and constant Identifiers. It should be considerably more performant due to it making as few as possible operations on the view graph.

In my tests it still was consistently about 11% faster than the previous implementation.

As soon as the reason for a view update is available ForEach’s update method should be optimized to do only whats necessary. For Example the whole diffing, reordering,… is obsolete when only the size changed. For now at least the correctness got fixed and performance at least slightly improved.

MiaKoring avatar Nov 06 '25 17:11 MiaKoring

Edit: Fixed

oh, apparently I mixed some of my branches… there are changes that not belong to this pr… I’m going to try and remove them...

MiaKoring avatar Nov 06 '25 17:11 MiaKoring

But again, I'm happy for that to be left as a future direction in order to get your existing ForEach improvements merged.

Good point, I was not aware of this being that bad on Gtk. I’m going to create an issue for the required update and fix it in a future PR.

MiaKoring avatar Jan 03 '26 17:01 MiaKoring

Actually, I just realised that we should have the original init with some fallback implementation and a deprecation warning. I'll do that now.

stackotter avatar Jan 05 '26 05:01 stackotter