#19878 closed Cleanup/optimization (wontfix)
Stop TemplateView automatically passing kwargs into the context
Reported by: | Alexey Boriskin | Owned by: | Adam Johnson |
---|---|---|---|
Component: | Generic views | Version: | dev |
Severity: | Normal | Keywords: | django-sprint |
Cc: | marc.tamlyn@…, mszamot@…, chris.jerdonek@…, Adam Johnson | Triage Stage: | Unreviewed |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Only TemplateView pushes self.kwargs to the context. ListView does not, I yet have to check others.
This is inconsistency and, I think, it should be fixed.
Change History (25)
comment:1 by , 12 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|---|
Type: | Bug → New feature |
comment:2 by , 12 years ago
Has patch: | set |
---|---|
Keywords: | django-sprint added |
Resolution: | → fixed |
Status: | new → closed |
https://github.com/django/django/pull/753
The CBV:
- BaseDetailView
- ProcessFormView
- BaseListView
Did not push the kwargs into the context
comment:3 by , 12 years ago
Resolution: | fixed |
---|---|
Status: | closed → new |
The ticket is not fixed until patch in merged into the django. Please don't set status to closed until it's not closed.
comment:4 by , 12 years ago
This can be backwards incompatible in keys in kwargs
clash with keys in the output of get_context_data()
.
The current behavior might be for backwards compatibility with the now-removed function-based generic views. Maybe this was discussed on django-developers?
This would need docs (release notes + check if the CBV docs need to be updated), and tests.
comment:5 by , 12 years ago
Needs documentation: | set |
---|---|
Needs tests: | set |
comment:6 by , 12 years ago
Cc: | added |
---|
I am strongly the other way on this issue - I think the feature is wrong in TemplateView
and should be deprecated.
For example, it means that it's hard to have context_object_name
and slug_url_kwarg
the same - whether you get the object or the slug would be rather arbitrary.
comment:7 by , 12 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Agreed with mjtamlyn, so marking this wontfix.
comment:8 by , 12 years ago
Jacob - what's your opinion on taking the deprecation the other way - this one-off behaviour of TemplateView
has bitten me before. Personally I'd like the views to be consistently *without* this feature.
comment:9 by , 12 years ago
Resolution: | wontfix |
---|---|
Status: | closed → new |
comment:10 by , 12 years ago
Has patch: | unset |
---|---|
Needs documentation: | unset |
Needs tests: | unset |
Triage Stage: | Design decision needed → Accepted |
IIRC TemplateView works like this for backwards-compatibility with the (now defunct) function-based generic views.
It would make sense to normalize it now, assuming we can figure out a deprecation path.
comment:11 by , 12 years ago
Summary: | Not all class based views push self.kwargs to context → Stop TemplateView automatically passing kwargs into the context |
---|
comment:12 by , 11 years ago
Cc: | added |
---|
comment:13 by , 11 years ago
Hmm, I know I'd done something at some point using these views which had bitten me, but I can't remember what it was. This is near impossible to deprecate nicely and probably shouldn't be that big a deal.
That said, it didn't exist before 1.5 in its current form - the way these arguments are used was (backwardsly incompatibly) changed then anyway. Personally I'd like to just remove it as it "feels wrong", but whether that is a good idea is another question.
History lesson
Django 1.2 had direct_to_template
which would pass through URL kwargs as an object called params
into the context
Django 1.3 & 1.4 continued this pattern in the new TemplateView
Django 1.5 changed TemplateView
to place the arguments directly into the context (and deprecated direct_to_template
)
My personal feeling is that using URLconf kwargs directly in a view is a bad pattern, but it is a historical Django pattern. This, and the ability to customise CBVs using the as_view()
call, are the remaining parts of it.
comment:14 by , 11 years ago
I ran into the same problem as @mjtamlyn with TemplateView
; and I also think that having the URL kwargs
directly in the context_data
is an anti pattern.
The anti pattern added to the discrepancy between the different views is IMO a problem big enough to warrant a documented backward incompatible change.
I tried to think of a transparent deprecation cycle, but what I found was at best fragile and hackish.
One option would be to ask people to set a kwargs_to_context = False
argument on their TemplatView
to get the "new" behavior and therefore opt out from the DeprecationWarning
.
Thoughts?
comment:15 by , 11 years ago
I'm not much of a user of CBVs but that deprecation plan sounds like it would work.
comment:16 by , 11 years ago
Note that there is a distinction between passing **kwargs
to get_context_data()
and having get_context_data()
return **kwargs
.
Personally, I would favor TemplateView
continuing to pass **kwargs
to get_context_data()
even if its return value is changed by default not to include kwargs
. This would keep overriding get_context_data()
simple because you can simply access **kwargs
in the method without having to decide between **kwargs
and self.kwargs
.
For consistency, I would like it if all generic views passed the full view's keyword arguments to get_context_data()
. Some classes like ProcessFormView
restrict what is passed to get_context_data()
, which was surprising to me because then **kwargs
for the method differs from self.kwargs
. I just opened #21964 which is about this issue.
comment:17 by , 11 years ago
Cc: | added |
---|
comment:18 by , 5 years ago
Has patch: | set |
---|---|
Needs documentation: | set |
Owner: | changed from | to
Patch needs improvement: | set |
Status: | new → assigned |
comment:19 by , 5 years ago
Cc: | added |
---|
comment:20 by , 5 years ago
Needs documentation: | unset |
---|---|
Patch needs improvement: | unset |
comment:21 by , 5 years ago
Triage Stage: | Accepted → Ready for checkin |
---|---|
Type: | New feature → Cleanup/optimization |
comment:25 by , 4 years ago
Has patch: | unset |
---|---|
Resolution: | fixed → wontfix |
Triage Stage: | Ready for checkin → Unreviewed |
Marked as wontfix
because we don't have a clear deprecation path (see #31877).
Setting this DDN pending comment from someone who uses class-based views. Seems like a new feature rather than a bug, in any case.