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

Enforce immutabillity #546

Closed
sporto opened this issue Jul 13, 2015 · 7 comments
Closed

Enforce immutabillity #546

sporto opened this issue Jul 13, 2015 · 7 comments

Comments

@sporto
Copy link

sporto commented Jul 13, 2015

At the moment it is quite easy to push mutable objects into immutable structures, e.g.

var record1 = {items: [1, 2]};
var list = Immutable.fromJS([record1]);

// but it doesn't complain if I push a mutable record by mistake
var record2 = {items: [3]}
list = list.push(record2)

// now we have a mix of mutable and immutable things
record2.items.push(4);
console.log(list.toJS())

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

I would expect something like this to either: throw an error or make the pushed object immutable automatically. It would be great if Immutable.js could provide stronger immutable guarantees.

@leebyron
Copy link
Collaborator

This is a common request, so I'll try to figure out a way to assemble an API to do this. Perhaps StrictMap and StrictList or something like that.

Doing so comes with the direct tradeoff that immutable collection of pointers to mutable values would not be possible. Definitely not what we want for the only behavior as this sort of thing is pretty common, but I also understand the desire to enforce strictness and throw errors if trying to put a non-immutable value into an Immutable.js collection.

@DaleLJefferson
Copy link

This feature would be great, i've seen quite a few people struggle with this. Once people get the idea of immutable js they expect everything you get out of a immutable data structure to be immutable.

var aMap = Immutable.Map({aList: [1, 2]});
var newList = aMap.get("aList").push(3);

// User expects newList to be contain [1, 2, 3]
console.log(newList); // 3

I'm not sure adding a whole load more types is the best way to do this.

  • List
  • Stack
  • Map
  • OrderedMap
  • Set
  • OrderedSet
  • StrictList
  • StrictStack
  • StrictMap
  • StrictOrderedMap
  • StrictSet
  • StrictOrderedSet

How about

Map<K, V>(iterable: Object, options: Object): Map<K, V>

Map({}, {strict: true});
Map({items: [1, 2]}, {strict: true, deep: true});

Strict throws an exception if you attempt to add a non immutable object and deep does what fromJS does.

@jareware
Copy link

#473 seems related, btw.

@leebyron
Copy link
Collaborator

Thanks, I'll keep considering ways to do this nicely. It's a careful line to walk between supporting generic collections and supporting deep values.

@AsaAyers
Copy link

With as useful as it is to .map() over something to produce potentially mutable objects, I don't think it's a good idea to have strict structures that enforce immutability. When rendering Immutable structures into React Components you'll end up having to convert your strict collection to a non-strict collection just so you can .map() over it normally. I'm using this function I built: assertImmutable.

prefix is to give you something to identify the root object. assertImmutable(userMap, 'User Store') might tell me that User Store. Array or Object found at: aayers.permissions.3

oldObj is an optional optimization. If your object has a small number of shallow changes it's significantly faster to include oldObj. However, if your tree is very large and contains significant changes it can actually be slower than not passing it at all.

function visit(obj, prefix = '', oldObj = undefined, path=[]) {
    for (let key of obj.keys()) { // eslint-disable-line prefer-const
        const value = obj.get(key)

        const oldValue = (oldObj == null || oldObj.get == null) ? null : oldObj.get(key)

        // There's no need to check branches that haven't changed.
        if (!Immutable.is(oldValue, value)) {

            if (value instanceof Collection) {
                const currentPath = path.concat([key])
                visit(value, prefix, oldValue, currentPath)
            } else if (value instanceof Error) { //eslint-disable-line no-empty
                // Errors don't really change. I guess they're ok
            } else if (value instanceof Array || value instanceof Object) {
                const currentPath = path.concat([key])

                throw new Error(prefix + 'Array or Object found at: ' + currentPath.join('.'))
            }
        }
    }
}

function assertImmutable(obj, prefix, oldObj = undefined) {
    if (prefix) {
        prefix = prefix.trim() + '. '
    }

    visit(obj, prefix, oldObj)
}

@sporto
Copy link
Author

sporto commented Aug 19, 2015

@AsaAyers I'll be happy to explicitly convert strict collections to non strict or mutable collections when I need to. For me strong immutable guarantees are more important than speed. Anyway, it would be great to have this choice.

@leebyron
Copy link
Collaborator

leebyron commented Mar 8, 2017

Closing - see #473

@leebyron leebyron closed this as completed Mar 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants