#13446 closed defect (fixed)
MVC - dot separated declarative ref with implied parent relative binding fails
Reported by: | ben hockey | Owned by: | ben hockey |
---|---|---|---|
Priority: | high | Milestone: | 1.7 |
Component: | DojoX MVC | Version: | 1.7.0b1 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description
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 200 200 parentBinding = parentBinding || this._getParentBindingFromDOM(); 201 201 if(parentBinding){ 202 binding = parentBinding.get(ref);202 binding = dojo.getObject(ref, false, parentBinding); 203 203 }else{ 204 204 try{ 205 205 binding = dojo.getObject(ref);
Change History (11)
comment:1 Changed 10 years ago by
Component: | General → DojoX MVC |
---|---|
Owner: | set to rahul |
comment:2 Changed 10 years ago by
comment:3 Changed 10 years ago by
Owner: | changed from rahul to ben hockey |
---|---|
Status: | new → assigned |
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?).
comment:5 Changed 10 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
comment:6 Changed 10 years ago by
Hi Ben, thanks for reopening this. (I am just making this comment to get on the cc list)...
comment:7 Changed 10 years ago by
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?
comment:8 Changed 10 years ago by
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?
comment:9 Changed 10 years ago by
Looks good to me, all tests ran successfully with both lines updated.
comment:10 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
comment:11 Changed 9 years ago by
Milestone: | tbd → 1.7 |
---|
+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).