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

Preserve hasmany order when pushing child #5806

Closed
wants to merge 6 commits into from

Conversation

BryanCrotaz
Copy link
Contributor

@BryanCrotaz BryanCrotaz commented Dec 22, 2018

In my app I have a websocket receiving jsonapi updates from the server. I use store.push to put these into ember-data.

If I push a model which is part of an ordered hasMany, it gets bumped to the end of the list.

I can't reproduce this in a test, although I've tried to in this PR. My fix, however, works in the app. It is also a nice optimisation when pushing a hasMany with very few changes. Previously every item was removed and then added again. I skip that if the item in the old and new array matches at this index.

@BryanCrotaz BryanCrotaz changed the title preserve hasmany order when pushing child WIP preserve hasmany order when pushing child Jan 5, 2019
@BryanCrotaz
Copy link
Contributor Author

@runspired do you know why this PR is failing with the good old runloop autorun in testing error but only in LTS 2.18?

@BryanCrotaz BryanCrotaz changed the title WIP preserve hasmany order when pushing child Preserve hasmany order when pushing child Jan 5, 2019
@BryanCrotaz
Copy link
Contributor Author

@runspired this is an important fix for us and has left us marooned on 3.6. Do you have a moment to see why it's only failing on the old LTS?

@BryanCrotaz
Copy link
Contributor Author

closed in favour of #7402

@runspired
Copy link
Contributor

Closing since stale (and rebooted in #7402), likely mostly addressed with #7493

@runspired runspired closed this May 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants