fix issues 1980 and 2003

[haimrait]

Fix issues 1980 and 2003

[danielkcz]

Thanks! Can we perhaps add one more test here for the case @urugator mentioned? Just for sake of everyone :) Make it in a separate commit and I will cherry-pick it to master also.

[danielkcz]

I am thinking about reverting the merged V5 PR so it's not accidentally published and breaks things. At least until a solution is discovered. Or can you @haimrait prepare a PR with added test and fix?

[urugator]

Good job, if you could add one more test it would be excellent:

test("#1980 .replace should not report changed unnecessarily", () => {
    const mapArray = [["swappedA", "swappedA"], ["swappedB", "swappedB"], ["removed", "removed"]];
    const replacementArray = [mapArray[1], mapArray[0], ["added", "added"]];   
    const map = observable.map(mapArray)        
    let autorunInvocationCount = 0;
    autorun(() => {
      map.get("swappedA");
      map.get("swappedB");
      autorunInvocationCount++;
    });    
    map.replace(replacementArray);
    expect(Array.from(map.entries())).toEqual(replacementArray);
    expect(autorunInvocationCount).toBe(1);
})

[urugator]

@mweststrate When/if you will have a time, could you take a look whether there is a particular reason for using observable keys array rather than keysAtom in v4?

[mweststrate]

From the top of my head there isn't, I think this just evolved over time and the implications could be the same in this regard. If making it the same still passes the tests, feel free to go ahead 😊 Op za 3 aug. 2019 11:38 schreef urugator <notifications@github.com>:

…

@mweststrate <https://github.com/mweststrate> When/if you will have a time, could you take a look whether there is a particular reason for using observable keys array rather than keysAtom in v4? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2058?email_source=notifications&email_token=AAN4NBDHYOMZJLDCWF5VK6LQCVGZZA5CNFSM4IG47UOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3PLDMY#issuecomment-517910963>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAN4NBENYLKUUD3XOMMXGITQCVGZZANCNFSM4IG47UOA> .

[haimrait]

@urugator @FredyC can we merge this pr?

[danielkcz]

I did not give it a proper thought, but @urugator approved, so let's roll :)

@haimrait Can you please make a second PR for MobX 5 as well?

[haimrait]

sure