CommonJS module rewriting incorrectly rewrites goog.modules

[supersteves]

I can't repro this in the debugger (can't guess what I need to configure to match my build), but here's what I'm seeing (names altered for brevity):

goog.module("a");
const f = v => {
  if (v instanceof Array) v.forEach(e => f(e));
  else console.log(v);
};
exports = {
  f
}

... ERROR - variable f$$module$a is undeclared ...

Debugging the compiler, the variable in scope is actually named:
module$contents$a_f

I'm experimenting with commonjs modules, if it's related.

The workaround is to use

exports = {
  f: f
}

[MatrixFrog]

Interesting. I assume this is with the compiler built from head, not the release? (If not, try it with a compiler built from head maybe...)

[supersteves]

Yes, 7a18029

[Dominator008]

@supersteves I thought I fixed this. I tried with:

// contents.js
goog.module("a");
const f = v => {
  if (v instanceof Array) v.forEach(e => f(e));
  else console.log(v);
};
exports = {
  f
}

run:

java -jar build/compiler.jar contents.js

got:

var module$contents$a_f=function(a){a instanceof Array?a.forEach(function(a){return module$contents$a_f(a)}):console.log(a)},module$exports$a={f:module$contents$a_f};

Can you provide a simple repro case, hopefully with the command line flags you used?

[supersteves]

Aha! I was experimenting with CommonJS modules and since gave up.

Short syntax works fine without enabling processing of CommonJS modules.
But when I turn on CompilerOptions.processCommonJSModules, it immediately fails.

No idea why - this is in a goog.module and it is being used by a goog.module...

I don't think I can currently create an online test case for CJS and ES6 modules. (Should I submit a PR with new enum entries in CompilationParam?)

Can't put together an offline test, our build process is a Java wrapper with masses of complexity and the JS codebase is huge too. In short, we use advanced obfuscation and turn on every check and optimisation that doesn't look like an internal or experimental option.

[Dominator008]

@supersteves Hmm. I tried

java -jar build/compiler.jar --process_common_js_modules contents.js 

and it's still working. Haven't tried with a Java wrapper though.

[ChadKillingsworth]

There's tests for that: https://github.com/google/closure-compiler/blob/master/test/com/google/javascript/jscomp/ProcessCommonJSModulesTest.java#L276-L331

[supersteves]

@Dominator008 did you try using the export? It's the use of the module that fails, I'm getting module$contents$a_f just like you are, but the point of use is trying to call f$$module$a().

[ChadKillingsworth]

Ugh - I bet the commonjs module rewriting is interpreting goog.module files as common js. We need to fix that.

[jimmyt857]

I stumbled across this, and I think I am having the same issue. I am trying to include a common js module in my project, and after adding --process_common_js_modules to my command line I started getting errors.

I have constructed the following minimal example in which the error occurs.

foo.js:

goog.module('foo');
exports.x = 1;

main.js:

goog.module('testing');

const foo = goog.require('foo');
console.log(foo.x);

My command line:

npx google-closure-compiler --compilation_level='SIMPLE' --language_in='ES6_STRICT' --module_resolution=NODE --dependency_mode=STRICT --js='main.js' --js='foo.js' --js='node_modules/google-closure-library/**.js' --entry_point testing --js_output_file='out.js' --process_common_js_modules
node out.js

The output of all of this is undefined. If I use @supersteves's workaround I get the expected result 1, although it comes with an annoying warning:
foo.js:5: WARNING - Suspicious re-assignment of "exports" variable. Did you actually intend to export something?

When the process_common_js_modules flag is omitted, everything works as intended.

[lauraharker]

Filed internal Google bug b/120092830

@jimmyt857 thanks for the repro!

@ChadKillingsworth do you know what's going on here?