AmpersandJS / ampersand-state

Core state management object.
MIT License
141 stars 75 forks source link

Iterating objects with _.each is unsafe #20

Closed latentflip closed 10 years ago

latentflip commented 10 years ago

If the object being iterated can have a .length property, like attributes, .each will break. We should change to iterate over .keys explicitly

kamilogorek commented 10 years ago

Are you talking about every available object that's being iterated? or only for a specific methods?

this._children
this._collection
this.attributes
this._derived
def.dataTypes
def.props
def.session
def.derived
def.collections
def.children
latentflip commented 10 years ago

Oops I threw this in to remind me about it but didn't explain the problem very well.

In the ampersand-state code somewhere (can't check right now from my phone) we iterate this.props with .each. .each iterates over objects yielding keys and values.

However, if your props has a 'length' property, that triggers _.each to iterate it like an array (yielding index and value) which breaks everything.

kamilogorek commented 10 years ago

You were right. It was breaking clear function :)