[Json-rpc-java] jsonrpc.js and Dojo shrinksafe

[Arthur Blake]

Here is the function that gets shrunk incorrectly by Doko Shrinksafe (there
may be more, but this is the only one that I've detected so far):

toJSON = function toJSON(o)
{
    if(o == null) {
        return "null";
    } else if(o.constructor == String) {
        return escapeJSONString(o);
    } else if(o.constructor == Number) {
        return o.toString();
    } else if(o.constructor == Boolean) {
        return o.toString();
    } else if(o.constructor == Date) {
        return '{javaClass: "java.util.Date", time: ' + o.valueOf() +'}';
    } else if(o.constructor == Array) {
        var v = [];
        for(var i = 0; i < o.length; i++) v.push(toJSON(o[i]));
        return "[" + v.join(", ") + "]";
    } else {
        var v = [];
        for(attr in o) {
            if(o[attr] == null) v.push("\"" + attr + "\": null");
            else if(typeof o[attr] == "function"); /* skip */
            else v.push(escapeJSONString(attr) + ": " + toJSON(o[attr]));
        }
        return "{" + v.join(", ") + "}";
    }
};

Gets shrunk to:

toJSON=function toJSON(o){
if(o==null){
return "null";
}else{
if(o.constructor==String){
return escapeJSONString(o);
}else{
if(o.constructor==Number){
return o.toString();
}else{
if(o.constructor==Boolean){
return o.toString();
}else{
if(o.constructor==Date){
return "{javaClass: \"java.util.Date\", time: "+o.valueOf()+"}";
}else{
if(o.constructor==Array){
var v=[];
for(var i=0;i<o.length;i++){
v.push(_a(o[i]));
}
return "["+v.join(", ")+"]";
}else{
var v=[];
for(attr in o){
if(o[attr]==null){
v.push("\""+attr+"\": null");
}else{
if(typeof o[attr]=="function"){
}else{
v.push(escapeJSONString(attr)+": "+_a(o[attr]));
}
}
}
return "{"+v.join(", ")+"}";
}
}
}
}
}
}
};

It's obviously a bit hard to read, but the errors are on these lines:

v.push(_a(o[i]));

v.push(escapeJSONString(attr)+": "+_a(o[attr]));

the _a in the above lines was a recursive call to the toJSON function ,
which Shrinksafe inexplicably replaced with "_a".
The only way I found this is with the firebug debugger.  And I came up with
the proposed solution by accident...
There may be another solution (one solution would obviously be "don't use
Dojo shrinksafe" :) )

So- the issue may have something to do with the local variable substitution
used in Dojo shrinksafe combined with recursive calls.

The original function does appear to be legal javascript...   maybe it also
has something to do with the variable and function name being the same (in
other places they were varied slightly)
e.g. _ctor added to the end, etc.

I understand the benefit of wanting to be able to completely reload/replace
the contents of the jsonrpc.js library "on the fly"
especially during development and testing of new features -- I hadn't
thought of this.

I agree with what you said about the extra semicolons.

I'll hold off on the check in for now.  I want to play with it and think
about it some more.

What are the other javascript compression programs you mentioned?  I have
only used JSMin and Dojo Shrinksafe up to this point.  I believe these are
the two most popular ones...

On 8/1/07, Michael Clark <michael at metaparadigm.com> wrote:
>
> Arthur Blake wrote:
> > For example,
> > I changed:
> >
> > toJSON = function toJSON(o)
> >
> > to just
> >
> > function toJSON(o)
> >
> > and
> >
> > JSONRpcClient.Exception =
> > function JSONRpcClient_Exception_ctor(code, message, javaStack)
> >
> > to just
> >
> > JSONRpcClient.Exception = function (code, message, javaStack)
> >
> > All the functions are defined redundantly this way, and I fixed them all
> in
> > my copy of jsonrpc.js.  I've been running this way for quite some time,
> and
> > I haven't encountered any errors as a result of running this way.
> > I can't think of any good reason why these redundant function
> definitions
> > are there in the first place... and when I remove them, Dojo Shrinksafe
> > properly shrinks the file.
> > I'd like to commit these changes if everyone agrees this is a good
> idea...
> >
>
> From memory I think the current form of variable declaration was to
> allow the code to be sourced twice and not encounter errors the second
> time. From memory, the short-hand function declaration will error if a
> function of that name is already declared.
>
> The long form variable declaration style of function assignment was
> supposed to allow a modified version of the code (for instance being
> changed during testing) to be loaded dynamically with an XMLHttpRequest
> and eval type script loader. But I'm not sure if it is being used like
> this anywhere.
>
> The functions are also named (hence the 2nd apparently redundant name)
> as the extra name was needed for this style  of declaration to get the
> function names to show up in the debugger (otherwise they show as
> anonymous).
>
> The semicolons are also needed with this style of assignment and they
> won't be needed after the conversion (but will on the ones you have left
> with the variable style assignment such as JSONRpcClient.Exception).
>
> I'm curious what the compressor is actually tripping up on. Perhaps it
> is just the named functions (what you had removed in your
> JSONRpcClient.Exception example above). Then the minimal change is to
> just remove the redundant names (from what you say it apparently worked
> with the variable assignment type declaration for
> JSONRpcClient.Exception so it should for the others too).
>
> If it makes things easier go ahead and commit it although I'm curious as
> to what the compressor needs as I believe the declarations are valid JS.
> (BTW the ";" was added to the end of all of the declarations to make
> them work nicely with other JS compressors - perhaps there was one
> missing?)
>
> Michael.
>