[patch] problem with shrinksafe and recursing using named functions

[ben hockey]

shrinksafe has a problem with the following input...

var clone = function recurse(it) {
        var d = dojo,
                prop,
                ret = it && d.isArray(it) ? [] : {};

        if (it && d.isObject(it)) {
                for (prop in it) {
                        if (it.hasOwnProperty(prop)) {
                                ret[prop] = recurse(it[prop]);
                        }
                }
        }
        else {
                ret = it;
        }
        return ret;
};

the output is:

var clone=function recurse(it){
var d=dojo,_1,_2=it&&d.isArray(it)?[]:{};
if(it&&d.isObject(it)){
for(_1 in it){
if(it.hasOwnProperty(_1)){
_2[_1]=_3(it[_1]);
}
}
}else{
_2=it;
}
return _2;
};

the problem is this line _2[_1]=_3(it[_1]); should be _2[_1]=recurse(it[_1]);

[xMartin]

Same problem, one more exampe:

(function myFunction(x){
	x++;
	console.log(x);
	if(x < 10){
		myFunction(x);
	}
})(7);

becomes

(function myFunction(x){
x++;
console.log(x);
if(x<10){
_1(x);
}
})(7);

So you can't use a named function while it is anonymous or has a different name in the containing scope.

In my opinion this is a critical issue as shrinksafe breaks valid JavaScript? code!

[xMartin]

Seems as if the problem even occurs if you use the same name, like var myFunction = function myFunction(){...

see #8691

[Remoun Metyas]

Include named functions in the ShrinkSafe? minifications

[Remoun Metyas]

I've attached a patch (with test cases) to include named functions in the compression. All (now 13) tests pass.

[Remoun Metyas]

Fix the ShrinkSafe? build script launching tests; some refactoring/cleanup.

[ben hockey]

#8691 is a duplicate of this ticket.

[bill]

I think closure will be the compressor of choice going forwards, and we don't have anyone working on shrinksafe, so closing this ticket.

[Remoun Metyas]

While closure is now the compressor of choice, it looks like shrinksafe is still available as an option. So why not integrate these patches?

[bill]

The patches still apply cleanly but when I try to run the regression tests I get an error:

$ ./runner.sh
js: uncaught JavaScript runtime exception: ReferenceError: "define" is not defined.

... so no idea how to test the patch.

@neonstalwart, you want to take it?

Also @rem do you have a CLA? Not sure of your real name so I can't confirm if you are on the list.

[ben hockey]

@bill i haven't touched java code in over 15 years. i don't think now is a good time to start.

for a while i just worked around this bug and now i've switched completely to closure - dead code removal alone was enough of a reason to switch. i'm not willing to put any time into a tool that i'm not going to use or help support.

[bill]

Shrinksafe is deprecated in favor of optimization using closure.

[ben hockey]

#18514 is a duplicate of this ticket.