Vote on {% include %} behaviour.
Tai Lee
There are several open tickets (some getting quite old now) that all
stem from the inconsistent behaviour of the {% include %} tag. When a
quoted string is used for the path, it is treated as a special case
and the include is executed at compile time. Otherwise, it is executed
as the template is rendered.
The docs don't seem to mention this special case, and a few examples
of buggy behaviour and confusion stemming from this include:
- Included templates with a missing path sometimes fail silently, and
sometimes loudly.
- Template loaders raise TemplateDoesNotExist when the specified
template *does* exist, if it happens to use an include with a quoted
path that references another template that does not exist.
- Included templates do not render in the full context of their
parent template as implied by the docs. This means you can't use `{%
block %}` tags inside included templates and override them in
templates that extend the parent template.
My feeling is that including templates should be consistent. If we
support variables as the path argument to the include tag, then ALL
uses of the include tag should be processed when the templates are
rendered.
This means that it will not be possible to use block tags inside
includes. This is not possible now, but there is an open ticket that
would like to make it possible (only for the special case quoted
string path includes).
I would also like to see includes either always fail loudly, or always
fail silently. I think that the most common and expected behaviour
currently is for includes to fail silently, but I don't really care
either way.
The best solution that I can see is to remove `ConstantIncludeNode`
entirely (as has been suggested in at least one of the tickets).
Includes would always be processed at render time, it would not be
possible to extend blocks defined inside an included template, and
including templates that does not exist would no longer raise a
TemplateDoesNotExist exception when getting the parent template (which
does exist).
Could we get a quick vote from any core committers and other
interested parties, so that we can move forward to fixing and closing
these tickets?
https://code.djangoproject.com/ticket/16147
https://code.djangoproject.com/ticket/12064
https://code.djangoproject.com/ticket/12008
https://code.djangoproject.com/ticket/598
Cheers.
Tai.
Jonathan Slenders
> includes. This is not possible now, but there is an open ticket that
> would like to make it possible (only for the special case quoted
> string path includes).
for the following case, which I often do:
> A includes B; B extends from C.
I really never want to have the {% block %} names of B/C in previous
example to be available for overriding in templates which inherit from
A. This would even cause unexpected collisions between block names.
The author of the include B, is not supposed to know where his
template will be included, and we can't expect him to choose block
names which don't collide with those of A.
I can understand you want to override some part of the include B in
the main template A, that's what I made {% decorate %} for. (Which I
still would like to become a django core tag.) It's different from {%
include %}.
https://github.com/citylive/django-template-tags/blob/master/src/django_template_tags/templatetags/decorate.py
Further, I'm not aware of any inconsistencies in include behaviour
when using variable template names. Maybe anyone else?
Cheers,
Jonathan
Tai Lee
On Jun 3, 9:32 pm, Jonathan Slenders <jonathan.slend...@gmail.com>
wrote:
> example to be available for overriding in templates which inherit from
> A. This would even cause unexpected collisions between block names.
> The author of the include B, is not supposed to know where his
> template will be included, and we can't expect him to choose block
> names which don't collide with those of A.
make it clear that the actual include is to occur at render time, and
any blocks contained therein are completely separate to blocks in the
parent template. It should not be possible to replace a block in an
included template by defining the same block in the including (parent)
template.
> Further, I'm not aware of any inconsistencies in include behaviour
> when using variable template names. Maybe anyone else?
path to the template to be included, `ConstantIncludeNode` is used
instead of `IncludeNode` as a special case. This tries to include the
specified template when the parent template is compiled instead of
when it is rendered. This is supposed to be functionally the same as
using a variable path argument (which is executed when the template is
rendered), but slightly optimised. Unfortunately, it's not
functionally the same.
One example of this difference causing a problem is that when
`TEMPLATE_DEBUG` is `True`, a `TemplateDoesNotExist` exception is
raised when compiling a template that does exist, if it contains `{%
include "template_that_doesnt_exist.html" %}`. This means that any
code that checks for the existence of a template (with
`select_template()`) could break and behave differently in production
and development environments based on the content of the template.
If the template tries to include the same non-existant template, but
passed to `{% include %}` as a variable, no exception is raised.
Another example cited in the tickets is that it is impossible to
conditionally include a template that uses a literal string as the
path argument. E.g. `{% if some_false_var %}{% include "..." %}{%
endif %}` will raise `TemplateDoesNotExist` because the include is
executed when the parent is compiled, even though it is never rendered
because the context at render time causes the condition to fail.
Cheers.
Tai.
Luke Plant
> Could we get a quick vote from any core committers and other
> interested parties, so that we can move forward to fixing and closing
> these tickets?
From what I've seen so far, your proposal sounds good. Is there a way
that we can keep the performance benefits of ConstantIncludeNode? For
example, if an IncludeNode detects that it resolves to the same template
path as last time, it doesn't load the template from disk again? If so,
that would remove my only objection.
Regards,
Luke
--
"Underachievement: The tallest blade of grass is the first to be
cut by the lawnmower." (despair.com)
Luke Plant || http://lukeplant.me.uk/
Aymeric Augustin
> From what I've seen so far, your proposal sounds good.
I agree — consistency is good.
> Is there a way
> that we can keep the performance benefits of ConstantIncludeNode? For
> example, if an IncludeNode detects that it resolves to the same template
> path as last time, it doesn't load the template from disk again? If so,
> that would remove my only objection.
ConstantIncludeNode improves performance because it calls django.template.loader.get_template() only once in __init__() and not in each call to render().
get_template() invokes the template loading machinery to create a compiled Template object, given a template path. If it's a performance bottleneck, we can memoize its results. That will improve performance of all template lookup operations, not only {% include %}.
We must ensure that during development, if we edit a template without touching the Python code, we get the new version and not the old compiled, memoized version. The easiest is to memoize only when TEMPLATE_DEBUG is False.
There is already some code to cache loaded templates in django.template.loaders.cached.Loader, but as far as I can tell, it's only used during Django's own tests. This is also true for the reset() method of template loaders. If memoizing doesn't work — it's a bit crude — maybe we could extend this code to cache all template loading operations.
Best regards,
--
Aymeric Augustin.
Luke Plant
> ConstantIncludeNode improves performance because it calls
> django.template.loader.get_template() only once in __init__() and not
> in each call to render().
>
> get_template() invokes the template loading machinery to create a
> compiled Template object, given a template path. If it's a
> performance bottleneck, we can memoize its results. That will improve
> performance of all template lookup operations, not only {% include
> %}.
We already have the cached template loader, and we don't want to invoke
that kind of behaviour unless asked for (it's up to users to put it in
their TEMPLATE_LOADERS settings), as it has significant memory overhead.
I was thinking something simpler, like this:
class IncludeNode(BaseIncludeNode):
def __init__(self, template_name, *args, **kwargs):
super(IncludeNode, self).__init__(*args, **kwargs)
self.template_name = template_name
def render(self, context):
try:
template_name = self.template_name.resolve(context)
if getattr(self, 'last_template_name', None) == \
template_name:
template = self.template
else:
template = get_template(template_name)
self.template = template
self.last_template_name = template_name
return self.render_template(template, context)
except:
if settings.TEMPLATE_DEBUG:
raise
return ''
If I'm thinking correctly, that will keep the included template in
memory only for the lifetime of the parent template, and will avoid
loading it more than once if it is a static string and the include is in
a loop.
Stephen Burrows
current implementation is correct and just needs to be explained
better. As for the other two... I agree that consistency is important,
and that it would make sense (in a way) for ConstantIncludeNode to
either always raise or always silence, but that can definitely be done
without removing the CIN or hacking together some other way to get
similar compilation caching.
It's also worth mentioning that if TEMPLATE_DEBUG is set to ``True``,
the (non-constant) IncludeNode will raise an exception when it
*renders*, even though the Django docs clearly state that "render()
should never raise TemplateSyntaxError or any other exception. It
should fail silently, just as template filters should." If anything,
this is a more egregious error than ConstantIncludeNode's behavior.
That being said, whatever is decided, perhaps it could be applied to
the ExtendsNode as well? Right now, constant extends are loaded at the
same time as variable extends - handling constants differently could
give a similar performance boost there. In fact, there's a ticket open
to do just that: https://code.djangoproject.com/ticket/6586.
George Karpenkov
should be rendered at the compile time or at the rendering time.
For some applications it is really useful to have recursive inclusion
of templates, which is impossible with compile-time inclusion. For
example, a few days ago I was writing a view which visualizes a diff
between two mongoDB documents with arbitrary levels of nesting. It can
be elegantly expressed with recursive includes, yet I was forced to
add additional render_to_string statements to my view, unnecessary
complicating the code.
The problem with processing the inclusion at render time to me is
that:
x Errors are left unnoticed for a longer time - eg if included
template has any errors you won't see them until you are actually
processing the view, and if your include is conditional it can get
complicated.
x Degrades performance.
I can't really think of an elegant syntax to that though. {% include
"blah.html" render %} seems ugly and would be very confusing to
beginners.
George