fix(core): make DefaultIterableDiffer keep the order of duplicates#23941
Closed
csr632 wants to merge 1 commit intoangular:masterfrom
Closed
fix(core): make DefaultIterableDiffer keep the order of duplicates#23941csr632 wants to merge 1 commit intoangular:masterfrom
csr632 wants to merge 1 commit intoangular:masterfrom
Conversation
csr632
force-pushed
the
fix-DefaultIterableDiffer-mismatch
branch
2 times, most recently
from
16b3875 to
6e1bac1
Compare
csr632
force-pushed
the
fix-DefaultIterableDiffer-mismatch
branch
from
6e1bac1 to
8284a0f
Compare
Contributor
Author
|
What do you guys think about this? |
Previously, in `_mismatch()`, the `DefaultIterableDiffer` first checks `_linkedRecords` for `itemTrackBy`, then checks `_unlinkedRecords`. This cause the `DefaultIterableDiffer` to move "later" items that match the `itemTrackBy` from the old collection, rather than using the "earlier" one. Now we check `_unlinkedRecords` first, so that the `DefaultIterableDiffer` can give a more stable and reasonable result after diffing. For example, rather than (`a1` and `a2` have same trackById) ``` a1 b c a2 => b a2 c a1 ``` we get ``` a1 b c a2 => b a1 c a2 ``` where a1 and a2 retain their original order despite both having the same track by value. Fixes angular#23815
petebacondarwin
force-pushed
the
fix-DefaultIterableDiffer-mismatch
branch
from
8284a0f to
9abb073
Compare
Contributor
|
I rebased to resolve the conflicts and tweaked the commit message slightly. |
petebacondarwin
requested review from
AndrewKushnir
and removed request for
alxhub
petebacondarwin
added
action: presubmit
petebacondarwin
added
the
target: minor
Contributor
mhevery
approved these changes
Jan 26, 2021
mhevery
added
action: merge
mhevery
removed
action: review
|
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, in _mismatch, DefaultIterableDiffer first check _linkedRecords for itemTrackBy, then check _unlinkedRecords.
This cause DefaultIterableDiffer match itemTrackBy with the "later" duplicate in the old collection, rather than the "preceding" one.
Now we check _unlinkedRecords first, so that DefaultIterableDiffer can give more simple and reasonable info after diffing:
a1 b c a2 => b a1 c a2 (a1 and a2 have same trackById)
See the new test case in default_iterable_differ_spec.ts and #23815.
Fix #23815
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #23815
What is the new behavior?
DefaultIterableDiffer can give more simple and reasonable info after diffing
a1 b c a2 => b a1 c a2 (a1 and a2 have same trackById). See #23815 .Does this PR introduce a breaking change?
Other information