Fix context.templateName for directly loaded templates being used as partials #368
Conversation
…mplateNames on directly loaded templates used as partials
| @@ -705,6 +705,7 @@ function getGlobal(){ | |||
| var partialChunk; | |||
| if (typeof elem === 'function') { | |||
| partialChunk = this.capture(elem, partialContext, function(name, chunk) { | |||
| partialContext.__templateName = partialContext.__templateName || name; | |||
There was a problem hiding this comment.
This is the meat of the PR.
There was a problem hiding this comment.
I thought name in this case was typically out: as in, the captured output from calling elem. Should that be used as the __templateName?
There was a problem hiding this comment.
+1
Looking at the captureOutput implementation it's parameters are out and chunk. How does this fix the issues?
There was a problem hiding this comment.
Will add a few unit tests to look into the issue deeper.
There was a problem hiding this comment.
the elem passed to this partial function is always only in reference to the name of the partiall, so this.capture will invoke the callback with out as the name, which is always actually the template name. I looked through very complex templates to confirm this, and tried to make a unit test to somewhat emulate that.
|
Since the improved release processed mentioned in #361 has not yet been implemented, please run |
| @@ -219,7 +219,7 @@ function getGlobal(){ | |||
| this.stack = stack; | |||
| this.global = global; | |||
| this.blocks = blocks; | |||
| this.templateName = templateName; | |||
| this.__templateName = templateName; | |||
There was a problem hiding this comment.
IIRC we've used templateName and __templateName__. Perhaps we should avoid a third option? :)
There was a problem hiding this comment.
agreed, decided to go with adding a getTemplateName function to Context, so we can go through as many name changes as we want.
|
-1 for making people reference a dunderVariable. Suggest adding a |
… partials, and dist files
|
@jimmyhchan completely agreed, since we've already gone through a lot of variations, so I added that function to the most recent commit. |
| @@ -270,7 +270,7 @@ function getGlobal(){ | |||
| var ctx = this.stack, | |||
| i = 1, | |||
| value, first, len, ctxThis; | |||
| dust.log('Searching for reference [{' + down.join('.') + '}] in template [' + this.templateName + ']', DEBUG); | |||
| dust.log('Searching for reference [{' + down.join('.') + '}] in template [' + this.__template_name__ + ']', DEBUG); | |||
There was a problem hiding this comment.
Shouldn't all of these use the getTemplateName method?
|
LGTM, but the code should probably use |
| this.stack = stack; | ||
| this.global = global; | ||
| this.blocks = blocks; | ||
| this.__template_name__ = templateName; |
There was a problem hiding this comment.
do we need to have this __?
this.templateName and prototype.getTemplateName seem good enough.
like stack, global and blocks this is an internally stored item which is consistent internally. Using context.* directly is not documented. Only the context.* methods are documented.
I'm not a fan of the inconsistency. Perhaps having _stack, _global, '_blocks` but that's a back compat issue.
There was a problem hiding this comment.
Agreed. As we discussed, I'll revert template_name to templateName
|
Looks good to me. Thanks for additional unit tests! |
Add context.getTemplateName. This method now correctly returns the template name even for directly loaded templates being used as partials. For end users please use this api for getting the template name instead of ctx.templatename
|
I’m updating the Wiki to cover 2.2.x versions. At 2.0.x we added {templateName} to global and then moved it to context. Now that seems to be gone. I guess this is a backward incompatible change. I see the getTemplateName method on Context but how might a user template wanting to display the template name do this now without dropping down into JS? |
|
Since templateName variable is no longer on global or part of head\tail of context I don't think user template can access templateName. Issue #385 created to discuss this |
Also change context.templateName to context.__templateName, since it's strange to not be allowed to use templateName in your json model.
The other way of doing this is to set it on the ctx object inside the compiled dust body_0. If that is preferred, I'll change this PR.