Summary

Switch our JavaScript compressor to UglifyJS.

Review Request #4319 — Created July 11, 2013 and submitted July 11, 2013, 1:45 a.m.

Information

Owner

chipx86

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Switch our JavaScript compressor to UglifyJS.

We were using jsmin for compressing JavaScript, but this has some
annoying license issues, and flat-out breaks with jquery.cookie. It's
also just not very good compared to modern tools.

This change switches to using UglifyJS. Uglify has the advantage of
being used and well-tested by large numbers of projects, including
jQuery. It's easy to install (npm install uglify-js), and since we use
npm for other tools (lessc, require.js soon), it's nice and consistent.

Another good option would have been Closure, but Closure's really mad at
a lot of our code (probably worth looking into anyway). It also seems to
have some inconsistent installation experiences on MacOS, from when I
first installed it. For those reasons, I'm erring in the favor of npm,
given that we already require it. (Hopefully this also make life a
little easier for our packagers.)

With this change, I can build working JavaScript bundles again.

Testing Done

Hand-inspected our bundles.

Deployed to reviews.reviewboard.org (since the deployment was broken due
to the jsmin </3 jquery.cookie issue), and everything seems to be working
fine.

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    reviewboard/settings.py
  Ignored Files:

This is a review from Review Bot.
  Tool: Pyflakes
  Processed Files:
    reviewboard/settings.py
  Ignored Files:

Ship it!

```
Ship It!
```

Status:: Completed

Not happy about this change. This introduces huge dependency chain on nodejs that Reviewboard didn't have before. During an upgrade from 1.7.15 I ran into

....
pipeline.compressors.CompressorError: ('Unable to pipe content to command: /usr/bin/env uglifyjs -nc ', IOError(32, 'Broken pipe'))

Because there is now a dependency on having nodejs installed. This creates all kinds of security issues with Reviewboard.

I would strongly recommend finding a way around having this as a required dependency for doing an upgrade.

chipx86 May 2, 2014, 1:36 p.m.

We already need node.js for lesscss. However, this is only necessary for package building time. You should never see any of this unless you are building your own packages, which is more of a specialized thing.
I'd like to get a sense for what you're doing and why you need to build these on your end.
However, we will not be reverting this change, since again, lesscss is a solid requirement that will remain, and also requires node.js.

ascotan May 5, 2014, 6:58 a.m.

False alarm. I was able to do the upgrade from an .egg without the need for node.js. I was attempting an easy install with the .tar.gz on pypi (which does require nodejs). My mistake. As long as I can upgrade from pypi without the need to have nodejs installed I'm happy.

chipx86 May 5, 2014, 12:07 p.m.

Glad to hear it :) Installing eggs is the preferred way to get RB, and we discourage installing tarballs for this reason.

Ship it!

```
Ship It!
```