Compilation level BUNDLE includes polyfill code that doesn't work in modules

[AshleyScirra]

Compilation level BUNDLE appears to include a lot of polyfill code, even for things supported by the language setting. The real problem though is this code relies on top-level this to refer to the global object, and hence the produced script fails if loaded as a module (or any other strict mode environment). Files used for repro are below.

main.js:

import * as Module from "./dep.js";

console.log(Module.GetMessage());

dep.js:

export function GetMessage()
{
	return "Hello world @ " + Date.now();
}

Command: java -jar ./compiler.jar --js main.js --js dep.js --dependency_mode PRUNE --entry_point main.js --compilation_level BUNDLE --language_in ECMASCRIPT_2021 --language_out ECMASCRIPT_2021 --formatting PRETTY_PRINT --js_output_file out.js

Expected result:

If the compilation level is set to SIMPLE, the output looks like this (a reasonable result):

function GetMessage$$module$dep() {
  return "Hello world @ " + Date.now();
}
var module$dep = {};
module$dep.GetMessage = GetMessage$$module$dep;
console.log(GetMessage$$module$dep());
var module$main = {};

Observed result:

With the compilation level set to BUNDLE, there are about 700 lines of polyfill code added to the output. This appears to include things like a polyfill for map and set, which aren't necessary given the specified language in and out are ECMASCRIPT_2021. However the real problem is near the end, the polyfill code includes:

this.CLOSURE_EVAL_PREFILTER = function(s) { return s; };
(function(thisValue){ /* ... contents omitted ... */)(this);

That code refers to top-level this in two places. In strict mode it will fail to run as top-level this is undefined. It is reasonable to expect the output to be run in strict mode, because the inputs were modules. This also looks like an oversight because earlier in the polyfill code it establishes the global object in $jscomp.global, so presumably that was meant to be used instead of this. But I'm not sure why the polyfill code is being included in the first place given the settings.

[niloc132]

It looks like you want to pass --inject_libraries=false to disable this behavior. This should prevent calling com.google.javascript.jscomp.deps.ClosureBundler#appendRuntimeTo, which seems to match what you are describing:

https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/deps/ClosureBundler.java#L203-L229

This should also omit the polyfills, which come in via https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/js/modules.js (and so pulls in https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/js/es6/modules/runtime.js and the referenced files from there).

Edit: That said, I would say you are correct that it might be possible to know about the $jscomp.global, which came in via util/global.js. It looks like this could be optional though - if you had manually constructed the bundler to use a different transpiler or runtime, you might not already have those utilities provided. This seems to have been added for "test/workaround related functions" and perhaps could be rolled back if those workarounds aren't needed any more?

https://github.com/google/closure-compiler/commit/1c17828883d2fd70d9e38be41d0db02568387d3e

[AshleyScirra]

Ah, thanks, --inject_libraries=false does the trick and works around this for bundle mode. I guess bundle mode has a different default depending on the compilation level? The Flags and Options page says it's always enabled but it looks like it's not the case in simple mode.

The use of this still breaks strict mode/modules though so it may be good to fix that anyway.

[niloc132]

Maintainers, would a patch that reverts https://github.com/google/closure-compiler/commit/1c17828883d2fd70d9e38be41d0db02568387d3e be welcome? Can any additional color be added about how long that workaround needs to exist (or are there other considerations other than in the commit message)?

[gunan]

We definitely do not want to revert Trusted Types support. However, updating the code to use globalThis should likely solve the problem. We can accept that patch.