Code review comment for lp://staging/~gary/juju-gui/fixDeltaFromChangeEdgeCase

Revision history for this message
Gary Poster (gary) wrote :

Line-by-line comments for reviewers.

https://codereview.appspot.com/13348056/diff/1/app/views/databinding.js
File app/views/databinding.js (right):

https://codereview.appspot.com/13348056/diff/1/app/views/databinding.js#newcode186
app/views/databinding.js:186: var index = indexBindings(bindings, null,
true);
The "true" argument and then the "result.bindings.push.apply" below is
the real fix. All the other changes streamline the logic, resulting in
much fewer duplicate results and hopefully clearer reading.

https://codereview.appspot.com/13348056/diff/1/app/views/databinding.js#newcode393
app/views/databinding.js:393: this._modelChangeHandler();
These were moved from _bind, mostly as an optimization for reading but
also to prevent unnecessary duplication of work.

https://codereview.appspot.com/13348056/diff/1/app/views/databinding.js#newcode533
app/views/databinding.js:533: index[dep] = source;
This is just an optimization, so that if we have more than one of these
implicit bindings on the same source, we don't create multiple copies of
the source. It wasn't causing a bug, but this just makes it easier to
debug further downstream.

https://codereview.appspot.com/13348056/

« Back to merge proposal