Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance issue with multiple range filters (same resources : scripts + styles loaded multiple times) #116

Open
daweedm opened this issue Mar 15, 2024 · 1 comment

Comments

@daweedm
Copy link

daweedm commented Mar 15, 2024

The DOM loading of list_filters is slowed down only when one ore more date range filters are used.

I think this is related to the scripts <script src=...> (and styles) that are defined in the following template :

<script type="text/javascript" src="{% url 'admin:jsi18n' %}"></script>
<script type="text/javascript" nonce="{{ spec.request.csp_nonce }}">
django.jQuery('document').ready(function () {
django.jQuery('.admindatefilter #{{ choices.0.system_name }}-form input[type="submit"]').click(function(event) {
event.preventDefault();
var form = django.jQuery(this).closest('div.admindatefilter').find('form');
var query_string = django.jQuery('input#{{ choices.0.system_name }}-query-string').val();
var form_data = form.serialize();
var amp = query_string === "?" ? "" : "&"; // avoid leading ?& combination
window.location = window.location.pathname + query_string + amp + form_data;
});
django.jQuery('.admindatefilter #{{ choices.0.system_name }}-form input[type="reset"]').click(function() {
var form = django.jQuery(this).closest('div.admindatefilter').find('form');
var query_string = form.find('input#{{ choices.0.system_name }}-query-string').val();
window.location = window.location.pathname + query_string;
});
});
{% comment %}
// Code below makes sure that the DateTimeShortcuts.js is loaded exactly once
// regardless the presence of AdminDateWidget
// How it worked:
// - First Django loads the model formset with predefined widgets for different
// field types. If there's a date based field, then it loads the AdminDateWidget
// and it's required media to context under {{media.js}} in admin/change_list.html.
// (Note: it accumulates media in django.forms.widgets.Media object,
// which prevents duplicates, but the DateRangeFilter is not included yet
// since it's not model field related.
// List of predefined widgets is in django.contrib.admin.options.FORMFIELD_FOR_DBFIELD_DEFAULTS)
// - After that Django starts rendering forms, which have the {{form.media}}
// tag. Only then the DjangoRangeFilter.get_media is called and rendered,
// which creates the duplicates.
// How it works:
// - first step is the same, if there's a AdminDateWidget to be loaded then
// nothing changes
// - DOM gets rendered and if the AdminDateWidget was rendered then
// the DateTimeShortcuts.js is initiated which sets the window.DateTimeShortcuts.
// Otherwise, the window.DateTimeShortcuts is undefined.
// - The lines below check if the DateTimeShortcuts has been set and if not
// then the DateTimeShortcuts.js and calendar.js is rendered
//
// https://github.com/silentsokolov/django-admin-rangefilter/issues/9
//
// Django 2.1
// https://github.com/silentsokolov/django-admin-rangefilter/issues/21
{% endcomment %}
function embedScript(url) {
return new Promise(function pr(resolve, reject) {
var newScript = document.createElement("script");
newScript.type = "text/javascript";
newScript.src = url;
newScript.onload = resolve;
if ("{{ spec.request.csp_nonce }}" !== "") {
newScript.setAttribute("nonce", "{{ spec.request.csp_nonce }}");
}
document.head.appendChild(newScript);
});
}
django.jQuery('document').ready(function () {
if (!('DateTimeShortcuts' in window)) {
var promiseList = [];
{% for m in spec.form.js %}
promiseList.push(embedScript("{{ m }}"));
{% endfor %}
Promise.all(promiseList).then(function() {
django.jQuery('.datetimeshortcuts').remove();
if ('DateTimeShortcuts' in window) {
window.DateTimeShortcuts.init();
}
});
}
});
</script>

This template is loaded for every "instance" of date filter and make the browser download the same scripts & css multiple times, which slows down the DOM rendering (mostly because of the repetition of scripts inside the HTML document).

A solution for this would be to define a Mixin for the Admin class which defines the needed resources only once using the Media subclass :

class RangeFilterMixin(admin.ModelAdmin):
    class Media:
        css = {
            'all': [
                'path/to/some.css'
            ]
        }

        js = [
            'path/to/some.js'
        ]

I can make a PR, but this will introduce breaking changes, since the inline scripts & resources will be removed from the date_filter.html template and the Mixin will be required to make the filter work.

What do you think about it ? Maybe you have another solution for this ?

@daweedm daweedm changed the title Performance issue with multiple range filters (same resources : scripts + styles loaded multiples times) Performance issue with multiple range filters (same resources : scripts + styles loaded multiple times) Mar 15, 2024
@silentsokolov
Copy link
Owner

A solution for this would be to define a Mixin for the Admin class which defines the needed resources only once using the Media subclass.

I think you could try improve OnceCallMedia

Mixin will be required to make the filter work

I don't think that is a good idea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants