Fixed #35886 -- Moved object-based Script media assets to the public API

[codingjoe]

Trac ticket number

ticket-35886

Branch description

This is a successor to ticket-29490 and a result of the discussion on ticket-22298 and ​its corresponding forum thread.

This ticket aims to move object-based media assets to the public API and documentation.
A sample implementation has been part of the test suite for years, see also:

django/tests/forms_tests/tests/test_media.py

Lines 717 to 771 in 97a6a67

	@html_safe
	class Asset:
	def __init__(self, path):
	self.path = path
	def __eq__(self, other):
	return (self.__class__ == other.__class__ and self.path == other.path) or (
	other.__class__ == str and self.path == other
	)
	def __hash__(self):
	return hash(self.path)
	def __str__(self):
	return self.absolute_path(self.path)
	def absolute_path(self, path):
	"""
	Given a relative or absolute path to a static asset, return an absolute
	path. An absolute path will be returned unchanged while a relative path
	will be passed to django.templatetags.static.static().
	"""
	if path.startswith(("http://", "https://", "/")):
	return path
	return static(path)
	def __repr__(self):
	return f"{self.path!r}"
	class CSS(Asset):
	def __init__(self, path, medium):
	super().__init__(path)
	self.medium = medium
	def __str__(self):
	path = super().__str__()
	return format_html(
	'<link href="{}" media="{}" rel="stylesheet">',
	self.absolute_path(path),
	self.medium,
	)
	class JS(Asset):
	def __init__(self, path, integrity=None):
	super().__init__(path)
	self.integrity = integrity or ""
	def __str__(self, integrity=None):
	path = super().__str__()
	template = '<script src="{}"%s></script>' % (
	' integrity="{}"' if self.integrity else "{}"
	)
	return format_html(template, self.absolute_path(path), self.integrity)

The goal is to support form assets with modern attributes, like:

<script type="module"></script>
<!-- or -->
<script defer></script>
<!-- or -->
<script async></script>

And other standardized attributes, see ​https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script

The same can be done for stylesheets via the link tag, which does support many more attributes than media, see also: ​https://developer.mozilla.org/en-US/docs/Web/HTML/Element/link

Checklist

• This PR targets the main branch.
• The commit message is written in past tense, mentions the ticket number, and ends with a period.
• I have checked the "Has patch" ticket flag in the Trac system.
• I have added or updated relevant tests.
• I have added or updated relevant docs, including release notes if applicable.
• I have attached screenshots in both light and dark modes for any UI changes.

Considerations & Scope

Defining CSS in dictionaries is moderately awkward with the new objects. It could be considered to move to a flat list going forward. However, this should be done in separate ticket.

[matthiask]

Thanks for working on this, this looks very good.

I think adding a test which adds a data attribute to a JS instance would be nice. The current API doesn't allow attributes with dashes since it uses **kwargs to pass attributes though, so that's not possible. I'm using a dictionary as a second argument instead of the **kwargs in django-js-asset so that things like this are possible:

https://github.com/matthiask/django-admin-ordering/blob/6f18be51851cfff3ad4dc8f53fb65af0d896a850/admin_ordering/admin.py#L58-L68

            js=[
                "admin/js/jquery.init.js",
                "admin_ordering/jquery-ui.min.js",
                JS(
                    "admin_ordering/admin_ordering.js",
                    {
                        "class": "admin-ordering-context",
                        "data-context": json.dumps(context),
                    },
                ),
            ],

I think that's a useful thing and it would be nice if the new JS object would support something like this as well. I wrote a blog post a few years back why I did it this way:
https://406.ch/writing/django-admin-apps-and-content-security-policy-compliance/

[codingjoe]

Thanks, @matthiask, I truly appreciate that you took the time to review this. I am also happy to see that many people care out this. Let's address some of your points in writing:

I wouldn't want to pass a dictionary to a method, to any method, really. As dictionaries are mutable in Python, it's more common to unpack and shallow copy them. That being said, your use case is easily solvable, like so:

CSS("path/to/file", **{"async": True, "date-foo": "bar"})

The only difference being **. You noticed is assertIsNot test. It's precisely there for that reason, to make sure we didn't pass a mutable object.

I will extend the documentation to accommodate more use cases.

[matthiask]

Thanks, @matthiask, I truly appreciate that you took the time to review this. I am also happy to see that many people care out this. Let's address some of your points in writing:

I wouldn't want to pass a dictionary to a method, to any method, really. As dictionaries are mutable in Python, it's more common to unpack and shallow copy them. That being said, your use case is easily solvable, like so:

CSS("path/to/file", **{"async": True, "date-foo": "bar"})

Ah yes, I always forget this is possible!

[amureki]

Love it! I left some minor notes.

[sarahboyce]

Thank you @codingjoe 👍 I have a couple of suggestions

[codingjoe]

Thank y'all for your review. Much appreciated 🙏
I hope to have addressed everything carefully.

[amureki]

👍 👍

[sarahboyce]

Thank you for your work and patience on this @codingjoe ⭐ this looks good to me.

I'm going on holiday and won't merge this till the new year 🎄
That also gives folks the opportunity to shout if they spot something 👀

[codingjoe]

Thank you for your work and patience on this @codingjoe ⭐ this looks good to me.

I'm going on holiday and won't merge this till the new year 🎄 That also gives folks the opportunity to shout if they spot something 👀

Thank you so much for your support @sarahboyce. What a wonderful Christmas gift. I hope you have a jolly good holiday yourself 🎄✨☃️