Compiler does not work properly after v20201102

[Daijobou]

I use https://github.com/jackmoore/colorbox and my project includes jquery.colorbox.js (as example)
In the past I use a compiler from 2015 with

java -jar compiler.2015.jar --language_out=ECMASCRIPT6 --compilation_level SIMPLE_OPTIMIZATIONS --js jquery.colorbox.js --js_output_file output1.js

Now I updated to compiler version v20210907

java -jar closure-compiler-v20210907.jar --language_out=ECMASCRIPT_2015 --compilation_level SIMPLE --js jquery.colorbox.js --js_output_file output2.js

EDIT: With closure-compiler-v20201102.jar its working

I get no warnings or errors in console, but the demo not working with "output2" but working fine with "output1". Something the compiler is doing is optimizing incorrectly. I get the overlay and box, but no image is included.

output1

output2

[Daijobou]

I checked the versions https://mvnrepository.com/artifact/com.google.javascript/closure-compiler

last working version is "closure-compiler-v20201102.jar" all releases after are not working with this plugin (colorbox) anymore.

[lvelden]

I reproduced this and diff'ed the two compiler outputs from

java -jar closure-compiler-v20201207.jar --language_out=ECMASCRIPT6 --compilation_level SIMPLE_OPTIMIZATIONS --js jquery.colorbox.js --js_output_file output1.js --debug --formatting=PRETTY_PRINT

vs.

java -jar closure-compiler-v20201102.jar --language_out=ECMASCRIPT_2015 --compilation_level SIMPLE --js jquery.colorbox.js --debug --js_output_file output2.js --formatting=PRETTY_PRINT

After stripping away number, this results in:

$ diff output1.js output2.js
<     $publicMethod$$.prep = function($JSCompiler_temp_const$jscomp$ $$) {
---
>     $publicMethod$$.prep = function($object$$) {
326,333c326,335
<         $$loaded$$ = $$tag$$("div", "LoadedContent").append($JSCompiler_temp_const$jscomp$ $$);
<         $JSCompiler_temp_const$jscomp$ $$ = $$content$$;
<         $settings$$.h = $settings$$.h || $$loaded$$.height();
<         $settings$$.h = $settings$$.mh && $settings$$.mh < $settings$$.h ? $settings$$.mh : $settings$$.h;
<         var $JSCompiler_temp_const$$ = {height:$settings$$.h}, $JSCompiler_temp_const$jscomp$ $$ = $$loaded$$.hide().appendTo($$loadingBay$$.show()), $JSCompiler_temp_const$jscomp$ $$ = $JSCompiler_temp_const$jscomp$ $$.css;
<         $settings$$.w = $settings$$.w || $$loaded$$.width();
<         $settings$$.w = $settings$$.mw && $settings$$.mw < $settings$$.w ? $settings$$.mw : $settings$$.w;
<         $JSCompiler_temp_const$jscomp$ $$.call($JSCompiler_temp_const$jscomp$ $$, {width:$settings$$.w, overflow:$settings$$.get("scrolling") ? "auto" : "hidden"}).css($JSCompiler_temp_const$$).prependTo($JSCompiler_temp_const$jscomp$ $$);
---
>         $$loaded$$ = $$tag$$("div", "LoadedContent").append($object$$);
>         $$loaded$$.hide().appendTo($$loadingBay$$.show()).css({width:function() {
>           $settings$$.w = $settings$$.w || $$loaded$$.width();
>           $settings$$.w = $settings$$.mw && $settings$$.mw < $settings$$.w ? $settings$$.mw : $settings$$.w;
>           return $settings$$.w;
>         }(), overflow:$settings$$.get("scrolling") ? "auto" : "hidden"}).css({height:function() {
>           $settings$$.h = $settings$$.h || $$loaded$$.height();
>           $settings$$.h = $settings$$.mh && $settings$$.mh < $settings$$.h ? $settings$$.mh : $settings$$.h;
>           return $settings$$.h;
>         }()}).prependTo($$content$$);

As something around "LoadedContent" changed, this looks related to me.

[concavelenz]

It does look like there is additional inlining occuring. Which might mean it is related to this change:
6bfa548

[concavelenz]

Manually reformatting:

         $$loaded$$ = $$tag$$("div", "LoadedContent").append($JSCompiler_temp_const$jscomp$ $$);
         $JSCompiler_temp_const$jscomp$ $$ = $$content$$;
         $settings$$.h = $settings$$.h || $$loaded$$.height();
         $settings$$.h = $settings$$.mh && $settings$$.mh < $settings$$.h ? $settings$$.mh : $settings$$.h;
         var 
            $JSCompiler_temp_const$$ = {height:$settings$$.h}, 
            $JSCompiler_temp_const$jscomp$ $$ = $$loaded$$.hide().appendTo($$loadingBay$$.show()), 
            $JSCompiler_temp_const$jscomp$ $$ = $JSCompiler_temp_const$jscomp$ $$.css;
         $settings$$.w = $settings$$.w || $$loaded$$.width();
         $settings$$.w = $settings$$.mw && $settings$$.mw < $settings$$.w ? $settings$$.mw : $settings$$.w;
         $JSCompiler_temp_const$jscomp$ $$.call(
            $JSCompiler_temp_const$jscomp$ $$, 
            {
               width:$settings$$.w, 
               overflow:$settings$$.get("scrolling") ? "auto":"hidden"
            }
            )
          .css($JSCompiler_temp_const$$)
          .prependTo($JSCompiler_temp_const$jscomp$ $$);

vs:

        $$loaded$$ = $$tag$$("div", "LoadedContent").append($object$$);
       $$loaded$$.hide().appendTo($$loadingBay$$.show()).css(
         {width:function() {
            $settings$$.w = $settings$$.w || $$loaded$$.width();
            $settings$$.w = $settings$$.mw && $settings$$.mw < $settings$$.w ? $settings$$.mw : $settings$$.w;
            return $settings$$.w;
           }(),
          overflow:$settings$$.get("scrolling") ? "auto" : "hidden"
         }).css(
          {height:function() {
           $settings$$.h = $settings$$.h || $$loaded$$.height();
           $settings$$.h = $settings$$.mh && $settings$$.mh < $settings$$.h ? $settings$$.mh : $settings$$.h;
           return $settings$$.h;
          }()}
         ).prependTo($$content$$);

The main thing that I see here is settings.h being set before settings.w and the call to loaded.height(). The original source has this comment:

.appendTo($loadingBay.show())  // content has to be appended to the DOM for accurate size calculations.

https://github.com/jackmoore/colorbox/blob/master/jquery.colorbox.js#L801

It seems reasonable that this reordering is the cause of the problem. I'm unclear however if:
(1) The compiler determined that "show" is side-effect free, which should never occur in simple mode (there isn't enough information to make that assumption about anything)
(2) The decomposition code has a bug that somehow resulted in this incorrect ordering (i.e. it thought it was preserving the order).

[concavelenz]

Related internal issue b/117935266

[concavelenz]

I believe I have a fix for this in the ExpressionDecomposer. I'm running the internal tests now

@@ -233,7 +233,9 @@
       } else if (expressionParent.isCall()
           && NodeUtil.isNormalGet(expressionParent.getFirstChild())) {
         Node callee = expressionParent.getFirstChild();
-        decomposeSubExpressions(callee.getNext(), expressionToExpose, state);
+        if (callee != expressionToExpose) {
+          decomposeSubExpressions(callee.getNext(), expressionToExpose, state);
+        }

[concavelenz]

158b9d8

This will be part of the next release and should address this issue.

[lauraharker]

@Daijobou this should be fixed as of yesterday's release v20211107