Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OrderedSet is changing item ordering (@4.0.0-rc.11+) #1663

Closed
pixmaster opened this issue Dec 12, 2018 · 6 comments
Closed

OrderedSet is changing item ordering (@4.0.0-rc.11+) #1663

pixmaster opened this issue Dec 12, 2018 · 6 comments
Labels
Milestone

Comments

@pixmaster
Copy link

const { OrderedSet, Map } = require('immutable@4.0.0-rc.11')
const i=[2];
OrderedSet([ Map({b:1, a:true}), Map({b:2,a:true}), Map({b:3,a:true}) ])
  .map((t) => {
  if (i.includes(t.get('b'))) return t.set('a', false);
  return t;
  })
  .toJS()

Result
image

@Soreine
Copy link

Soreine commented Jan 6, 2019

Your example looks like the expected behavior.
During the map, if you return a different value, then it will probably remove the old value and add the new value, effectively pushing it to the end of the new OrderedSet

@pixmaster
Copy link
Author

pixmaster commented Jan 8, 2019

@Soreine it only looks like, because I expect than OrderedSet don't change order in any cases. What the sense have type is OrderedSet if that changes order? Especially till 4.0.0-rc.11 it worked as I expected. In our project we updated Immutable package and face this issue. I think it isn't as most of dev expected.

@B-R-Bender
Copy link

B-R-Bender commented Jul 10, 2019

same with me, I've expected order to be persist
is there any workaround you end up with? @pixmaster @Soreine

@B-R-Bender
Copy link

my workarount is to converto to List first and after updates is done convert it back to OrderedSet

return lineData.toList().map(dataItem => dataItem.update(
                            Y_VALUE,
                            value => value + prevLineData.getIn([dataItem.get(X_VALUE), Y_VALUE])
                        )).toOrderedSet();

@jdeniau
Copy link
Member

jdeniau commented Jun 29, 2021

Thank you for your bug report. The immutable-js oss fork will soon be merged and a fix for this issue has been included in #1833. Once this PR is merged, this issue will be resolved in the main branch. We will then do our best to to release the 4.0.0 version.

Commit reference: 37c34ec

@jdeniau
Copy link
Member

jdeniau commented Jul 8, 2021

Hi!
The 4.0.0-rc.14 version has been released and should fix this issue.
Feel free to test it and re-open this issue if the issue is still present.
Thank you!

@jdeniau jdeniau closed this as completed Jul 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants