[Django] #21741: render_to_string without providing a dictionary still puts a dictionary into the context
Django
into the context
-----------------------------------------+--------------------
Reporter: Keryn Knight <django@…> | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Template system | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------------+--------------------
given the following scenario:
{{{
>>> from django.template.loader import render_to_string
>>> from django.template import Context
>>> render_to_string('file.extension', context_instance=Context())
>>> # or ...
>>> render_to_string('file.extension')
}}}
where a dictionary is explicitly not being provided, the final context
given to the template when `render()` is called will have `{}` as the last
of the `.dicts`
The reason is because if
[https://github.com/django/django/blob/fe995e6cbdcf2766cf0aa951c8ef4549ad27694c/django/template/loader.py#L167
none is provided], a new dict is created, and then it is provided to the
context instance
([https://github.com/django/django/blob/fe995e6cbdcf2766cf0aa951c8ef4549ad27694c/django/template/loader.py#L173
1],
[https://github.com/django/django/blob/fe995e6cbdcf2766cf0aa951c8ef4549ad27694c/django/template/loader.py#L176
2]) without checking the dict's `len()`
Marking as cleanup/optimization because it doesn't cause any problems (the
only one being: it appears in debug_toolbar), but providing the ticket at
least means the option is there to change it, and a history of why not to
change it exists if ''wontfix'd'' (which may be the best course of action,
on balance, but that's not for me to say)
--
Ticket URL: <https://code.djangoproject.com/ticket/21741>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
into the context
Reporter: Keryn Knight | Owner: nobody
<django@…> | Status: new
Type: | Version: master
Cleanup/optimization | Resolution:
Component: Template system | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by bmispelon):
* needs_docs: => 0
* needs_better_patch: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
Hi,
I'm +0 on this but I think it'd be easier to discuss over a concrete
proposal so I'll accept the ticket.
Any proposal should not add too much complexity and should aim at
remaining as backwards-compatible as possible.
Thanks
--
Ticket URL: <https://code.djangoproject.com/ticket/21741#comment:1>
Django
into the context
Reporter: Keryn Knight | Owner: nobody
<django@…> | Status: new
Type: | Version: master
Cleanup/optimization | Resolution:
Component: Template system | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by Keryn Knight <django@…>):
The patch, such as it is,
- is in this branch
https://github.com/kezabelle/django/tree/bugfix/ticket_21741
- diff against master is
https://github.com/kezabelle/django/compare/bugfix;ticket_21741
- the commit in question is currently
https://github.com/kezabelle/django/commit/b2f6a4fa7049e888465a50394f038f6c7a7d31f4
All tests pass, but there's no test to evidence the proposed fix works as
intended, because I'm not sure ''how'' to write such a unit test - the
function is a black box - I can't verify the context given to the template
is of the correct length, because the return value is obviously a string.
Any ideas on how one might test the context for correctness in this
scenario would be gratefully received.
--
Ticket URL: <https://code.djangoproject.com/ticket/21741#comment:2>
Django
into the context
Reporter: Keryn Knight | Owner: nobody
<django@…> | Status: new
Type: | Version: master
Cleanup/optimization | Resolution:
Component: Template system | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by bmispelon):
I guess it'd be possible to write a custom template tag that outputs info
about the context and use it, but that might be a bit complicated.
Another way would be to create a custom `Context` and pass it as
`context_instance`. That way you could make sure that no extra layer is
added to it.
The downside of this is that you're only testing half of the change but
other than that, it's quite simple to implement.
--
Ticket URL: <https://code.djangoproject.com/ticket/21741#comment:3>
Django
into the context
Reporter: Keryn Knight | Owner: numerodix
<django@…> | Status: assigned
Type: | Version: master
Cleanup/optimization | Resolution:
Component: Template system | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* keywords: => nlsprint14
* owner: nobody => numerodix
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/21741#comment:4>
Django
into the context
Reporter: Keryn Knight | Owner: numerodix
Type: | Version: master
Cleanup/optimization | Resolution: fixed
Component: Template system | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: nlsprint14 | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"7e1376c2b0ea9e02e61c1c764f02cd40c6f7e849"]:
{{{
#!CommitTicketReference repository=""
revision="7e1376c2b0ea9e02e61c1c764f02cd40c6f7e849"
Fixed #21741 -- Fixed render_to_string to stop pushing empty dictionaries
to its Context
Thanks to kezabelle for the report and original patch
and to numerodix for his improved patch.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21741#comment:5>