JQuery UI failure with Requirements:combine_files

[tazzydemon]

This one is well known in the wild although the best solution eludes me. If I do a combine_files including an already minified jquery-ui then there a a point where a function does this:
(this.id="ui-id-"+ ++n)

becoming

(this.id="ui-id-"+++n)

Resulting in Uncaught ReferenceError: Invalid left-hand side expression in postfix operation.

The immediate solution is to wrap the ++n in parentheses but then jquery ui is non standard. The error lies in in jsmin.php as in pilif/sacy#17 and also in similar libraries like. Its been fixed in jshrink:
https://github.com/tedivm/JShrink/issues/19

[tractorcow]

Don't do Requirements::combine_files on the minified version then. ;)

[tazzydemon]

The conclusion I was coming to as well! I merely thought this was worth pointing out!

From: Damian Mooyman [mailto:notifications@github.com]
Sent: Monday, 12 May 2014 10:22 a.m.
To: silverstripe/silverstripe-framework
Cc: tazzydemon
Subject: Re: [silverstripe-framework] JQuery UI failure with Requirements:combine_files (#3126)

Don't do Requirements::combine_files on the minified version then. ;)

—
Reply to this email directly or view it on GitHub #3126 (comment) .Description: Image removed by sender.

[hafriedlander]

There is Requirements_Backend->combine_js_with_jsmin which can disable using jsmin (which is pointless if all your JS is already minified) but it's not set using the standard config system.

Strictly any situation where valid JS is being broken by JSMin is a bug. Given that the library we're using now explicitly recommends using jshrink instead, we should probably look at changing at some point.

[tazzydemon]

I saw 3132 possibly proposed configurable minifiers? This is becoming more of a problem with, for instance, knockout.js v3 which wont combine successfully in either form

[tazzydemon]

Its pretty much anywhere that the javascript has a
variable+ ++variable2

[peavers]

Have just come across this 'error' (are we calling it that?) in 3.1.6. Pretty annoying. Seems to happen occasionally with non-minified files too.

[tazzydemon]

Ok, I read the #3132 above and it leaves me bemused. This seems like a simple issue of changing shrink library. I still have to test and not merge certain js (and in the right order). Its a nuisance!!
And yes it does happen with non minified.

[PonchoPowers]

I'd like to see this issue fixed too, just a simple thing like this can cause chaos when it comes to the release of a website, just spent an hour (sounds like nothing) trying to work this one out, now the client is unhappy and has rejected the work.

[PonchoPowers]

As a temporary solution, add the following to the init method of the Page_Controller class in the mysite folder:
Requirements::backend()->combine_js_with_jsmin = FALSE;

This disables the compression of JavaScript files.

[tractorcow]

I've done a bit of digging and there seems to be a modified version of the (now unmaintained) jsmin.php at https://github.com/mrclay/minify/blob/master/min/lib/JSMin.php, which resolves this issue.

We could possibly update the 3.1 version of this library with the above code, without greatly risking a breaking change by swapping to a new library.

[peavers]

Still seems like a dirty fix. A proper replacement strategy/roadmap should
be put in place.

On Fri Oct 31 2014 at 9:07:21 AM Damian Mooyman notifications@github.com
wrote:

I've done a bit of digging and there seems to be a modified version of the
(now unmaintained) jsmin.php at
https://github.com/mrclay/minify/blob/master/min/lib/JSMin.php, which
resolves this issue.

We could possibly update the 3.1 version of this library with the above
code, without greatly risking a breaking change by swapping to a new
library.

—
Reply to this email directly or view it on GitHub
#3126 (comment)
.

[tazzydemon]

Not if it works. What's there now only works when it feels like it. Yes a proper strategy by all means but an actual fix in the meanwhile would be excellent.

[tazzydemon]

Matthew, disabling it is not a "solution"! I now carefully select what i can and can't compress and require them in the appropriate order. That's a better solution but still unnecessary if the minifier worked.

[tractorcow]

I think that for 3.2 we could look at updating to a new library, but for 3.1 a manual fix to an outdated library is probably fine for now.

[PonchoPowers]

The best solution would be to allow us to be able to override what library is used opposed to just giving us an option to disable it. That way the framework is not dictating what library to use. I personally would rather use Google Closure:
http://closure-compiler.appspot.com/home

Cherry-picking what to compress and what to not compress isn't a good solution with a website that does weekly sprints, so disabling the compression altogether works best for the project I work on.

[kinglozzer]

JSMin has been removed in #6902