arqex / freezer

A tree data structure that emits events on updates, even if the modification is triggered by one of the leaves, making it easier to think in a reactive way.
MIT License
1.28k stars 56 forks source link

Object <-> Array #1

Closed ArnoBuschmann closed 9 years ago

ArnoBuschmann commented 9 years ago

hi @arqex

I'm about to test a Baobab structure in Freezer and it seems I get an object, where it should be an array instead.

Given this structure:

mylayouts: {"lg":[{"w":8,"h":2,"x":2,"y":0,"i":"0"},{"w":2,"h":20,"x":0,"y":2,"i":"1"},{"w":8,"h":2,"x":2,"y":5,"i":"2"},{"w":2,"h":2,"x":0,"y":0,"i":"4"},{"w":8,"h":3,"x":2,"y":2,"i":"3"}]}

And setting the cursor in Baobab to:

layoutsCursor: ['mylayouts']
console.log( this.cursors.mylayouts.get() ) // => Object {lg: Array[5]}

In Freezer:

console.log( data.mylayouts ) // => Object {lg: Object[5], __: Object}

As data.mylayouts.lg is indeed an array, Freezer shouldn't make it an object, should it?

arqex commented 9 years ago

Ey Arno,

I was sure you'd be the first in pointing something out :)

Well, I really wanted arrays to be arrays in Freezer but I needed it to be fast and using common arrays as I did in Curxor wasn't the way.

I need arrays but I also need to override the push, pop, splice... methods to behave as immutable. In Curxor I instantiated an array for every node and then I add the methods one by one, and that made it slow, and, in a big tree, unsustainable.

So I need an Array "subclass" with all the methods already in its prototype to create the array objects. Just this change make Freezer 4x faster than Curxor. The problem is that in JS it is not possible to create a Subarray that inherits from Array and behaves like an actual Array object (see here).

If you try something like this you get an object.

subarr = Object.create( Array.prototype );

So arrays in Freezer are Array and Object at the same time, ( see the Object[5] output in your console). You can use it as if it was a real array, almost all Array methods are usable ( only concat is giving problems ) and also in your example:

// It's an instance of array
data.lg instanceof Array; // true
data.lg.constructor; // Array

But there some stuff you must know:

// It is not a real array
Array.isArray( data.lg ); // false
Object.prototype.toString( data.lg ); // [object Object]

// But it is easy to get one
Array.isArray( data.lg.slice() ); // true

// Concat method doesn't work as expected, maybe I override it in the future
data.lg.concat(3); // [Object[5], 3]

The most important thing to keep in mind with Freezer arrays is not to leave gaps on it and not to add non-numerical indexes to it, doing so would create an unexpected behaviour.

ArnoBuschmann commented 9 years ago

I was sure you'd be the first in pointing something out :)

Such an honor ;)

// It is not a real array Array.isArray( data.lg ); // false

Yes, while using a third party library it is doing type checking I guess, so I get this error:

layouts.lg must be an array!

When trying

layouts: {lg: data.mylayouts.lg.slice()}

I get:

TypeError: Cannot assign to read only property 'i' of #<Object>
arqex commented 9 years ago

Unless somebody brings an idea on how to create Array subclasses or add methods to the array itself quickly Array.isArray( data.lg ) won't recognize freezer arrays as arrays.

The second error seems not to be in the slice method, but in the assignation to an immutable object. That is the error you get when you try to add a new property to a immutable object.

If layouts is an immutable object you need to

layouts = layouts.set({lg: data.mylayouts.lg});

It is much better and faster adding a reference to the pseudo-array object than cloning it using slice, even if you add an plain, it will be converter to a freezer array-like object.

Here you have a jsbin to play with freezer, and see how slice is working http://jsbin.com/wavusonuda/3/edit?html,js,console

ArnoBuschmann commented 9 years ago

Yes, it's working that way. All in all: nice work! :) Closing this now.

arqex commented 9 years ago

Ey Arno, thanks for your feedback!

I have updated freezer and now you can use concat as usual

var store = new Freezer( [1,2,3] ).get();
store.concat( [4,5] ); // returns [1,2,3,4,5];

But unfortunatelly it is still impossible to concat a freezer array to an actual array:

[4,5].concat( store ); // [4,5, [Object object] ]

Not having real arrays will give me a lot of user complaints in the future :P

donaldpipowitch commented 8 years ago

Wow, this is a big gotcha! Is this still true? The Readme says Uses common JS array and objects to store the data, so you can use it with your favourite libs like lodash, underscore or ramda, but as soon as a library would check Array.isArray this wouldn't be true anymore right? Could you warn about this in the Readme? All I could found was Array nodes have modified versions of the push, pop, unshift, shift and splice methods which to me sounds more like you did some patching on the Array.prototype[methods].

arqex commented 8 years ago

Hi @donaldpipowitch

This is an old issue, freezer arrays are actual arrays for a long time now and Array.isArray or Array.prototype.concat are working as expected.

http://jsbin.com/jalereroho/edit?html,js,console

Keep in mind that Freezer stores immutable data so any modifier operations in freezer arrays won't update them:

var f = new Freezer([1,2,3]);
var arr = f.get();

arr.push(4);
console.log( arr ); // logs [1,2,3]

It is true than freezer's arrays have a modified version of those updated methods ( but not overriding Array.prototype as you commented, you can use common arrays and freezer together ), the modification is just to trigger an update in the Freezer object, so a new immutable data structure can be created with the update.

var f = new Freezer([1,2,3]);
var arr = f.get();

arr.push(4); // this will make f to update its data
console.log( arr ); // logs [1,2,3] still immutable
console.log( f.get() ); // logs [1,2,3,4]
donaldpipowitch commented 8 years ago

That sounds great. Thank you.