Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating the model from within an rv-each doesn't update the DOM #700

Closed
maxmumford opened this issue Dec 7, 2016 · 10 comments
Closed

Updating the model from within an rv-each doesn't update the DOM #700

maxmumford opened this issue Dec 7, 2016 · 10 comments

Comments

@maxmumford
Copy link

Hi all,

I'm trying to update my model from within an rv-each but the DOM isn't updating as a result.

I use rv-each to create a bunch of inputs, one for each object in a list. On input keyup, I check the validity of the input value, and update a "valid" flag on the model. This all works and the console prints the correct value for the validity flag, but the DOM isn't updated as a result.

My sleuthing suggests that the model parameter passed to the validation function is a different object to the one that is bound to the DOM.

Here's a simplified example on jsfiddle to show you what I'm talking about:

https://jsfiddle.net/maxmumford/9pazye3c/2/

Either I'm doing something very wrong, or this is a bug. Any help?

Max

@kojilab
Copy link

kojilab commented Dec 7, 2016

I think it has something to do with passing the model in your event handling function. It's not the same model object it looks like. If you remove it, it works.
https://jsfiddle.net/6hrna9kn/

@maxmumford
Copy link
Author

... you're right. That's pretty damn bizarre... so am I right in thinking accessing the model passed in by rivets, or the original declaration of model (declared when binding rivets for the first time) is exactly the same? Is there a best practice for one or the other?

@kojilab
Copy link

kojilab commented Dec 7, 2016

I always access the "higher scope" variable if I may call it that way. Never tried to do it your way :) Maybe the internals of rivets clone the model.

@maxmumford
Copy link
Author

Ok thanks, I'll do the same :)

@blikblum
Copy link
Contributor

blikblum commented Dec 7, 2016

Rivets clone the properties of the model. It has some side effects like the above.
In my branch i chenged to not clone the properties. Here is a fiddle: implementing the behavior with my changes: https://jsfiddle.net/9pazye3c/3/

@maxmumford
Copy link
Author

@blikblum thanks for the info, it seems like very counterintuitive behaviour to me... why provide the model at all if it's a clone and we just have to access the higher scope model anyway? Perhaps I'm missing something? @mikeric could you shed any light?

@benadamstyles
Copy link
Collaborator

benadamstyles commented Dec 14, 2016

Ok sorry for missing this one. This actually has nothing to do with cloning (EDIT: actually, it may have to do with cloning, but that doesn't help us arrive at the solution below). Here is an updated fiddle that works: https://jsfiddle.net/LeedsEbooks/9pazye3c/4/

There is another closed issue regarding this precise problem but I can't find it now. TL;DR: all top-level properties of the model object must be objects or arrays. Top-level primitives will not be observed for changes.

The problem derives partly from how rivets manages to observe your model object and react to changes – the rivets 'magic' if you will. It does this (by default – this can be changed of course) by assigning getters and setters for all the properties of your model. However, it treats the model object passed into rivets.bind(view, model) as a collection of models (the argument is actually named models in the source code) and doesn't observe the immediate properties of that object, but instead all the properties of all the immediate properties of your top-level model object.

One approach is to do this instead: rivets.bind(view, {model: model}), and then prefix all your references in your html with model.

The approach I use is simply to put everything in an object.

SECOND EDIT: What actually happens (translated to JS) is:

for (key in models) {
  this.models[key] = models[key];
}

Rivets copies each property of the top-level object you pass in – models – to its own internal this.models object. It doesn't clone, it copies – but because of this and how JS works, objects get copied by reference, but primitives get copied by value.

@blikblum
Copy link
Contributor

There is another closed issue regarding this precise problem but I can't find it now

These are related issues: #486 #512 #417

Top-level primitives will not be observed for changes.

They are observed, just evaluate with devtools scope.main and scope.nested properties from this fiddle and note that these properties are transformed with getters and setters. But due to property being copied in each and if binders, it can lead to such issues.

One approach is to do this instead: rivets.bind(view, {model: model}), and then prefix all your references in your html with model

This is an workaround to the limitation of the current implementation. With a rivets version that does not copies the properties, observing root properties with primitive values works. See this fiddle from #512

I also created a more complex example to highlight the issue:
Current rivets: http://codepen.io/blikblum/pen/eBQzJO
Updated rivets: http://codepen.io/blikblum/pen/MKXXOX

Try to use the input fields and see the difference

@nbarbettini
Copy link

@blikblum Sorry to ping an old issue, but is there an issue I can follow for fixing the "observing root properties with primitive values" problem?

@blikblum
Copy link
Contributor

AFAIK no.

I proposed it, among other changes, in #558 .

Later when i already implemented i pointed how to fix through #486 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants