Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Why don't update position of Model in collection when using collection.update? #2068

Closed
shadel opened this Issue · 15 comments

6 participants

@shadel

Ex:
old collection:
collection.models: [{id:2},{id:4}, {id:5}]

collection.update([{id:1}, {id:2}, {id: 3}, {id: 5}]);

new collection:
collection.models: [{id:2}, {id:5},{id:1}, {id: 3}]

i want :
collection.models: [{id:1}, {id:2}, {id: 3}, {id: 5}]
why not do it?

@tgriesser
Collaborator

Short answer - use collection.reset() rather than collection.update() for this case, or define a comparator on the collection to keep the models sorted by id.

Longer answer is pinging @braddunbar and @caseywebdev on this one to have a little more discussion on the dueling update and reset api's.

@caseywebdev
Collaborator

If you want to keep a collection sorted a certain way, define a comparator.

In your case it's as easy as

comparator: 'id'

Regarding reset vs update, I would cut reset as update can do everything reset can and more, though it is slower at dropping models.

@tgriesser
Collaborator

@caseywebdev and it doesn't fire add and remove events, which is often desirable.

@caseywebdev
Collaborator

@tgriesser A little confused, are you saying add and remove events are or aren't desirable?

@tgriesser
Collaborator

Yeah, could have worded that much better :) - I was trying to say that the add/remove are not always desirable.

@caseywebdev
Collaborator

I feel like if you're completely replacing models in a collection and disregarding previous models you might as well create a new collection instance. Other than that I see add and remove events as helpful in determining how the collection has been altered.

Or leave the existing update and reset as is so they can serve their own distinct purposes.

@philfreo

I can see how you might want a smartly updating collection to give reorder events along with add and remove, upon fetch.

Imagine a sortable list that gets auto-refreshed from the server by polling, like a collaborative todo list. The add/remove/edit part of this is already easy to do with fetch/update and change events (I do this frequently) - the only thing missing is reordering. You don't want to recreate the DOM elements (users may be editing items in the list) which is what reset does right now. So you'd either want a reset event that uses the same/previous model instances, or you'd want add/remove/reorder.

@caseywebdev
Collaborator

@philfreo sort is called after adds in update if the collection was sorted

@philfreo

Ah true, forgot about the sort event. But you might want to base your collection order off of the order of the items in a server's response. Or even if you can sort it client-side, would it automatically sort after update, if you wanted it to sort on some attribute like last_modified, which might have changed after a fetch?

@braddunbar
Collaborator

@philfreo The "sort" event is new as of 0.9.9.

@caseywebdev In this case, I think that a comparator is likely the answer, but reset certainly shouldn't be replaced with new Collection(...). There is no reason to rebind events and recreate references just to replace the current set of models with another.

@caseywebdev
Collaborator

@braddunbar I agree I think they both have their place and should stay.

@shadel

If use "reset" function, models will be removed, and i can use "add" function to add models. But i want use "update" function, because my models changed more than models added.
And i can't use "sort" function, because my models sorted at server.

Why not sort new collection same models added ???

@caseywebdev caseywebdev referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@caseywebdev
Collaborator

@philfreo

Or even if you can sort it client-side, would it automatically sort after update, if you wanted it to sort on some attribute like last_modified, which might have changed after a fetch?

Yup, that use case has been accounted for in master, try it out!

@shadel I've created a pull request at #2085 that would allow you to pass the {reorder: true} option to update, yielding the result you were looking for.

@caseywebdev caseywebdev referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@caseywebdev caseywebdev referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@caseywebdev caseywebdev referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@caseywebdev caseywebdev referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@caseywebdev caseywebdev referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@caseywebdev caseywebdev referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@caseywebdev caseywebdev referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@caseywebdev caseywebdev referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@caseywebdev caseywebdev referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@caseywebdev caseywebdev referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@caseywebdev caseywebdev referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@caseywebdev caseywebdev referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@caseywebdev caseywebdev referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@caseywebdev caseywebdev referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@caseywebdev caseywebdev referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@caseywebdev
Collaborator

Moving the discussion to the pull request #2085

@caseywebdev caseywebdev referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@mateusmaso

+1 for that, I want a smart update/sort from server side response when I already have a collection models.. just as @philfreo said, render all over again the DOM is wasteful and the update should behave just as reset but smart.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.