Closure compiler treats definitions inside closures as redefinitions

Question

I've been working with google closure, trying to get a large body of JavaScript to compile cleanly for minimization using the Google compiler. I came across a problem though:

goog.provide('test');
goog.provide('test2');

/**
 * @constructor
 */
test = function () {
    this.x = 10;
    this.y = 13;
};

(function () {
    /**
     * @constructor
     */
    test2 = function () {
        this.x = 10;
        this.y = 13;
    };
})();

The former is fine. The latter generates a constant-redefinition error:

JSC_CONSTANT_REASSIGNED_VALUE_ERROR. constant test2 assigned a value more than once at /home/hbrown/tmp/closure-test/foo.js line 16 : 10
BUILD FAILED: 1 error(s), 0 warning(s)

Is there some way to coerce plovr/closure compiler to allow this construct? I've looked around and found nothing.

Later: on a further point, why does closure/plovr consider test2 a constant? I suspect it has to do with plovr/closure's creation of a namespace for test2 when goog.provide is called. it would be nice to see the intermediate form that it is working with when it generates the error.

The Closure Compiler treats global-level object definitions differently from objects defined inside a functional wrapper. It does a lot more optimizations when not inside a wrapper closure. In your case, it will collapse the goog.provide and the definition of test1, so you won't get the error. — Stephen Chung, Sep 22 '11 at 08:27
Is there a reason you're declaring a global constructor test2 inside of a function closure? Should it really be var test2? The format is a little confusing and seems likely to lead to mistakes. I mention this because resolving this awkward syntax will likely also resolve your Closure Compiler issue. — Chris Moschini, Oct 19 '11 at 23:13

score 0 · Answer 1 · answered Sep 21 '11 at 15:04

0

I'm typing this as an answer even though it's just a guess, because comments are terrible for code.

Have you tried something like this:

test2 = (function () {
    /**
     * @constructor
     */
    function inner_test2() {
        this.x = 10;
        this.y = 13;
    };

    // ...

    return inner_test2;
})();

I wouldn't suggest that that's a convenient refactoring, particularly if that anonymous function is big and complicated, but it'd be interesting (sort-of) to see what it is that's confusing the compiler.

answered Sep 21 '11 at 15:04

Pointy

405,095
59
585
614

I have ~900 files that use this idiom. I'd far rather find some way to configure closure/plovr than change a large body of code at this stage of the development. – hughdbrown Sep 21 '11 at 15:09
Yes yes, as I said I was just thinking in terms of experimentation. If nothing else it might help formulate a good bug report :-) I should probably Community Wiki the "answer". – Pointy Sep 21 '11 at 15:50

score 0 · Answer 2 · answered Sep 22 '11 at 00:48

0

Depending on why you need the anonymous function, you might try replace the the anonymous function with a goog.scope

http://closure-library.googlecode.com/svn/docs/closure_goog_base.js.html

answered Sep 22 '11 at 00:48

John

5,443
15
21

I think it's fair to reiterate the comment I made previously on changing the code: "I have ~900 files that use this idiom. I'd far rather find some way to configure closure/plovr than change a large body of code at this stage of the development." Beyond that, I don't get how this addresses my problem. And there are no uses of goog.scope in all of the closure library. – hughdbrown Sep 22 '11 at 12:46
If you don't want to use goog.scope or make other changes, then it may be best to file a bug/feature request: http://code.google.com/p/closure-compiler/issues/list As I recall both recognizing the type declaration in immediately called anonymous closure and making the goog.provide'd name spaces constant are relatively recent changes. It might be worth check the version of the compiler at the top-of-the-tree to see if there are any recent changes to the behavior. – John Sep 26 '11 at 00:08

score 0 · Answer 3 · answered Oct 23 '11 at 03:33

Declare test 2 outside the function closure, without assigning it:

var test2;

(function() {
    test2 = function(...

I realize this isn't a Closure Compiler configuration change like you wanted, but it will both improve code readability and resolve Closure Compiler's objections.

A little bit of what you get with Closure Compiler is really Google internal Javascript code guidelines, because of its history. So for example you can't use the with statement because that's against policy, even though you as a public user just want to minify your code, and may have a policy that allows for the with statement at your company.

That said, I do think it's not the best practice to declare a global inside a function closure (even though it's legal Javascript). And it wouldn't be that hard to write a script that goes around looking for /(\w[\w\d-]+) = function/ and declaring it with var at the top of the offending files. And, it would probably result in all the files modified that way being easier to analyze by coders new to a given file.

Your remaining option is to modify the open source Closure Compiler code so it warns instead of errors about this violation of Google JS policy.

Closure compiler treats definitions inside closures as redefinitions

3 Answers3