Test case for cid order when adding model hashes #2073

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@wyuenho
Contributor
wyuenho commented Jan 4, 2013

Collection#add in 0.9.9 inserts object hashes in reverse. This has the unintended consequence of creating cids on the vivified models in reverse order as well, which broke code that relies on the cid as the marker for model insertion order. This behavior is reversed back to 0.9.2 behavior on master. This test case aims to ensure this behavior persists.

Collaborator

Is this something we want to guarantee? It seems reasonable, but at the same time I feel like you should always treat arbitrary identifiers as just that, arbitrary. /cc @jashkenas @braddunbar @tgriesser

Collaborator

Agreed. cid is guaranteed to be unique, not sequential or increasing.

Contributor
wyuenho commented Jan 7, 2013

@braddunbar In general yes, but at the same time when you are adding an array object hashes, wouldn't having cids created being in the order of insertion make more sense as well?

Collaborator

@wyuenho I don't think so. Is there a use case in which you rely on that guarantee (sequential/increasing)?

Contributor
wyuenho commented Jan 7, 2013

https://github.com/wyuenho/backgrid/blob/master/src/header.js#L54

The gist:

In a grid header, I need to be able to cycle back to the original insertion order of the upon clicking the sort caret 3 times. Every grid is a self-contained instance and every subview knows nothing about its parent, so I can't just take a snapshot of the original insertion order once and put it somewhere where the individual header cells can have access to without doing some jujitsu to make a registry for each grid to remember just this map of position and model cid, or in this case, have a copy of the snapshot in every header cell.

The behavior in 0.9.2, tho not an optimal solution for this problem, it does take care of this use case most of the time.

Another use case, suppose I need to sort a list of collections by both id and cid at the same time (first by comparing left and right ids, if one id is missing on either side, compare with the cid), assuming they are both sequential at the time of creation, this guarantees newly created models, after sorting, will always be at the bottom in insertion order.

(id is a number type)

Collaborator

Client ids take the form: c1, c2, c3 ...

The docs do give the impression that cid is always sequential, but I think we should probably remove it. It's not even guaranteed to be that since views also use _.uniqueId.

_.uniqueId('c'); // c1
_.uniqueId('view'); // view2
_.uniqueId('c'); // c3

Given that cid is only guaranteed to be unique and not sequential or increasing, I would not recommend using it for implementing the functionality you are describing. If you need to, you can always create your own sort order in much the same manner.

@braddunbar braddunbar added a commit that closed this pull request Jan 7, 2013
@braddunbar braddunbar Fix #2073 - Remove cid description. 3817742
@braddunbar braddunbar closed this in 3817742 Jan 7, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment