Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#31524 closed Cleanup/optimization (fixed)

Stop minifying only some admin static assets

Reported by: Jon Dufresne Owned by: nobody
Component: contrib.admin Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Here is a list of JavaScript files in the admin app and their size:

  20K django/contrib/admin/static/admin/js/admin/DateTimeShortcuts.js
  15K django/contrib/admin/static/admin/js/inlines.js
  13K django/contrib/admin/static/admin/js/SelectFilter2.js
 8.8K django/contrib/admin/static/admin/js/urlify.js
 7.6K django/contrib/admin/static/admin/js/calendar.js
 6.7K django/contrib/admin/static/admin/js/actions.js
 5.9K django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js
 5.4K django/contrib/admin/static/admin/js/core.js
 5.3K django/contrib/admin/static/admin/js/SelectBox.js
 5.2K django/contrib/admin/static/admin/js/inlines.min.js
 3.2K django/contrib/admin/static/admin/js/actions.min.js
 1.9K django/contrib/admin/static/admin/js/collapse.js
 1.5K django/contrib/admin/static/admin/js/prepopulate.js
 1.1K django/contrib/admin/static/admin/js/autocomplete.js
  911 django/contrib/admin/static/admin/js/collapse.min.js
  878 django/contrib/admin/static/admin/js/cancel.js
  674 django/contrib/admin/static/admin/js/change_form.js
  569 django/contrib/admin/static/admin/js/popup_response.js
  495 django/contrib/admin/static/admin/js/prepopulate_init.js
  379 django/contrib/admin/static/admin/js/prepopulate.min.js
  363 django/contrib/admin/static/admin/js/jquery.init.js

Some things to notice:

  1. Only 4 out of 17 files are minified.
  2. The largest file, DateTimeShortcuts.js, is only 20 KB.
  3. The largest file is also not included in the list of minified files.
  4. All uncompressed files are smaller than the size of the 3 font assets, each ~80 KB.
  5. All uncompressed files are smaller than the minified jQuery, ~87 KB.

I'm not sure if there is a deliberate or historical reason that only a fraction of the static assets are minified, but it looks like it could be an oversight. The approach is inconsistent.

Minifying is step a contributor must manually do. The documentation for doing so is here:

https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/javascript/#javascript-patches

This is a step that is easy to forget, myself included. Whether or not one remembers to compress static assets will also affect the outcome of tests.

I suggest we drop the minificaiton of admin files altogether. For such small files, it doesn't seem worth it to add a build step and inconsistently at that.

In a typical production scenarios, the static assets will be cached and possibly compressed. Compressing static assets largely removes the size gains of minification. Additionally, there are third party apps to fill the role of static asset optimization.

I think we should continue to distribute the vendored libraries minified, however as they are not manually handled during typical contributions.

Change History (6)

comment:2 by Carlton Gibson, 4 years ago

Triage Stage: UnreviewedAccepted

Yes, I think this is reasonable. (It's been slowly bubbling up my nag list for a while so... 👍)

comment:3 by Nick Pope, 4 years ago

There are additional problems that could occur with minifying along with changes rather than at release/build time:

  • It is possible to forget to do it and missing changes are only noticed in production, i.e. with DEBUG = False.
  • Two competing pull requests could minify independently and the one merged second could lose the changes of the first unless rebased and minified correctly.
Last edited 4 years ago by Nick Pope (previous) (diff)

comment:4 by Carlton Gibson, 4 years ago

Triage Stage: AcceptedReady for checkin

comment:5 by Carlton Gibson <carlton@…>, 4 years ago

Resolution: fixed
Status: newclosed

In 81ffeda:

Fixed #31524 -- Removed minified static assets from the admin.

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In 9d211f1:

Refs #31524 -- Moved release notes for 81ffedaacc0d907b9feb73783edefdffd0ced606 to 3.2.

Note: See TracTickets for help on using tickets.
Back to Top