Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WHITESPACE_ONLY mode does non-whitespace-only optimizations #3637

Open
AshleyScirra opened this issue Jul 13, 2020 · 7 comments
Open

WHITESPACE_ONLY mode does non-whitespace-only optimizations #3637

AshleyScirra opened this issue Jul 13, 2020 · 7 comments
Assignees
Labels

Comments

@AshleyScirra
Copy link

Closure compiler v20200614

in.js:

{
	function foo(p) { self["external1"](p) }
	
	console.log(foo());
	console.log(foo());
}

Command: java -jar ./closure-compiler.jar --js in.js --js_output_file out.js --compilation_level WHITESPACE_ONLY --formatting PRETTY_PRINT

out.js:

 {
  var foo = function(p) {
    self["external1"](p);
  };
  console.log(foo());
  console.log(foo());
}
;

Note that function foo(p) was rewritten as var foo = function(p). This is not a whitespace-only change. It also causes a bug (#3623). Therefore even using the lowest tier of WHITESPACE_ONLY optimizations, we cannot escape this bug. I think it is reasonable to expect WHITESPACE_ONLY to only do what it says and not make other adjustments which alter the way scripts work.

@ctjlewis
Copy link
Contributor

ctjlewis commented Jul 18, 2020

Following for updates. This is expected if language_in and language_out are different (transpilation will still be necessary), but unfortunately this persists even if you manually specify the same output for both (i.e. --language_in STABLE --language_out STABLE).

@brad4d, I managed to make progress on running the goog.module runtime tests, but they all currently fail because WHITESPACE_ONLY still causes $jscomp name mangling due to transpile, which is unavoidable because of this issue. Will try to work around. (Non-goog.module tests are working as expected).

image

@concavelenz
Copy link
Contributor

Any language_out above ECMASCRIPT_2015 should avoid this issue. "STABLE" as language_out is ECMASCRIPT5 currently (but for no good reason at this point).

I think that I would like to take WHITESPACE_ONLY back to its original intent: simply parse and print. That however, would mean that this would be restricted to a single "module" file input, or any number of scripts (which can be concatenated) but not both.

@ctjlewis
Copy link
Contributor

@concavelenz After spending a few days diving through the source I found the NO_TRANSPILE language_out flag, which seems to be the magic bullet for situations like this.

@AshleyScirra
Copy link
Author

Is that an undocumented option? The flags and options documentation doesn't appear to reference it.

@ctjlewis
Copy link
Contributor

ctjlewis commented Jul 27, 2020

@AshleyScirra Yeah, I only found it in passing. The only public reference I can find to it is @brad4d's issue #3256.

See the output for google-closure-compiler -O WHITESPACE_ONLY --formatting PRETTY_PRINT --language_out NO_TRANSPILE:

'use strict';
{
  function foo(p) {
    self["external1"](p);
  }
  console.log(foo());
  console.log(foo());
}
;

I would submit a PR to add it to the documentation, but Github doesn't support it for wiki pages. =(

Another secret flag I've found is the --renaming FALSE flag, which only works in SIMPLE mode and was mostly custom-tailored for the React team.

@AshleyScirra
Copy link
Author

What's the difference between NO_TRANSPILE and setting both the input and output language to the same thing?

@ctjlewis
Copy link
Contributor

ctjlewis commented Jul 27, 2020

@AshleyScirra There isn't supposed to be one - same input/output language should also imply no transpilation. It doesn't end up being 100% coverage, but it does work usually:

$ google-closure-compiler -O WHITESPACE_ONLY --formatting PRETTY_PRINT --language_in ES_NEXT --language_out ES_NEXT

'use strict';
{
  function foo(p) {
    self["external1"](p);
  }
  console.log(foo());
  console.log(foo());
}
;

Also works for ECMASCRIPT5 and a few others I tested. I think what the issue is here is that both language_in and language_out are set to STABLE by default, but STABLE is actually different for input and output (ends up being STABLE_IN and STABLE_OUT internally).

google-closure-compiler -O WHITESPACE_ONLY --formatting PRETTY_PRINT --language_in STABLE --language_out STABLE

 {
  var foo = function(p) {
    self["external1"](p);
  };
  console.log(foo());
  console.log(foo());
}
;

Using STABLE as input language still, but trying NO_TRANSPILE output target to prevent any unexpected behavior:

google-closure-compiler -O WHITESPACE_ONLY --formatting PRETTY_PRINT --language_in STABLE --language_out NO_TRANSPILE

'use strict';
{
  function foo(p) {
    self["external1"](p);
  }
  console.log(foo());
  console.log(foo());
}
;

It's definitely a bug, but --NO_TRANSPILE output mode is a good trick to have to work around it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants