Add ability to specify `Widget.attrs` as a class var

[coredumperror]

Code of Conduct

• I agree to follow Django's Code of Conduct

Feature Description

I'd like to be able to specify the attrs value for a subclass of Widget as a class var, in order to make it possible to fully specify a custom widget as a reusable class. Currently, you have to specify attrs as a keyword arg, which is redundant any time you want to use the same Widget subclass multiple times.

Problem

Right now, the only way to specify custom attrs on a Widget is via sending the attrs keyword arg to __init__(self, attrs=None). This makes it impossible to build a reusable widget subclass that specifies custom attrs without doing a fairly gross hack (converting the class var to the attrs keyword arg in the subclass's call to super().__init__()).

The reason you can't just specify WidgetSubclass.attrs right now is that the code in Widget.__init__() overwrites the attrs instance var with whatever was sent to __init__() as the attrs keyword arg.

Request or proposal

proposal

Additional Details

The reason this issue with Widget came up is that I've recently started using django-select2, which allows you to specify quite a bit about your custom select2 widgets via class vars... but you can't specify attrs that way because of what Widget.__init__() does. And you need to specify attrs to send configuration to the select2 javascript library.

Implementation Suggestions

I believe this change to Widget would be sufficient:

class Widget(metaclass=MediaDefiningClass):
    ...
    attrs = {}

    def __init__(self, attrs=None):
        self.attrs = self.attrs | (attrs or {})

The new __init__() will assign to the instance variable attrs a new dictionary which is a union of the class variable attrs and the value sent as the attrs keyword arg (if a value was sent). This allows keyword-provided keys to override those provided in the class var, and also ensures that Widget.attrs, itself, is never modified via changes to instance.attrs.

Note that the dict Union operator was added in Python 3.9, which is before the earliest Python that Django currently supports.

[github-actions]

Thank you coredumperror for sharing your idea! We have a lot of them so please be patient. You can see the current queue here.

Community instructions

For commenters, please use the emoji reactions on the issue to express support, and/or concern easily. Please use the comments to ask questions or contribute knowledge about the idea. It is unhelpful to post comments of "I'd love this" or "What's the state of this?"

Reaction Guide

• 👍 This is something I would use
• 👎 This is something that would cause problems for me or Django
• 😕 I’m indifferent to this
• 🎉 This is an easy win

[codingjoe]

Hi there 👋,

As mentioned in the origin Django-Select2 thread, mutability of dictionaries is something to consider. However, Django uses mutable objects on classes in other places too.

Generally the form rendering is fully template-based now, but the Python API, especially of widgets, shows considerable signs of Django's legacy.

@carltongibson since you are the official form guru in chief, any thoughts or wisdom? I feel this could be an easy win, but maybe I am missing something.

@coredumperror perhaps consider starting a thread in the forum too to find more people for your cause.

Best
Joe

[coredumperror]

Thanks for the suggestion! I've never used the django forum before, so I'm hoping that my post is in the right place, haha.

[shangxiao]

Absolutely a no from me for a solution involving mutable class attributes. Sorry but "Django already does this" is not a good enough reason to keep doing a dangerous thing 🤷‍♂️

I've raised what can happen in the forum, from what I've personally seen newbies do when they're new to programming. The consensus was that yes it's dangerous but there may be people relying on this bad behaviour and it's probably worse to change it now – so that's why this anti-pattern is still in the codebase.

@adamchainz has laid out how to make immutable dictionaries here: https://adamj.eu/tech/2022/01/05/how-to-make-immutable-dict-in-python/ Could there be a solution using this approach?

Edit I should've mentioned: I'll be happy to vote for this but as long as it's using a sound approach 👍

[codingjoe]

Howdy 🤠,

Yes, defaulting to a mutable objection probably isn't a great idea. However, this is easily overcome:

class Widget(metaclass=MediaDefiningClass):
    ...
    attrs = None

    def __init__(self, attrs=None):
        self.attrs = (self.attrs or {}) | (attrs or {})

If someone chooses to use a mutable object, this should be up to the developer. There are good cases to use a mutable object on a class if you want to register things later in your code.

Cheerio!
Joe

[coredumperror]

Is it really a problem that the attrs attribute is mutable, given that __init__() overrides it with a copied instance variable? What's the actual point of danger? Legit question, as I'm fairly ignorant about the arguments against mutable class attributes.

Alternatively, would redefining Widget.attrs to None, and then dealing with that in __init__() so that the instance variable ends up being {}, solve the problem?

[carltongibson]

@carltongibson since you are the official form guru in chief, any thoughts or wisdom? I feel this could be an easy win, but maybe I am missing something.

I think that's likely @smithdc1 😅

IDK how much need there is for a change here. It seems to me a Widget subclass can override __init__ quite easily. ("fairly gross hack" seems overplayed — it's what a couple of lines?)

I do wonder if the widget is the right place for this customisation… Wouldn't a Select2Field be better off customising the bound field, say? 🤔

[codingjoe]

I do wonder if the widget is the right place for this customisation… Wouldn't a Select2Field be better off customising the bound field, say? 🤔

Hm… the library doesn't ship any fields, just widgets. It felt odd to include generic behavior for inheritance that has nothing to do with Select2. I'd say it's either done in Django or not done at all. Whatever the consensus is here will also apply to Select2.

That being said, for me and probably most who are familiar with forms, overriding the constructor works just fine. I think it boils down to what's nicer for newcomers… and that might be highly subjective. That being said, it's also such a small change that, at least as far as I am concerned, anything goes 🤷

🙌 Joe

[coredumperror]

What I meant by a "fairly gross hack" is that you have to read in the values of the attrs class variable (if set) and apply them to the __init__() kwargs. Here's the Widget mixin I wrote to do this for my app's needs:

class MultitenantDS2WidgetMixin:

    def __init__(self, *args, **kwargs):
        # Add 'attrs' to the kwargs if they're set as class vars. Don't override any kwargs that are already set.
        kwargs["attrs"] = {**getattr(self, "attrs", {}), **kwargs.get("attrs", {})}

        # Add our custom default attrs to the widget, if they aren't already set.
        kwargs["attrs"].setdefault("data-ajax--delay", 500)  # Apply a 500ms debounce to user input.
        kwargs["attrs"].setdefault("data-width", "auto")
        kwargs["attrs"].setdefault("data-allow-clear", "true")  # Allow the user to clear the selection.

        super().__init__(*args, **kwargs)

Maybe it's just me, but converting from class vars to constructor kwargs feels like a significant code-smell. I would much prefer if this mixin could simply be:

class MultitenantDS2WidgetMixin:
    attrs = {
        "data-ajax--delay": 500,
        "data-width": "auto",
        "data-allow-clear": "true",
    }