Skip to content

Ignore order on unordered Immutable.js structures, fixes #4618#5501

Merged
cpojer merged 1 commit intojestjs:masterfrom
robinpokorny:ignore-order-on-unordered-immutable
Feb 9, 2018
Merged

Ignore order on unordered Immutable.js structures, fixes #4618#5501
cpojer merged 1 commit intojestjs:masterfrom
robinpokorny:ignore-order-on-unordered-immutable

Conversation

@robinpokorny
Copy link
Copy Markdown
Contributor

@robinpokorny robinpokorny commented Feb 9, 2018

Summary

Immutable.js structures, that do not maintain order, should be equal regardless of the insertion order. This follows how native Set and Map are compared.

As toEqual should provide deep equality, Immutable.is (or .equals on a structure) cannot be used.

Sentinels are used for identifying the structures, as we do in pretty-format.

Fixes #4618 (includes additional info and reasons)

Test plan

Added most used Immutable.js structures to unit tests: List, Map, OrdereMap, Set, OrderedSet.

.set(1, 'one'),
],
[
Immutable.Map({1: Immutable.Map({2: {a: 99}})}),
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is to show that deep equality works.

.set(2, 'two'),
],
[
Immutable.Map({1: Immutable.Map({2: {a: 99}})}),
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is to show that deep equality works.

@robinpokorny robinpokorny force-pushed the ignore-order-on-unordered-immutable branch 2 times, most recently from 8e161ac to 5a0a42c Compare February 9, 2018 08:32
@robinpokorny robinpokorny force-pushed the ignore-order-on-unordered-immutable branch from 5a0a42c to bc2ff08 Compare February 9, 2018 08:46
@robinpokorny
Copy link
Copy Markdown
Contributor Author

The CI fails during restoring cache because ‘Permission denied’. Does anybody know why this happens?

@cpojer cpojer merged commit 97f0308 into jestjs:master Feb 9, 2018
@cpojer
Copy link
Copy Markdown
Member

cpojer commented Feb 9, 2018

Thanks Robin! This looks good, I merged it but will monitor CI to see if it fails. This sometimes happens when a CI run is interrupted, pretty weird.

@github-actions
Copy link
Copy Markdown

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Immutable.Map comparison with not the same key order

3 participants