MVC - dot separated declarative ref with implied parent relative binding fails

[ben hockey]

the summary is a mouthful... the short version is, this doesn't work with a parent binding

<div data-dojo-prop="ref:'foo.bar', ..."></div>

i'm expecting this to get parentBinding.foo.bar but instead it's essentially trying to get parentBinding['foo.bar']. subtle difference but not useful :)

the solution is simple and may even help explain the problem better

_DataBindingMixin.js

@@ -199 +199 @@
                         }else{ // defaults: outermost refs are expressions, nested are relative to parents
                                 parentBinding = parentBinding || this._getParentBindingFromDOM();
                                 if(parentBinding){
-                                        binding = parentBinding.get(ref);
+                                        binding = dojo.getObject(ref, false, parentBinding);
                                 }else{
                                         try{
                                                 binding = dojo.getObject(ref);

[rahul]

+1, I'm quite sure the proposed fix was the intent ;-)

Unrelated to this ticket, but related to the topic, I've also thought about the pluggability of the ref syntax itself i.e. say if someone wants to come along and use something like JSONPath instead of a dot separated ref expression, could we afford that change to be made more easily (and the related topic of a real dependency graph in the model when relative references are not simple as dot separated property access, but thats truly a digression).

[ben hockey]

part of my outline in #13263 may lend itself towards making the ref syntax a little more pluggable. my outline includes _setRefAttr which is dijit's version of a setter for the ref property. any time someone calls widget.set('ref', ref) the _setRefAttr function is called with ref being passed to it. this provides an extension point that would be suitable for various implementations of ref parsing. we could either leave it for subclasses to override the default implementation if they want different behavior (favors programmatic) or we could implement it in such a way that it provided a framework for plugins to interpret ref (favors declarative). some of the date-based widgets have a similar thing to allow various date packages to be used (see datePackage in dijit.CalendarLite?).

[ben hockey]

fixed in r25798

[Ed Chatelain]

Hi Ben, thanks for reopening this. (I am just making this comment to get on the cc list)...

[ben hockey]

reopening this because ed let me know that r25798 broke cases where a property was trying to be set but the name of the property is an integer rather than a string.

eg

// broken
model.set(1, value);

// vs

//works
model.set("1", value);

the question is at what level do we want to fix this problem?

i could probably change r25798 such that it was dojo.getObject('' + ref, false, parentBinding); and cast ref to a string but i'm not sure if this is the right level to fix this since Stateful::get is documented as needing a string for the property name. this change would make dojo.getObject work but we would still have code trying to do "illegal" things. also, this line of code was just a copy of what's happening a few lines above in the branch that handles declarative rel type refs so that branch would break the same as this if a test covered that as well.

how do you guys think this should be fixed?

[ben hockey]

hmm... i think i spoke too soon and i'm wrong about the code doing "illegal" things. it could be the right thing to just change that call to getObject so that we cast ref to a string. if there's no objections i'll just make that change. should i change the similar call on line 188?

[Ed Chatelain]

Looks good to me, all tests ran successfully with both lines updated.

[ben hockey]

(In [25818]) coerce dojo.getObject calls to use strings. fixes #13446