Use prototype inheritance in each-binder

[jhnns]

This fixes a problem where arrays and objects where not updated inside the each-binding.

I've created a JSFiddle to demonstrate the problem: http://jsfiddle.net/hnodsc7n/1/ When the items-array is changed, the each-binding still holds a reference to the old items-array. This is because rivets just copies the properties from one model to another. However, this fix uses prototype inheritance. Thus every change on the super-model is propagated to the sub-model. Check out the difference: http://jsfiddle.net/znv28cmj/1/

There are two potential problems with this fix:

• I'm using Object.create() which is not available in IE8. However, there is a polyfill for it (which I haven't included because imho libraries shouldn't bring their own polyfills).
• If someone is iterating over the sub-model and checks for if (obj.hasOwnProperty(key)) it will not iterate over all properties anymore.

All tests are running

[mikeric]

@jhnns This approach sounds very promising, and would solve a ton of issues around binding to root properties within nested views. The two potential problems you have listed aren't that problematic (compared to the benefits of this implementation), in my opinion.

Reviewing / testing this now.

[jhnns]

Cool :+1:

[mikeric]

@jhnns This is close. But it seems like it doesn't solve the problem entirely. The main issue is the following:

If you bind to a property within an iterated view that was originally set on the parent object (the prototype of the child object), it will redefine that property on the child object and observe changes on the child object's property instead of the parent object. See this demo: http://jsfiddle.net/znv28cmj/4/

The way that I've been trying to get things to behave is that if you change a root property on the parent scope, any bindings to that property in a nested scope should get the changes as well (it should point to the same property). Likewise, changing that property in the child view should also update it in the parent view (again, ideally they should point to the same property), but this is not as necessary as the former scenario (parent property updates the same property on child views).

If we can get this to work, it would greatly simplify things. You should technically be able to bind your entire view / viewmodel as the scope object and all nested views would react the same way to properties on that view, as long as your models, collections and functions are reachable from your view object.

rivets.bind(el, myView)

[mikeric]

Couple of solutions that I can think of:

1. Instead of copying over the parent properties onto the child scope, we can have two scopes for each view — the parent scope (which will just point to the parent's view.models object) and the scope of the current view (which will be a new object containing only local properties to that view).

   When binding, we would first check the local scope if the property exists, if it does then we bind to that property, else we bind to the property on the parent scope.

2. Same as the first solution, but instead of checking the scopes and deciding for the user, let the user specify if the property is local to the current scope or is on the parent scope. For example, user would be the user property on the local scope and $user would be on the parent scope?

   Not sure how I feel about this method, as it pollutes the keypaths with specific symbols that the user may want to use / already using for an adapter.

[jhnns]

Mhmm you're right, of course...

I don't like the second solution because it is counter-intuitive and introduces a special syntax. But I don't get your first solution either – what is the difference between the scope and view.models?

Imho there are these two solutions:

1. Use JavaScript setter/getter to "proxy" properties from the parent model. This would just do what most users would probably expect, but it will remove compatibility with IE 8 (which is imho not that bad by now)
2. Don't copy anything to the child scope but add a parent-property to the child scope which gives full power and flexibility back to the developer.

[mikeric]

@jhnns The idea behind the first solution, is to never store properties more than once. Each view's scope (currently view.models) will only contain the local properties to that view. In addition to this, the nested view will also reference a parent scope, which Rivets will use to continue to look for a property if it doesn't exist in the local scope.

To visualize this:

<div id="main">                 <-- 1

  <h1>{ model.title }</h1>      // this will bind to 1's model.title

  <div rv-each-item="items">    <-- 2

    <p>{ item.name }</p>        // this will bind to 2's item.name

    <p>{ model.type }</p>       // this will bind to 1's model.type

    <rv-view                    <-- 3
      ref="DetailsForm"
      model="item">
    </rv-view>

    <p>{ test }</p>             // doesn't exist in scope chain. bind to 1 or 2?
  </div>
</div>

Initializing the root view like this:

rivets.bind($('#main'), { model: myModel, items: [myItem] })

Will create these views:

1:
  parent: nil
  scope: { model: myModel }

2:
  parent: 1
  scope: { item: myItem, index: 0 }

3:
  parent: 2
  aliases: { model: 'item' }
  scope: {}

I suppose the problem with this is that inside your event handlers you would need to know about the scopes and where to find the properties you need access to.

The other thing to consider is where do we bind if the property doesn't exist in the scope chain? Should we bind to the local scope or the root scope (like javascript does when you assign a variable without var)?.

[jhnns]

Ok, now I get it. That's exactly how prototype inheritance works under the hood. Maybe we should just use prototype inheritance but check on hasOwnProperty() before setting a value:

// no parent
var scope1 = {
    model: myModel
}; 
// scope1 as parent
var scope2 = Object.create(scope1);
scope2.item = myItem;
scope2.index = 0;

Reading values is no problem, it's just prototype inheritance:

scope2.item; // returns myItem;
scope2.model; // returns myModel

Writing values would work like this:

while (scope.hasOwnProperty(key) === false) {
    scope = Object.getPrototypeOf(scope);
}
scope[key] = value;

Again, that wouldn't work in IE8 because there is no Object.getPrototypeOf... but well, I guess it's not too hard to implement this behavior in rivets as you described it.

[jhnns]

Oh an concerning your question: I think it should write to the current scope. JavaScript's behavior has been removed with strict mode for good reasons :wink:

[sebastianconcept]

<h1>{ model.title }</h1>      // this will bind to 1's model.title

  <div rv-each-item="items">    <-- 2

    <p>{ item.name }</p>        // this will bind to 2's item.name

    <p>{ model.type }</p>

if you want a controller and a model to work on the div that has the rv-each- and model type is a model of a child controller with its child model and you bind them that way, then I would not expect it to be the same instance as {model.title}

[jhnns]

In your example model.title and model.type will work on the same model instance.

[sebastianconcept]

yes @jhnns my point is that I need the opposite of mixed concerns :)

I do have a controller that is concerned about lists of controllers (and uses each- to implement how that gets maintained, which is awesome) and the children controllers that are one per item with its own instance of controller and model.

Nothing mixed, all decoupled and complexity scales fantastically well

[jhnns]

And what would model then be?

[sebastianconcept]

The model of each children would be whatever they where configured to be. Typically the parent controller will instantiate them and tell what their model is. Usually a (sub)part of the parent model (but not necessarily).

Answering your question with an example:

<h1>{ model.name }</h1>      // this model is a car

  <div rv-each-tire="tires"> 
    <p>{ model.pressure }</p> // this template should have its own controller and model is a tyre

As example this is probably too simplistic and is tempting to think is just overhead to have one controller per each but think in having really complex things there and it will make sense. Instead of a <p> with the tyre pressure, think of an entire widget that monitors and controls pressure, other for temperature, other for X parameter, etc. And in turn they can have their own sub widgets or lists of widgets (and submodels).

[jhnns]

It's possible to initialize all scope variables with custom values, but I think it's a good default to just inherit from the parent scope because that's very common.

With inheritance you have the flexibility as you've described and the convenience as I suggested.

[mikeric]

It's possible to initialize all scope variables with custom values, but I think it's a good default to just inherit from the parent scope because that's very common.

@jhnns I agree, but I think it really depends on the particular iterated / child view and where the functionality of that view is coming from. Two scenarios that I see:

1. When the iterated / child view is coupled to the parent view (shared functionality and scope), then it would obviously make sense to inherit scope of the parent. This should be the default behaviour of the each-*, if and unless binders when used on any regular element. The general idea here is that the entire view, along with the iterated / child views, are implemented in the same parent scope.

   <!-- parent view template -->
   <div rv-each-item="items">
     <button rv-on-click="removeItem"></button>
   </div>
   

   // parent view scope
   {
     items: [ … ],
     removeItem: function(ev, scope) {
       scope.item.remove()
     }
   }
   

2. When the iterated / child view is a reusable component and is intended to be used independently or within many different parent views, then it should have isolated scope. The scope for that view would likely be a controller or some sort of viewmodel that is only concerned with that particular component.

   <!-- parent view template -->
   <rv-item rv-each-item="items" item="item"></rv-item>
   

   <!-- item view template -->
   <button rv-on-click="remove"></button>
   

   // parent view scope
   {
     items: [ … ]
   }
   

   // item view scope
   {
     item: item,
     remove: function(ev, scope) {
       scope.item.remove()
     }
   }
   

@sebastianconcept Does the second scenario describe your use-case more closely?

[jhnns]

angular's scope model is picking up this requirement pretty good

[sebastianconcept]

@mikeric yes. The second scenario is what I wildly use. Thanks for the nice description.

[jhnns]

And what should we do next? :grin:

[Arjeno]

Just joining in and giving my two cents. Keeping track of the parent view and falling back to a key path when a local one is missing would definitely be a good idea. This way there is also less pollution on each object in the iteration right? This would also lead to less complex user code when you have multiple nested iterations.

[stalniy]

@mikeric any updates on this?

[stalniy]

this can be fixed easily if you use object like this: http://jsfiddle.net/w23uLttu/

CC: @blikblum, @mikeric, @jhnns

[stalniy]

@Leeds-eBooks @Duder-onomy @blikblum @jccazeaux guys this is something what we should also pick up. This will also improve performance of rivets.bind function

UPDATE: by the way as @mikeric mentioned this can be used for every binding, not only in each-* (unless, if, component binder). In order to keep things consistent and do not introduce a break change we could add one more option usePrototypeInheritanceInViews: true/false and depends on that option create an util function models = Rivets.Util.createObject(this.view.models). So, then inside we can decide what to do (i.e. for..in or Object.create).

[jccazeaux]

I never encountered this problem because I always used an global scope object like suggested in @stalniy fiddle. I don't see a real break change here.

• If you use a global scope object, this problem doesn't really exist and inheritance won't change anything
• If you don't use a global scope object using inheritance could only solve weird behaviors. Besides IE8 compatibility... but default adapter already uses unsupported defineProperty so is it really a problem? I wonder why @mikeric has not done it already? Anything I'm missing or was it only because of IE8?

@stalniy IMHO components are not concerned. If their model is not independent from parent, they won't be easily reusable.

[stalniy]

@jccazeaux I'd not say that. When PR with optional template option is merge it will be possible to extend default behavior of forms and inputs. This is probably the case where you would like to have access to parent variables inside form component

[jccazeaux]

@stalniy If component has access to parent, parent will have access to component. This will lead to side effects : if a parent defines same attribute as a component the values will be linked

As an example, here are two fiddles. scope.foo exists in both parents and component models. If there is inheritance, databinding in component will affect parent model.

• Components with prototype inheritance : parent and component scope.foo are the same
• Actual component : parent and component scope.foo are isolated

A component can't know if it is defining an attribute already defined in parent (it doesn't know what will be the parent). This side effect has good chance to happen and can be really annoying.

If a component needs something from parent, parent must send the data through attributes.

[stalniy]

@jccazeaux Probably I wasn't clear enough. What I'm saying is that it useful to extend default html elements' behavior, so for example form component (doesn't have template as it depends on context):

<form name="myForm">
   <input required rv-value="user.name" /> 
   <span class="error" ng-show="myForm.invalid">error!</span>
</form>

In this case you don't want to pass all the parent variables into form component, do you?

P.S.: myForm is a controller of rivets.components.form component

[jccazeaux]

Indeed I didn't understand it right. My idea of components is not to extending default html elements, but to create new ones. Mixing these capabilities could lead to complex API as these are not the same objectives. Maybe what you seek is more something like a binder linked to element's name?