Achieve parity with classic foreach functionality (afterRender etc.)

[nmehlei]

In my codebase, I implemented a wrapper around the foreach binding that handles custom scrolling behavior. To update scrolling, I use the afterRender and beforeRemove hooks of said binding.

It would be nice if knockout-fast-foreach could also offer these hooks. That way, it could act as a drop-in replacement as currently I can't use it without them.

[cervengoc]

@nmehlei @brianmhunt I'm planning to work on it soon, because I need it too. Doesn't seem to be too difficoult at first glance.

[brianmhunt]

Fixed (I believe) as of v0.5.4? :)

[cervengoc]

You've added afterAdd and beforeRemove. Not sure if this is compatible. Shouldn't afterAdd be called afterRender?

I think @nmehlei ment compatibility like one should be able to just replace all foreach to fastForEach. But for that to work, parameter names should be exactly the same.

[4ver]

What's being passed to the two callbacks is different so they're not compatible- afterAdd returns the item added to the array whereas afterRender returns the rendered dom elements.

[brianmhunt]

Yeah I noticed that the callbacks are not compatible – I'm not clear on what they should do. Mulling it now. :)

[brianmhunt]

Sorry folks – I've dropped the ball on afterRender and afterAdd, and I've a couple things to do before I get back to it – I'd welcome a PR, but will hope to get it fixed up ASAP.

Thanks for the feedback and patience!

Cheers

[cervengoc]

Some notes for future development.

• Knockout exposes two events: beforeMove/afterMove. It will need some thinking how we could add support for these. I made a move implementation which is still in a PR state, but that implementation detects moves only when data gets reinserted, so I see now way to have something like beforeMove. However, afterMove could be called when a move is detected at reinsertion.
• afterRender should be implementable just fine. We could call it when inserting the DOM nodes for each data item.
• afterAdd should be implementable too, by checking wether the insertion comes from the initial load, and skip calling it if yes (if I understood correctly the current KO behaviour). I also like the idea of allowing users to return promise-like objects. It's questionable if when a move is detected we should still call this, or only afterMove.
• beforeRemove needs thinking too. Because of my current move implementation, we don't know at node remove time if it finally will be really a remove, or the nodes will get reinserted due to a detected move. Still thinking about it when and how this should be called. UPDATE Currently the code calls beforeRemove at actual removal after move detection because while move detection hasn't finished, the pending removed nodes remain in their places. This should be fine I think. So when a move is detected, only an afterMove call would be made.

@brianmhunt What additional value would be added here by implementing a ...All version of these callbacks? Don't exactly see now. I think as far as these callbacks go, being synchronous or not shouldn't matter too much.

[brianmhunt]

Thanks for the comments @cervengoc –

I am of the mindset that an afterAddAll would be misleading in the sense that because it's all processed in an asynchronous batch one might come under the misunderstanding that ...All refers to everything that's added – but the reality is that it's nondeterministic. It might be clearer if they were named afterAddBatch. Is this a real concern?

The single-item callbacks should be synchronous, in the same event loop as FFE's modification of the respective DOM item.

From FFE's perspective, the batch-item callbacks should naturally happen after the queue is flushed. One could batch them on some other condition, but I'm not sure what it would (could) be in library-land.

[cervengoc]

@brianmhunt What do you think about the beforeMove problem?

[brianmhunt]

What do you think about the beforeMove problem

I think it's tricky ;)