search

jashkenas / backbone

Give your JS App some Backbone with Models, Views, Collections, and Events
http://backbonejs.org
MIT License
28.09k stars 5.38k forks source link

Collection.comparator fails on IE #1328

Closed vkovalskiy closed 12 years ago

vkovalskiy commented 12 years ago

Environment: IE8

When creating a comparator function and returning string value from it IE throws exception saying: Number Expected. The exception point shown by IE is backbone.js:723

https://github.com/documentcloud/backbone/blob/master/backbone.js#L723

returning int from comparator makes IE happy

braddunbar commented 12 years ago

Hi @vkovalskiy, thanks for the issue! That sounds about right. How many arguments are used in the declaration of your sort function? comparator was recently changed to use [].sort directly instead of _.sortBy. when declared with two parameters.

If you're interested, there is a discussion regarding this change going on in #1301 as well.

vkovalskiy commented 12 years ago

Thanks for info @braddunbar!

I am using one argument. The recent changes you mention are not in the 0.9.2 release, right? I'll monitor #1301 for the resolutions.

I suppose that one-argument sorting comparator by string should be available. As you have pointed out it is very convenient to sort something based on style, kind, etc.

braddunbar commented 12 years ago

That's definitely interesting. If you can work up a reproducible test case and post to jsfiddle or similar I would love to see it.

wookiehangover commented 12 years ago

I went ahead and made a test case for this (jsfiddles: v0.9.2, HEAD) but I haven't gotten it to fail in either of my IE8 environments (win7 and xp). @vkovalskiy could you take a look and see if you can get a reproducible error?

vkovalskiy commented 12 years ago

@wookiehangover thanks! I do not have Windows at hands now so I'll check it out tomorrow and get back with results.

braddunbar commented 12 years ago

I'm going to close this one as "not reproducible" for the time being, but please comment here if you can reproduce this again and I'll reopen it.

rsirotins commented 12 years ago

Stumbled upon the same issue. It is actually 743 line. It uses JS sort instead of SortBy because comparator function has no arguments which I don't understand. Can't think of a decent workaround aswell. If using sortby it works fine. JS doesn't work probably because of IE8 Array sortings bugs: http://www.zachleat.com/web/array-sort/

rsirotins commented 12 years ago

This problem was because "comparator" function was binded to model, removed binding and it works OK.

vkovalskiy commented 12 years ago

@rsirotins coould you please elaborate more on how you have fixed the problem? thanks in advance.

gucki commented 11 years ago

@vkovalskiy Just stumbled upon the same error. You have to use -> instead of => for the method defintion, then it works.

vkovalskiy commented 11 years ago

@gucki wow, thanks!