Opened 8 years ago
Last modified 7 years ago
#27317 new New feature
Make Form subclasses combine Form.Media from all parents
Reported by: | James Pic | Owned by: | nobody |
---|---|---|---|
Component: | Forms | Version: | 1.10 |
Severity: | Normal | Keywords: | |
Cc: | James Pic | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I wanted to define a mixin to use with Form classes that would add extra JS. So, I tried how Form.Media would behave with inheritance, and it turns out they don't merge. For example:
class A(Form): class Media: js = ('jquery.js', 'a.js') class B(Form): class Media: js = ('jquery.js', 'b.js') class C(A, B): pass
I expected:
C.Media.js == ('jquery.js', 'a.js', 'b.js')
But that results instead in:
C.Media.js = ('jquery.js', 'a.js')
Eventually, I wanted to push it a bit further and use a Mixin, having exactly the same but with A(object)
instead of A(Form)
.
Can I work on a patch for this ?
Thanks for your support
Change History (7)
comment:1 by , 8 years ago
Type: | Bug → New feature |
---|
comment:2 by , 8 years ago
Summary: | Form.Media inheritance → Make Form subclasses combine Form.Media from all parents |
---|
comment:3 by , 8 years ago
Thanks for re-centering the discussion. I understand this design decision.
As the documentation says about Meta, "Normal Python name resolution rules apply. If you have multiple base classes that declare a Meta inner class, only the first one will be used.
Oh, I was talking about Media... I don't see any case where you'd want to render a widget or form without a certain script that it requires, users would just end with broken forms :D
But, let's admit that such a paralogism is valid, even then, explicit inheritance doesn't fix it:
In [3]: from django.forms.forms import Media In [4]: class MediaA(Media): ...: js = ('jquery.js', 'a.js') ...: In [5]: In [5]: class MediaB(Media): ...: js = ('jquery.js', 'b.js') ...: In [6]: class MediaC(MediaA, MediaB): ...: pass ...: In [7]: MediaC.js Out[7]: ('jquery.js', 'a.js')
Shouldn't we be expecting MediaC.js = ('jquery.js', 'a.js', 'b.js')
here ? I can't think of any case where we wouldn't: this multiple inheritance is after all, explicit.
By the way, we're thinking about ways to replace Media with a better solution, although no solution seems imminent (#22298).
There's no hurry, I got a solution that works until them: using a Widget to render additional script without breaking the user Form class yaayyyy :D it will be easy to change when we have another solution.
Meanwhile, should we fix inheritance in Media.js and Media.css attributes ?
comment:4 by , 8 years ago
Triage Stage: | Unreviewed → Accepted |
---|
I don't know, I guess we could see what the code looks like. I'm just wary of investing time in something that's discouraged and/or headed toward deprecation. Of course, I guess there are people love it.
comment:5 by , 7 years ago
Yes, because currently we can not know if we should or not add a media. For example this would allow it:
class YourForm(forms.Form): def add_media(self, media): if not media.js.find('jquery.js'): # might have been added from another app? media.js.add('/yourapp/jquery.js') media.js.add('b.js')
Also, probably we should be limited to one Media object per request.
comment:6 by , 7 years ago
Absolutely agree with Tim, which is why i didn't invest time on this, but in adapters instead.
Full comment on the corresponding PR: https://github.com/django/django/pull/9743
comment:7 by , 7 years ago
Honestly, I would prefer the "normal" inheritance, where attributes overwrite one another. I am not a big fan of the magic Django does in Model.Meta
. Its drives complexity and is inconsistent too. It makes migrations and the state builder a pain... anyways. Forms. I do want to repeat the same mistakes here.
Python overwrites attributes by default and I for one would want to keep it that way.
I'm not sure if that's a good idea. As the documentation says about Meta, "Normal Python name resolution rules apply. If you have multiple base classes that declare a Meta inner class, only the first one will be used. This means the child’s Meta, if it exists, otherwise the Meta of the first parent, etc." There was a slightly related thread on django-developers were it was discussed that model Meta inheritance should be explicit rather than automatic. Can you think of any precedents in Django where automatic merging similar to what you proposed takes place?
By the way, we're thinking about ways to replace
Media
with a better solution, although no solution seems imminent (#22298).