mikeric / rivets

Lightweight and powerful data binding.
http://rivetsjs.com
MIT License
3.23k stars 310 forks source link

rv-each not updating after changes #685

Open dswitzer opened 8 years ago

dswitzer commented 8 years ago

I'm having an issue where the rv-each method is not updating DOM elements if the collection I'm using has changed. I'm using Backbone as the source for my models and collections. My expectation is that if the object passed to the {{rv-each}} binder changes, then the DOM should reflect the change.

I've put together an example that shows off the issue:

https://jsfiddle.net/dswitzer/10grn2d1/3/

When you click the "Change List" button, you'll see that 2 items get added, but the original 3 items are not updated. I've tracked the problem down to the code around the this.iterated[index].models[modelName] !== model line. This calls the view's update method (i.e. this.iterated[index].update(data)). The problem is because the view's bindings have no update() defined, the DOM isn't updated.

I've found if I change the code from:

        } else if (this.iterated[index].models[modelName] !== model) {
          this.iterated[index].update(data);
        }

to:

        } else if (this.iterated[index].models[modelName] !== model) {
          this.iterated[index].unbind();
          this.iterated[index].update(data);
          this.iterated[index].sync(); // this is needed to make sure "if" binder logic is re-applied
          this.iterated[index].bind();
        }

It fixes the problem. However, I'm not sure if the view should be unbound and bound again.

What am I doing wrong?

ghost commented 8 years ago

I've also been struggling with this exact problem, while trying to incorporate rivets into my project.

codeMonkeyAndy commented 8 years ago

Hi all, I'm battling with this as well, so far I've found that during a View.sync operation, the child Views are not being iterated through, I've hacked in a small change that appears to allow this behavior, let me know if it helps at all :)

View.prototype.sync = function () { var binding, _i, _j, _len, _ref1; _ref1 = this.bindings; for (_i = 0, _len = _ref1.length; _i < _len; _i++) { binding = _ref1[_i]; if (typeof binding.sync === "function") { binding.sync(); } // AR: child views will now be synced if the parent is synced if ((binding.iterated !== undefined) && (binding.iterated.length > 0)) { for (_j = 0; _j < binding.iterated.length; ++_j) { binding.iterated[_j].sync(); } } } };

dswitzer commented 7 years ago

I updated my example. Turns out I need to run trigger the {{sync()}} method in some cases. This happens when I have an {{rv-if}} binder attach.

See the following example:

https://jsfiddle.net/dswitzer/10grn2d1/4/

Run the example, then remove the comment for the {{fixBinder()}} at the end of the script and run the example again.

You'll see that it works correctly.

If you remove the {{sync()}} event, the text bindings get updated correctly, but the "asterisks" does not get updated.

dswitzer commented 7 years ago

I was able to spend a little more time on this today.

Since this problem only seemed to affect Backbone collections, I wanted to dig into the adapter to see if it could be resolved by changing the adapter.

I was able to fix the screen synchronization issues, by changing the adapter I'm using:

https://jsfiddle.net/dswitzer/10grn2d1/6/

The trick was to apply the default adapter when the object passed into the observer isn't a Backbone Model or Collection object.

What appears to be the root cause of the issue is that when observing a Collection as an array, it wasn't detecting that a model in a array position has changed, so it's not firing off the necessary events to trigger that the model changed.

My fix ultimately ends up sending the Backbone's Collection model property to the default observer, so that if the array changes it triggers updates.

This seems to be working.

blikblum commented 7 years ago

FYI: i fixed this bug in my fork: https://github.com/blikblum/rivets/tree/svelte

It was the same bug as #515

See https://jsfiddle.net/10grn2d1/8/