Fix bug with nested children of other children

[rostunic]

Hi, there was a bug when loading the items and their child-parent relationships. The bug occured when the parent was itself a child of another item, for example an attachment. When a child (annotation) of that parent (attachment) is behind the parent in the map, the parent was already removed from the map and the child could not find its parent. I don't know if this is the best solution, but I added the removed items to a list and added a fallback loop to check if the parent is in the list of removed items.

A consequence of that was that not all annotations were associated with an attachment.

[frthjf]

Thanks for reporting! I can see the issue and your solution makes sense.

I have one question, though. It looks like the removedChildren variable goes out of scope at the end of the function and I wondering if the ZoteroItem might get de-allocated (not sure how JavaScript handles this). I wonder if it's safer to keep track of what keys to delete and only delete them after the loop is finished, something like:

const removedChildren = new Set();
for (...) {
   removedChildren.add(item.key)
}
// remove now that loop is done

[rostunic]

That is a good question, but I think we don't have to worry about that. The ZoteroItem has to be part of removedChildren because it is later used by its children to add them to the parent.children property. I thought about that the list would be garbage collected, but considered that as wanted behavior. The JS garbage collection algorithm works by marking all objects that are still reachable and removes the rest. The ZoteroItem is only added to the removedChildren list, if it was added to the children list of its parent. So it is still reachable and should not be garbage collected.At least if they stay referentially equal and I haven't missed something abot the garbage collection.

It would be indeed problematic if we removed the roots of the trees from the map.items list, because then the whole tree would be garbage collected. But since we only remove an item if it is a child of another item, we should not be removing any roots.

[frthjf]

That makes sense, thanks for double checking!

[frthjf]

This should now be available in v1.3.5. Thank you!

[rostunic]

Perfect thank you. And the most important part, all annotations are available.