[ccla][patch]Shrinksafe could munge strings together

[dante]

It has been suggested in the forums that shrinksafe be enhanced to handle removing useless string concatenation as a result of newlines:

var a = "b" +
      "c" +
      "d";

should just be var a = "bcd";

but should not fail on: var a = "b" + c + "d" + "e";

should be: var a = "b"+c+"de";

patch and unit tests attached.

[dante]

updated patch adds new unit test

[Adam Peller]

would it be possible/preferable to do this with tokens rather than a full pass with a regexp?

[dante]

so this isn't safe yet.

[dante]

var a = "+"; fails, and is an interesting issue.

[Adam Peller]

concat strings in compress method from tokens

[dante]

@peller - thanks for this patch, it's great! seems to work perfectly. I've got most of a new unit test going, trying to think of more cases to check. Will likely commit patch and tests this evening.

[dante]

let a sleeping trac, sleep. fixed in [19078]

[Adam Peller]

hmm... what I did with the tokenizer is right 99% of the time, but not strictly valid.

!"a"+"b"

should yield

"falseb"

not

false

or !"ab" Perhaps we really need an AST to perform these types of optimizations?

[Adam Peller]

A regression against my own patch. Granted, it looks like an edge case, but Shrinksafe ought to be safe (this could end up being a frustrating typo that shrinksafe would mask)

[dante]

I tested:

(function(){
	var x = !"a" + "b";
})();

with YUICompressor, and the output was:

(function(){var a=!"ab";})();

so this "bug"/limitation exists there (not justifying the fail, just interesting that it exists there, too.

the overall savings of this patch against dijit-all.js (only templateString inline / pre-build) uses multi-line concatenation like this) is minimal: 60 bytes or so.

I would, however, like to offer this option as a cmd line toggle perhaps? classify this string munging as a "microoptimiztion that could potentially break code". We similarly break on "complex console.* stripping", though neek has a patch attempting to fix that.

[James Burke]

dante: you mention this saves 60 bytes on dijit-all.js, what is the total file size with the 60 bytes? This file: ​http://o.aolcdn.com/dojo/1.3/dijit/dijit-all.js

is 83KB gzipped. So a 60 byte savings is like 0.07% file savings. I would say it is not worth providing the option at all, given the extra possibility of error, the extra explaining of the option, and a new feature to support.

[Eugene Lazutkin]

Replying to jburke:

dante: you mention this saves 60 bytes on dijit-all.js, what is the total file size with the 60 bytes?

To me this optimization is about run-time cost savings: no expensive concatenations. But if it doesn't work in some cases, I think we should skip it to save our users from frustrating debugging efforts. I am saying it as someone who debugged build problems once or twice. :-(

[dante]

(In [19120]) fixes #8828 - backing out regression causing shrinksafe patch, but leaving code commented out as the regression is a super egde case and something someone may way to manually enable for themselves.