omniscientjs / immstruct

Immutable data structures with history for top-to-bottom properties in component based libraries like React. Based on Immutable.js
374 stars 21 forks source link

[question] immstruct version of this immutable code #77

Closed kswope closed 9 years ago

kswope commented 9 years ago

Just messing around and trying to figure out the proper way to build up a data structure in immstruct. First here is the immutable.js version of what I want to do.

var Immutable = require('immutable');                                                                                                             
var map = Immutable.fromJS({a:undefined});                                                                                                        
map = map.updateIn( [ 'a' ], () => Immutable.Map( { b: undefined } ) )                                                                            
map = map.updateIn( [ 'a', 'b' ], () => Immutable.Map( { c: 'here' } ) )                                                                          
console.log(map.getIn(['a','b','c'])) //=> 'here 

Here is the immstruct version. It works but what bothers me is that is uses Immutable.Map(). Is there a strictly immstruct way to do this?

var immstruct = require('immstruct');                                                                                                             
var structure = immstruct({a:undefined});                                                                                                         
var cursor = structure.cursor( [ 'a' ] ).update( () => Immutable.Map( { b: undefined } ) )                                                        
var cursor = structure.cursor( [ 'a', 'b' ] ).update( () => Immutable.Map( { c: 'there' } ) )                                                     
console.log(structure.cursor(['a','b', 'c']).deref()) //=> 'there' 
dashed commented 9 years ago

you can do this in one line: structure.cursor(['a','b', 'c']).update(() => 'there')

you can have cursors at paths whose keys do not exist yet.

according to docs (http://facebook.github.io/immutable-js/docs/#/Map/updateIn):

If any keys in keyPath do not exist, new Immutable Maps will be created at those keys.

dashed commented 9 years ago

To elaborate, Map.updateIn() is implicitly used in Cursor.update(): https://github.com/facebook/immutable-js/blob/b12e43aceb1e4e402931079406b7cc095d307a25/contrib/cursor/index.js#L142

kswope commented 9 years ago

Thats interesting I didn't realize that. But I think my question still stands. I looks like to insert a hash into a path and then have it itself searchable in a path you need to insert a Immutable'ized hash otherwise the searchable path just stops ( its not easy explaining that, hope it made sense ). I assume there's a reason immstruct (and immutable.js) don't just see a hash and Immutable'ize it automatically. I hope that didn't sound too ignorant, lol.

dashed commented 9 years ago

Sorry. I don't quite follow what you meant by "insert a hash".

kswope commented 9 years ago

Changing one line below breaks this code, the 'path' doesnt work anymore

var Immutable = require('immutable');                                                                                                             
var map = Immutable.fromJS({a:undefined});                                                                                                        
map = map.updateIn( [ 'a' ], () => { b: undefined }  ) // <---- CHANGED                                                                            
map = map.updateIn( [ 'a', 'b' ], () => Immutable.Map( { c: 'here' } ) )                                                                          
console.log(map.getIn(['a','b','c'])) //=> 'here 
kswope commented 9 years ago

So my point is you have to go 'outside' immstruct and use Immutable.map() to achieve what I did above. I was just wondering if there was a solution built into immstruct

dashed commented 9 years ago

Let's frame it this way with 100% dependency on Immutable lib and no dependency on immstruct:

const Immutable = require('immutable');
const Cursor = require('immutable/contrib/cursor');

// reduced API of immstruct
function immstruct(rawData) {

    let data = Immutable.fromJS(rawData);

    function onChange(newData) {
        data = newData;
    }

    return {
        cursor: function(path) {
            path = path || [];
            return Cursor.from(data, path, onChange);
        }
    };
}

const structure = immstruct({ a: undefined });

const cursor1 = structure.cursor( [ 'a' ] ).update(() => Immutable.Map( { b: undefined } ));
const cursor2 = structure.cursor( [ 'a', 'b' ] ).update( () => Immutable.Map( { c: 'there' } ) );

console.log(structure.cursor(['a','b', 'c']).deref()); // => there

It should be clear that an external dependency on Immutable is required.

dashed commented 9 years ago

Also, structure.cursor(path) returns a Cursor object whose API is different from immstruct's. See: https://github.com/facebook/immutable-js/tree/master/contrib/cursor

There's really no way around to avoid using Immutable.Map to creating nested maps via immstruct.

kswope commented 9 years ago

I'm starting to think this issue doesn't come up often.

dashed commented 9 years ago

If you're worried about a map that doesn't exist at ['a','b']. You can do something like this:

const NOT_SET = {};

const val = structure.cursor(['a','b', 'c']).deref(NOT_SET);

if(val === NOT_SET) {
    // value not set at ['a','b', 'c']
} else {
    // value set at ['a','b', 'c']
}
mikaelbr commented 9 years ago

Changing one line below breaks this code, the 'path' doesnt work anymore

var Immutable = require('immutable');                                                                                                             
var map = Immutable.fromJS({a:undefined});                                                                                                        
map = map.updateIn( [ 'a' ], () => { b: undefined }  ) // <---- CHANGED                                                                            
map = map.updateIn( [ 'a', 'b' ], () => Immutable.Map( { c: 'here' } ) )                                                                          
console.log(map.getIn(['a','b','c'])) //=> 'here 

I see what you mean. It might be odd, at least the first time, you have to pass in a Immutable.js construct to something that looks like it's a immstruct construct, but in fact the cursors and structures generated by immstruct is Immutable.js. We could expose the Immutable library, but I think it's better to be explicit about what you do. And having to maintain the API of another library would be an unwieldy task.

It also migth seem odd that you have to remember to make the structures you update a immutable object with, into immutable objects. But this is as Immutable objects aren't necessarily restricted to storing data structures. You can also have "complex" objects like event emitters, functions, instances etc. And by automatically converting all input in methods like .update or .set to Immutable data, you would have a hard time separating these type of objects from each other.

I'm afraid there's no good way around the tedious, manual, operation here.

kswope commented 9 years ago

And by automatically converting all input in methods like .update or .set to Immutable data, you would have a hard time separating these type of objects from each other.

That's exactly where I went wrong! I was flailing for hours because of that assumption. Is anybody actually sticking functions or event emitters in there or was it just a wild assumption from the start? At least with the use-case of omniscient I'd think its all about hashes, arrays, but I'm just a beginner.

mikaelbr commented 9 years ago

In the case of Omniscient, you wouldn't probably do that - but again, in some cases you might use immutable data for all of your props, and use an event emitter to "talk back to parents". But Immstruct doesn't make any assumption about your use case, and isn't only a Omniscient.js library. There are several people using immstruct in a non-Omniscient way, or even without component trees.

Also as it's just Immutable.js objects, they behave in the exact same way as Immutable.js objects. immstruct doesn't try to alter the behaviour of Immutable.js objects, just allow for a different way of creating them.

kswope commented 9 years ago

How about this suggestion which is somewhat consistent with immutable.js. I guess it would also need a setFromJS(). I think they both would just be wrappers for Immutable.fromJS(). It's just cheese but it would also illustrative and informative if found in the API lineup.

var immstruct = require('immstruct');                                                                                                             
var structure = immstruct({a:undefined});                                                                                                         
var cursor = structure.cursor( [ 'a' ] ).updateFromJS( { b: undefined } )  <--- NEW 
var cursor = structure.cursor( [ 'a', 'b' ] ).updateFromJS( { c: 'there' } ) <--- NEW
console.log(structure.cursor(['a','b', 'c']).deref()) //=> 'there' 
mikaelbr commented 9 years ago

The result of structure.cursor( [ 'a' ] ) is a Immutable.js construct. So the API from that cursor is in its entirely consistent with Immutable.js. I don't think wrapping cursors or immutable.js objects to provide a different API would be the best thing to do.

This is the same behaviour you'd expect from Immutable.js objects not created from immstruct as well.

var f = Immutable.fromJS({ foo: { bar: 'hello' } });

f.updateIn(['foo'], function () { return { b: 'bye' } }).get('foo');
//=> Object {b: "bye"}
f.updateIn(['foo'], function () { return Immutable.fromJS({ b: 'bye' }) }).get('foo');
//=> Object {size: 1, _root: We, __ownerID: undefined, __hash: undefined, __altered: false}