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

Remove JS minification from the web applications. #6415

Closed
zenmonkeykstop opened this issue Apr 29, 2022 · 1 comment · Fixed by #6425
Closed

Remove JS minification from the web applications. #6415

zenmonkeykstop opened this issue Apr 29, 2022 · 1 comment · Fixed by #6425
Assignees
Labels
needs/discussion queued up for discussion at future team meeting. Use judiciously.

Comments

@zenmonkeykstop
Copy link
Contributor

Description

In order to improve performance of the web applications, support for minification for JS and CSS files was added in e74b390, using flask-assets, cssmin, and jsmin.

Since then the application has started using SASS for CSS minification (see #2808), meaning only JS is being minified. Only 2 files are currently being minified, static/js/{source,journalist}.js, with savings of <1k and <2k respectively.

To reduce code complexity at the cost of 1-2k in page size, it would make sense to drop JS minification as well. Alternatively, the files could be minified during the build process (tho IMO it's overkill given the minimal gains).

One other disadvantage of server-side minification is that the Apache user needs to be able to write the minified files somewhere on the webroot. Removing it would allow for tightening up the relevant apparmor profile, removing the related write perms

User Research Evidence

n/a

User Stories

n/a

@zenmonkeykstop zenmonkeykstop added the needs/discussion queued up for discussion at future team meeting. Use judiciously. label Apr 29, 2022
@zenmonkeykstop zenmonkeykstop self-assigned this Apr 29, 2022
@legoktm
Copy link
Member

legoktm commented May 2, 2022

Once you factor in gzip, the benefit of minification becomes pretty negligible. For source.js on 2.3.1:

$ du -b securedrop/static/js/source*.js* | sort
1167	securedrop/static/js/source.min.js.gz
1578	securedrop/static/js/source.js.gz
3271	securedrop/static/js/source.min.js
4501	securedrop/static/js/source.js

So +1 to getting rid of this extra complexity. The jsmin package is also unmaintained (c.f. tikitu/jsmin#33 (comment)) and problematic because the version we're on uses a now-removed setuptools option, which I had to workaround in #6358 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs/discussion queued up for discussion at future team meeting. Use judiciously.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants