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

merge can create partial Immutables #10

Closed
raorao opened this issue Nov 6, 2014 · 1 comment · Fixed by #11
Closed

merge can create partial Immutables #10

raorao opened this issue Nov 6, 2014 · 1 comment · Fixed by #11
Labels

Comments

@raorao
Copy link
Contributor

raorao commented Nov 6, 2014

When merging an immutable object with a mutable object, the resulting immutable object can contain mutable objects, but still identifies as immutable. For example, here's current behavior:

obj = Immutable( { key: 'value', collection: [ 1,2,3 ] } )
Immutable.isImmutable( obj ) //true
Immutable.isImmutable( obj['collection'] ) //true

newObj = obj.merge( { collection: [ 1,2,3 ] } )
Immutable.isImmutable( newObj ) //true
Immutable.isImmutable( newObj[ 'collection' ] ) //false
Immutable.isImmutable( newObj[ 'collection' ].push ) //danger!

its unclear what expected behavior should be. Personally, I think .merge should deeply convert everything that gets passed in to be immutable, as these partially immutable objects can create strange bugs. But maybe we should give users more flexibility. We could just add an isDeeplyImmutable function, and let users deal with the consequences.

@rtfeldman
Copy link
Owner

Yep, it should be deeply immutable. This is a bug. The guarantees of immutability are important to maintain...I'll fix this in a jiffy. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants