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

Cannot sort arrays #104

Open
dantman opened this issue Apr 15, 2017 · 4 comments
Open

Cannot sort arrays #104

dantman opened this issue Apr 15, 2017 · 4 comments

Comments

@dantman
Copy link

dantman commented Apr 15, 2017

Sorting arrays does not appear to be sorted.

> var Freezer = require('freezer-js')
> var f = new Freezer([3,2,1]);
> f.get().sort();
TypeError: Cannot assign to read only property '1' of object '[object Array]'
    at Array.sort (native)
    at repl:1:9
    at ContextifyScript.Script.runInThisContext (vm.js:23:33)
    at REPLServer.defaultEval (repl.js:339:29)
    at bound (domain.js:280:14)
    at REPLServer.runBound [as eval] (domain.js:293:12)
    at REPLServer.onLine (repl.js:536:10)
    at emitOne (events.js:101:20)
    at REPLServer.emit (events.js:191:7)
    at REPLServer.Interface._onLine (readline.js:241:10)

It would also be nice if var arr = (new Freezer([1,2,3]).get(); arr.sort() === arr; and didn't result in an update (same order).

Might as well also support a sortBy since 3rd party implementations like lodash's probably can't be used with the immutable code.

@arqex
Copy link
Owner

arqex commented Apr 17, 2017

Hey @dantman, thanks for the suggestion. It fails because can't sort an immutable array, but maybe the sort method for arrays is something that can be added to the next version.

In the meanwhile you can try the long way,

var sorted = f.get().toJS();
sorted.sort();
f.set( sorted );

@dantman
Copy link
Author

dantman commented Apr 18, 2017

It's a bit more complex but in the meantime my workaround actually is:

items = items.splice(0, items.length, ...sortBy(items.slice(), sortFunction));

.slice() just the array to a mutable array, sort, and re-integrate into the immutable array by splicing the entire immutable array with the new order of values.

And for now to get arr.sort() === arr I'm just keeping track of whether I probably need to do any sorting or not (in my case did any items get added or have any of the order properties changed).

@arqex
Copy link
Owner

arqex commented Apr 18, 2017

I am afraid that you will never get arr.sort() === arr working with freezer. By convention all the update operations return the new updated version of the object. So arr.sort() would return a new array, result of sorting the previous one.

@dantman
Copy link
Author

dantman commented Apr 18, 2017

@arqex I'm talking about the scenario where your array is [1,2,3] and so .sort() does not change the order of items in the array. .set behaves this way, returning the original immutable object instead of a new one when no modification is done.

> var a = new Freezer({a:1}); a.get() === a.get().set({a:1});
true

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

No branches or pull requests

2 participants