Allow EntityCollection events to be reentrant #3739
Conversation
The `EntityCollection.collectionChanged` event would have odd behavior if you modified an entity inside the collection from within a `collectionChanged` callback. The added/removed/changed entity would get appended to the current arrays being passed around and a new event would get raised immedediately which included duplicates of all current event parameters plus the new entity. At the same time, remaining handlers would ultimately end up seeing the second event but not the first. This change makes the code rentrant safe and causes all reentrant events to be queued and then properly raised at the end of the current event cycle.
| @@ -30,15 +30,27 @@ define([ | |||
| }; | |||
|
|
|||
| function fireChangedEvent(collection) { | |||
| if (collection._firing) { | |||
There was a problem hiding this comment.
_firing is never set to true, as far as I can tell. I think you must be missing some tests.
There was a problem hiding this comment.
Whoops. What's weird is the test I wrote fails in master and passes in this branch (doing the right thing as far as I can tell). But what I'm not catching is checking that the second event is not fired until the first even is finished, which might be hard to test. I'll look into it.
There was a problem hiding this comment.
Okay, this should be fixed. I could add a more elaborate test if you think it's worth it that uses multiple spies to make sure things are called in the expected order.
| collection._firing = true; | ||
| do { | ||
| collection._refire = false; | ||
| var addedArray = collection._addedEntities.values.slice(0); |
There was a problem hiding this comment.
these should use the local variables already present above, added, removed, etc.
Allow EntityCollection events to be reentrant
The
EntityCollection.collectionChangedevent would have odd behavior if you modified an entity inside the collection from within acollectionChangedcallback. The added/removed/changed entity would get appended to the current arrays being passed around and a new event would get raised immedediately which included duplicates of all current event parameters plus the new entity. At the same time, remaining handlers would ultimately end up seeing the second event but not the first.This change makes the code rentrant safe and causes all reentrant events to be queued and then properly raised at the end of the current event cycle.