add failing test case for #111 #112
Conversation
smhg
changed the title
src/cortex.js
Outdated
| @@ -44,9 +44,29 @@ module.exports = (function() { | |||
| } | |||
| } | |||
|
|
|||
| __remove(path) { | |||
| for(var i = 0, ii = this.__diffs.length; i < ii; i++) { | |||
There was a problem hiding this comment.
I'm not sure I follow the logic here. It's a little inefficient with O(n^2). It seems the simplest solution is to just compare the initial and final state. If they're the same the rollback/cancel the update
There was a problem hiding this comment.
You mean loop once through all queued diffs in this._diffs comparing them to this.__value?
If so, you mean here inside the __remove() event handler, right?
There was a problem hiding this comment.
Instead of using diffs, it’s probably better to let the update runs its course and as a final check perform a deep comparison between final and initial values (they’re immutable so already separate objects). If they’re the same then don’t run the callbacks.
smhg
force-pushed
the
issue-revert-change
branch
from
23268ac to
e80e805
Compare
smhg
changed the title
|
I've removed the possible fix from this PR as it took the wrong route (see #111). |
Similar to the example in #111.
Maybe it is nice to also test whether the the original value and the final value don't refer to the same object? To make sure the set happened (since the value was actually changed more or less).