-
Notifications
You must be signed in to change notification settings - Fork 27.1k
DefaultIterableDiffer should keep the order of duplicates #23815
Description
I'm submitting a...
[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report
[ ] Performance issue
[x] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question
[ ] Other... Please describe:
Current behavior
When DefaultIterableDiffer is diffing(it updates _itHead list according to the new collection), it fail to keep the order of items that have same trackById.
For example:
a1 b c a2 => b a1 c a2 (a1 and a2 have same trackById)
intermediate state(show the changes of `_itHead` list, `_unlinkedRecords` shows in parentheses):
a1 b c a2
b c a2 (a1) // b move from index 1 to 0
b a2 (a1 c) // a2 move from index 3 to 1, and its identity change to a1
b a2 c (a1)
b a2 c a1 // a1 move from 0 to 3, and its identity change to a2
See stackblitz demo.
The identity change is not necessary. And the move of a1 and a2 is not so nice.
In _mismatch, DefaultIterableDiffer first check _linkedRecords for itemTrackBy, then check _unlinkedRecords. This cause DefaultIterableDiffer match itemTrackBy(a) with the "later" item(a2), rather than the preceding one(a1).
Why not check _unlinkedRecords first?
If we check _unlinkedRecords first, we can get this:
a1 b c a2 => b a1 c a2 (a1 and a2 have same trackById)
intermediate state:
a1 b c a2
b c a2 (a1) // b move from index 1 to 0
b a1 a2 (c) // a1 move from index 0 to 1. don't need identity change
b a1 c (a2)
b a1 c a2 // don't need to move a2. don't need identity change
Expected behavior
Only have to move b from 1 to 0 and move a1 from 0 to 1. No identity change necessary.
Minimal reproduction of the problem with instructions
What is the motivation / use case for changing the behavior?
The reason is similar to _verifyReinsertion. We should make the diff result as "reasonable" as we can.
Environment
Angular version: X.Y.Z
Browser:
- [x] Chrome (desktop) version XX
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX
For Tooling issues:
- Node version: XX
- Platform:
Others: