[regression] _Widget.get breaks dtl.Context when using _DomTemplated

[ben hockey]

in dtl a Context has a function called get and so now if you pass a widget as a context (as _DomTemplated does), then the widget's get overrides the dtl Context's get.

this causes a problem if you have something like the following in your template:

<input type='text' dojoAttachPoint='valueNode' value='{{ value }}'></input>

and then in your js you have something like:

_getValueAttr: function(){ return this.valueNode.value;}

as the template renders, it calls the get function of the context (which is the widget's get) and it ends up calling _getValueAttr and causes a problem because valueNode does not yet exist. classic chicken and egg problem :)

for now, i've worked around it by adding _setValueAttr which updates this._value and use _value in the template.

<input type='text' dojoAttachPoint='valueNode' value='{{ _value }}'></input>

_setValueAttr: function(value){
    this._value = value;
    if(this._started){
        this.render();
    }
}

i've marked the component as dijit since this is what has caused the error. dtl hasn't changed in about 6 months. also, note that the code i've used here is not my actual code so it might not work but it shows the problem

[bill]

Regression from #10839

[Mark Wubben]

This seems to be a problem with DTL's Context though. It mixes in the original context dictionary when the context is created, allowing it to override context methods. Instead it should wrap the dictonaries.

[ben hockey]

Replying to markwubben:

This seems to be a problem with DTL's Context though. It mixes in the original context dictionary when the context is created, allowing it to override context methods. Instead it should wrap the dictonaries.

maybe so. but it can also be argued that mixing in the dictionary is a feature that allows you to provide a getter as part of your dictionary to override the default getter of the context. how many people, knowing that this is how it works, have intentionally exploited this "feature"?

personally i don't have a strong opinion either way - i can see both sides. i just happened to experience this regression and raised it as an issue. i'm less concerned about "how" it's resolved and more interested in seeing it resolved somehow soon. wrapping the dictionary would work for me since i've never intentionally exploited this "feature".

in #11220 neil mentions that he's updating the architecture of dtl and if that's the case then it might be possible that changing dtl will be the fix for this issue but i just hope it comes sooner rather than later.

[Kris Zyp]

I am not sure I understand, I would have thought that being able to correctly access attributes of a widget through a template (by going through get()) would be a desirable feature. Does markwubben have some patch for whatever he is suggesting? (I don't understand that one either)

[ben hockey]

Replying to kzyp:

I am not sure I understand, I would have thought that being able to correctly access attributes of a widget through a template (by going through get()) would be a desirable feature. Does markwubben have some patch for whatever he is suggesting? (I don't understand that one either)

the problem is that dtl has a get for it's Context objects. the constructor for a Context then mixes in a dictionary which it uses for rendering the properties of that dictionary. in the case of dojox.dtl._DomTemplated the widget is the context and so the mixin in the constructor of the Context causes the widget's get to override the Context's get.

it is actually not desirable to "correctly access attributes of a widget" for rendering the template since these attributes could be lazily evaluated with a dependency on a DOM element in the template - eg the value of a widget often considers the value of an input element in the template when responding to widget.get('value'). obviously, you can't render a template with properties that have a dependency on elements in the template - ie you can't render the result of widget.get('value') since it can't be determined until after the template is rendered. it would be preferable to render widget.value rather than widget.get('value')

what markwubben is suggesting is that rather than have the Context mixin the dictionary, it should wrap it. for example, currently, the code for the constructor is:

	dd._Context = dojo.extend(function(dict){
		// summary: Pass one of these when rendering a template to tell the template what values to use.
		dojo._mixin(this, dict || {});
		this._dicts = [];
	}

and the get for the Context is:

		get: function(key, otherwise){
			if(typeof this[key] != "undefined"){
				return this._normalize(this[key]);
			}

			for(var i = 0, dict; dict = this._dicts[i]; i++){
				if(typeof dict[key] != "undefined"){
					return this._normalize(dict[key]);
				}
			}

			return otherwise;
		}

as i understand it, the approach that markwubben suggests would be more like this:

	dd._Context = dojo.extend(function(dict){
		// summary: Pass one of these when rendering a template to tell the template what values to use.
		this._dict = dict || {};
		this._dicts = [];
	}

		get: function(key, otherwise){
			if(typeof this._dict[key] != "undefined"){
				return this._normalize(this._dict[key]);
			}

			for(var i = 0, dict; dict = this._dicts[i]; i++){
				if(typeof dict._dict[key] != "undefined"){
					return this._normalize(dict._dict[key]);
				}
			}

			return otherwise;
		}

this would allow the template to use the widget.value property when rendering rather than making a call to widget.get('value'); - this is the way it used to work. however, it is possible that this could break code which had already intentionally exploited the fact that the Context mixes in the dictionary. knowing that the Context mixes in the dictionary, it is possible that someone may have intentionally defined their own get function in a dictionary, which would no longer be used if we wrap the dictionary. the question is... did anybody ever do this and does it matter so much if we break it on them? i would guess that it probably isn't done often so wrapping the dictionary might be the way to move forward.

i don't know if the code i've provided above will work but it illustrates what i understand markwubben to be suggesting. other code would probably need to be updated, i can see that at least the update function of the Context would need to be updated but i don't know what else would be affected by this kind of change.

i don't know if neil has any ideas but i've added him in case he has something to say.

[Neil Roberts]

(In [22452]) Fixes #11105. If a get function is passed, it's preserved and used by the Context's get function rather than overriding it or getting thrown away !strict

[ben hockey]

this does the exact opposite of what the ticket was opened for. to be clear - the getter was already preserved and the ticket was opened because this was not desirable. i wanted the context to ignore the getter since using the getter may have a dependency on the template and you can't render the template with a value that depends on the template.

[Neil Roberts]

(In [22454]) Fixes #11105. Delays getter use until after the widget has been created !strict

[ben hockey]

looks like it works - thanks. i should have made a test case earlier but i'm attaching it now in case anyone wants to refer to it in the future.