Make Form subclasses combine Form.Media from all parents

[James Pic]

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

[Tim Graham]

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).

[James Pic]

​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, not Meta... 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 or is there any other result possible from this ?

Can you think of any precedents in Django where automatic merging similar to what you proposed takes place?

I'm thinking that Form.Media inherits from the Widget Media because it's a requirement for the form to function properly. But I might be missing something again like the explicit inheritance design decision in my initial report.

If we have reasons to not merge Meta automatically, we've been merging so for Media so far, and we have the opportunity to improve it.

Anyway, can we still fix explicit inheritance meanwhile ? It doesn't do the trick in the following example:

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: ​https://github.com/yourlabs/django-dynamic-fields/blob/master/src/ddf/form.py#L28

Meanwhile, should we fix inheritance in Media.js and Media.css attributes ?

[Tim Graham]

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.

[James Pic]

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.

[James Pic]

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

[Johannes Maron]

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.