Skip to content
This repository was archived by the owner on Jul 23, 2021. It is now read-only.

Fix ordered set with map #206

Merged

Conversation

jdeniau
Copy link

@jdeniau jdeniau commented Nov 25, 2020

Fixes #115

This issue has been introduced by immutable-js#1606 : to keep the reference equality, it did remove and add all values that changes.

I tried several implementations to fix this, and finally used the other approach that @jessezhang91 dropped for a really small performance reason : replace the .map by a .mapEntries and keep track if at least one value did change.

// keep track if the set is altered by the map function
let didChanges = false;

const newMap = updateSet(
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reused updateSet here as it was the base implementation (see here for the original implementation)

Copy link

@Methuselah96 Methuselah96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I would be interested in seeing if we could get this working using withMutations (would probably have to change Map#mapEntries), but this looks like it will work as well.

@Methuselah96 Methuselah96 merged commit 37c34ec into immutable-js-oss:master Dec 8, 2020
@jdeniau jdeniau deleted the fix-ordered-set-with-map branch December 8, 2020 14:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OrderedSet is changing item ordering (@4.0.0-rc.11+)
2 participants