Skip to content

DefaultIterableDiffer should keep the order of duplicates #23815

@csr632

Description

@csr632

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

https://stackblitz.com/edit/iterablediffers-stability

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:

Metadata

Metadata

Assignees

No one assigned

    Labels

    P3An issue that is relevant to core functions, but does not impede progress. Important, but not urgentarea: coreIssues related to the framework runtimecore: differsfreq2: mediumstate: has PRtype: bug/fix

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions