Check for circular references in checkChanges #80
Conversation
|
Tests please! :-) |
| if(visited[obj]) { | ||
| return; | ||
| } else { | ||
| visited[obj] = true; |
There was a problem hiding this comment.
I don't think this will work, because Models (by default) will be serialized to "[object Object]". Might need to be more clever about how to index them.
There was a problem hiding this comment.
Good point, my brain was stuck in ruby land. Looks like Javascript has no concept of "object id" but it does support comparisons. Might just have to use an array and scan over it repeatedly...
There was a problem hiding this comment.
I'm fine with a less-than-optimal implementation to start... as long as there are tests. Also, because you're modifying the visited object in the function anyway, just define that variable outside of the closure (as opposed to passing it to each call of the method.
Backbone collections set up a circular reference: collection[i] is a reference to a model, and model.collection is a reference back to the collection. checkChanges needs to not run in circles when encountering this.
|
Revised to actually work, and added tests. Without the change, the test fails with a "Maximum call stack size exceeded" error. |
Check for circular references in checkChanges
Backbone collections set up a circular reference: collection[i] is a reference
to a model, and model.collection is a reference back to the collection.
checkChanges needs to not run in circles when encountering this.
Fixes #79