#11673 closed defect (wontfix)
[patch] problem with shrinksafe and recursing using named functions
Reported by: | ben hockey | Owned by: | Richard Backhouse |
---|---|---|---|
Priority: | low | Milestone: | tbd |
Component: | ShrinkSafe | Version: | 1.5 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description
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]);
Attachments (2)
Change History (12)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
Seems as if the problem even occurs if you use the same name, like var myFunction = function myFunction(){...
see #8691
Changed 10 years ago by
Attachment: | shrinksafe.named_functions.patch added |
---|
Include named functions in the ShrinkSafe? minifications
comment:3 Changed 10 years ago by
I've attached a patch (with test cases) to include named functions in the compression. All (now 13) tests pass.
Changed 10 years ago by
Attachment: | shrinksafe.cleanup.patch added |
---|
Fix the ShrinkSafe? build script launching tests; some refactoring/cleanup.
comment:5 Changed 9 years ago by
Resolution: | → patchwelcome |
---|---|
Status: | new → closed |
I think closure will be the compressor of choice going forwards, and we don't have anyone working on shrinksafe, so closing this ticket.
comment:6 Changed 9 years ago by
While closure is now the compressor of choice, it looks like shrinksafe is still available as an option. So why not integrate these patches?
comment:7 Changed 9 years ago by
Resolution: | patchwelcome |
---|---|
Status: | closed → reopened |
Summary: | problem with shrinksafe and recursing using named functions → [patch] problem with shrinksafe and recursing using named functions |
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.
comment:8 Changed 9 years ago by
Priority: | high → low |
---|
@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.
comment:9 Changed 8 years ago by
Resolution: | → wontfix |
---|---|
Status: | reopened → closed |
Shrinksafe is deprecated in favor of optimization using closure.
Same problem, one more exampe:
becomes
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!