So I've build up a different approach based on some points that were bothering me. It's too big to be inlined on a comment so I created that gist.

Concerns addressed

1. Too much callbacks : in the current version you have 2 callbacks one for context creation and one for context update. It's not implemented yet but this was also meant to add callback so user can react to item changes (itemAdded, itemMoved, itemRemoved, identityChanged). So I tried something that would cover everything with a single callback updateContext(context, item, operation) because when we need to update a context it's because something changed. TODO rename because when item is removed we don't really update the context.
2. Indirect dependency to IterableDiffer: When we first talked we didn't know if that class would be tied to IterableDiffer or abstracted from it. Now it seems there is no doubt it should rely exclusively on IterableDiffers but yet the dependency is implicit and hidden. There is no direct dependency to IterableDiffer but applyChanges() require an IterableChanges. I really think we had better directly manage the IterableDiffer ourselves we'll reduce even more the boilerplate for users.

New Concern

With that implementation the constructor looks like this :

constructor(
    private _viewContainer: ViewContainerRef,
    private _template: TemplateRef<R>,
    private _trackBy: TrackByFunction<T>,
    private _differs: IterableDiffers,
    private _updateContext: (context: LoopContext<T> | null, item: T, action: ItemAction) => void
) {}

Seems like a lot of arguments, also the usage will require user to inject most of it on the Directive to just pass them by to that class.
I think a good way to resolve that would be to build that class as a Directive service (as opposed to singleton service) so it can inject itself most of the stuff it needs. The usage could then be :

@Directive({
  selector: 'myNgFor',
  providers: [ ViewSyncService ]
})
export class MyNgFor {
  @Input()
  set trackBy(trackBy: TrackByFunction<T>) {
    this.viewSync.setTrackByFn(trackBy);
  }

  constructor(private viewSync: ViewSyncService) {
   viewSync.setUpdateContext( ... );
  }
}

Defining trackBy and updateContext using a set function isn't optimal but at least for trackBy it isn't that bad because even if it's not intended to change it's still an @Input() so it could change over time.

@ghetolay I have a few issues with with the mentioned changes.

Making it a Directive

• I could be wrong here but I don't think Directives are shakable by the common bundlers, which for new API's I think is an important attribute to have.
• Just sort of feels hacky, a class feels right for this kind of API

Directly managing ItterableDiffers

• Since you use this.differs.find(array).create(trackBy), that means everyone will have to use that, but what if they want to use it in some other way, or if the differ already exists, or hook into some other part of the IterableDiffers' changes.

In general I think managing the IterableDiffers is creeping a bit far out of what I think this class should handle. I think it should just handle Updating the View from an IterablesChanges

As for my implementation I was thinking of providing an Observable that you can subscribe to for the update, move, delete operations rather than proving callbacks, similar to how you've done it but Observables instead of callbacks, it'll clean up that constructor a bit doing it this way

I could be wrong here but I don't think Directives are shakable by the common bundlers, which for new API's I think is an important attribute to have.

I think you meant service not directive and I don't know if non-singleton directive are shakable or not. But as I have stated here #18015 I like that pattern but not as it is at the moment because if you forget providers: [ Service ] you may end up with no error but the wrong service (a parent one). This makes it error-prone, I was just throwing an idea.

As for my implementation I was thinking of providing an Observable that you can subscribe to for the update, move, delete operations

My idea here was since we need a updateContext() for most operations (all except delete) we could use that same callback to notify about the operation resulting to that need of context update. Ending to only 1 callback instead of 1 callback + 4 callback/observable and only one callback called for each operation instead of 2 (update context + notify).

Since you use this.differs.find(array).create(trackBy), that means everyone will have to use that, but what if they want to use it in some other way, or if the differ already exists, or hook into some other part of the IterableDiffers' changes.

I stand my position here but I think we covered it, we just disagree and someone will have to make the call.
Quick recap about why I think we should include the differ :

• More user friendly API, straightforward for the user as opposed to building its own differ which is viewed as expert stuff.
• There is only 1 way to create a differ and it only has 1 function so :
  • I don't see what else it could be used for by the user.
  • We'll reduce redundant code as it's always the same code to create and run a differ.
• Even if user wanted to hook into the callback on the differ we're giving him the exact same callbacks so it can directly use those we provide. If we don't manage the differ I don't see why we're providing the same hooks as the differ already does (add, remove, move, identity changed).
• Indirect dependency, that class has no direct dependency to IterableDiffer but by using IterableChanges it really does have one because I don't see anybody using that interface so that dependency is hidden.

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}