Re: [closure-compiler-discuss] Variable assignment issue / incorrect flatening of an object

[Nick Santos]

As far as I know, nothing in the compiler should try to take a local
variable in a constructor and make it a global variable.

There are a couple other things in your email that sound similarly
fishy, but it's hard to unwind them unless we can see what you're
seeing. Would you be comfortable sending me exact repro steps (e.g.,
source code, etc) off-list so that I can see if anything weird is
happening?

Also worth pointing out: The only real semantic difference between
debug mode and non-debug mode is that the compiler gives longer names
to variables and properties. If the code works when you turn debug
mode on, but doesn't work when you turn debug mode off, that usually
means you've got a property conflict. i.e., you're accessing some
property like
window[x] = ...;
and the property "x" that you're choosing is conflicting with a
variable that the compiler is choosing for something else.

This can happen if you're using [] access with a window, or if you've
got other JS code on your page that the compiler can't see. A good way
to diagnose this issue is to open up your debugger and add a
breakpoint for the property you're interested in. e.g.,
Object.defineProperty(obj, 'x', {set: function() { debugger; }});
and see who is re-assigning it.

Nick

On Thu, Jul 19, 2012 at 3:33 AM, <ryan.mc...@gmail.com> wrote:
> I have an issue with closure compiler in advanced mode with variable
> assignments.
>
> I have a flag called has_size_changed (var has_size_changed = false) I
> iterate through a for loop and if the size changes I set has_size_changed =
> true: so
>
>
> /**
> * @constructor
> */
> var myObject = function( ) {
> var has_size_changed = false;
>
> this.doSomething = function( ) {
> ...
> if( condition_that_returns_true ) {
> has_size_changed = true
> }
> }
>
> this.checkSomething = function( ) {
> return has_size_changed;
> }
> }
>
> var foobah = new myObject( );
> foobah.doSomething( );
> alert( foobah.checkSomething( ) );
>
> The interesting thing is that if I assign has_size_changed the value of true
> then it returns correctly, however if I leave it with the value false then
> it returns an unrelated function and not false as I would expect.
> has_size_changed is not assigned elsewhere, it is in an if statement further
> down in the function if( has_size_changed ) { ... } and that is it.
>
> I've had this issue before when a variable was assigned null it returned the
> same function, when I gave it a default state of 0 however it returned
> correctly.
>
> the variable has_size_changed does not appear in the map file if I output a
> variable_map_output_file
>
> If I compile in advanced mode with debug on then the code behaves normally,
> if I compile in simple mode then the code behaves normally, it only
> missbehaves in advanced mode with debug off.
>
> This is a very complex application and I could not replicate this issue with
> simpler code, why is closure compiler doing this? Is it a problem with the
> way that I am assigning variables? It's in a constructor function that has
> been initialised as a new object.
>
> The scope of this seems correct (logging instanceof this before and after
> every assignment) however looking at the code it appears as though the
> variable has_size_changed within the object is actually flattened to a
> global variable (in this case it appears to be p=-1 at the very top of the
> compiled code) , which causes the issue.
>
> If I change var has_size_changed to this.has_size_changed then all hell
> breaks loose and when I create a new instance of the object that
> this.height_has_has_size_changed is in none of its methods are available.
>
> It appears as though I have scope issues for my variables within my objects
> and I am wondering what I am doing wrong, if I am not following established
> practices, or if this is a bug in the compiler. I'm really concerned about
> scope issues within the project further down the track that I may not be
> picking up now.
>
> I'm running version 20120710
>
> Thanks
>
> - Ryan

[Ryan McCarvill]

The problem is that the project is 4 1/2 thousand lines of code at the moment, I tried to isolate only the code and objects that pertain to the problem however that compiled and ran correctly - in fact I find that if I even modify the code slightly it works, for instance if I leave everything as is but initialise var has_sized_change = 0 rather than = false then it compiles and works as expected. Changing arbitrary things in other files also fixes it, either there is something that I am doing that is completely abnormal and is breaking the AST or there is a bug in closure compiler advanced mode. The fact that I have no idea what it is doing really frightens me.

Uglifyjs is about 20% bigger but doesn't break the code, and doesn't seem to be as much of a productivity killer as closure is - I think I will keep compiling my code to closure compiler to make sure that it compiles OK / check my typing, etc, but test and deploy in uglify - hopefully one day closure compiler will work, if not...

It's a shame because you obviously want to squeeze as many bytes as you can out of the minifier, however it's not worth going grey over.

Thanks for your help

- Ryan