FIX: Use Terser for minification even if uglify-js is not available (PR #13683)

In #12656, the default js minifier was switched to Terser from uglify-js, but in order for Terser to be used, uglify-js still has to be around. This fixes that.

GitHub

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Thanks @talyz, there is a TODO on line 105 in this file

# TODO: Remove uglifyjs when base image only includes terser

Since the base image now has terser, we can simply remove support for uglifyjs entirely. If you want to make that change in this PR, I’m happy to approve.

Sure! Maybe I should say that I’m not actually using the docker image - I maintain the discourse package for NixOS and its accompanying module (deployment scripts, essentially). In that package, we build the assets in a separate sandboxed build, so they’re part of the package and not built on deployment. That’s how I found out about this bug to begin with. Removing uglifyjs would add some extra work when backporting future releases to our stable branch, since we don’t currently have terser in stable, but it’s definitely solvable.

Added a fixup now. If it looks good to you and you prefer to remove uglifyjs, I’ll squash it. :slight_smile:

This looks good @talyz, I’ll just wait for the tests to pass (they should) and then I’ll approve. I’ll handle the squashing when merging as well.

(Sorry to add work to your plate by removing uglifyjs, but it’s something we were eventually going to do, anyway.)

Great! Yeah, it’s no big deal, really. :slight_smile:

Changes look good to me so I’m merging this. Thank you for your contribution @talyz