fix(core): properly update collection with repeated keys in @for#52697
fix(core): properly update collection with repeated keys in @for#52697pkozlowski-opensource wants to merge 1 commit intoangular:mainfrom
Conversation
This change fixes a bug in the new list reconcilation algorithm that could lead to an infinite loop in certain situations. More specifically, it adjusts the internal MultiMap implementation such that an entry returned from the .get call is the same entry (for an identical key) removed by the .delete call. The existing logic of the MultiMap was leading to a situation where one view was requested and attached to LContainer, but a very different view was removed from the MultiMap. This was leaving an attached LView in a collection that was supposed to hold only detached views. Closes angular#52524
| imports: [ChildCmp], | ||
| template: ` | ||
| @for(task of tasks; track task.id) { | ||
| <child-cmp/> |
There was a problem hiding this comment.
Is the child comp needed, instead of doing e.g. |{{ task.name }}|? We could then have an assertion that the textContent has been updated to reflect the reordering + deletion accurately, and I'd expect the additional component to be irrelevant for the test (the embedded views get out of sync, not the component view?)
There was a problem hiding this comment.
The child component is need to show that the "wrong" view is attached and corrupts LView / Container data structures. I couldn't reproduce the app freeze without a child component view.
We could then have an assertion that the textContent has been updated to reflect the reordering + deletion accurately
This would introduce an assumption about the order of views with duplicated keys and I would rather not do it at this point. Unless I'm misunderstanding the idea of an alternative test...
There was a problem hiding this comment.
Ah, interesting! I wouldn't have expected that.
My thinking behind the assertion was that it would verify that the list updates have been properly reflected in the DOM. The items with the same ID also have the same name, so they could be reordened at will without problem.
|
This PR was merged into the repository by commit ea8c9b6. |
) This change fixes a bug in the new list reconcilation algorithm that could lead to an infinite loop in certain situations. More specifically, it adjusts the internal MultiMap implementation such that an entry returned from the .get call is the same entry (for an identical key) removed by the .delete call. The existing logic of the MultiMap was leading to a situation where one view was requested and attached to LContainer, but a very different view was removed from the MultiMap. This was leaving an attached LView in a collection that was supposed to hold only detached views. Closes #52524 PR Close #52697
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
…ular#52697) This change fixes a bug in the new list reconcilation algorithm that could lead to an infinite loop in certain situations. More specifically, it adjusts the internal MultiMap implementation such that an entry returned from the .get call is the same entry (for an identical key) removed by the .delete call. The existing logic of the MultiMap was leading to a situation where one view was requested and attached to LContainer, but a very different view was removed from the MultiMap. This was leaving an attached LView in a collection that was supposed to hold only detached views. Closes angular#52524 PR Close angular#52697
This change fixes a bug in the new list reconcilation algorithm that could lead to an infinite loop in certain situations.
More specifically, it adjusts the internal MultiMap implementation such that an entry returned from the .get call is the same entry (for an identical key) removed by the .delete call.
The existing logic of the MultiMap was leading to a situation where one view was requested and attached to LContainer, but a very different view was removed from the MultiMap. This was leaving an attached LView in a collection that was supposed to hold only detached views.
Closes #52524