Referencing ES6 modules from JsInterop in J2CL+Closure

[niloc132]

Depending on your perspective, this may be a bug report.

Description
The Closure Compiler supports ES6 modules, provided they are adapted to work with Closure's own module system. However, J2cl emits code which is incompatible with es6 modules, at least without an extra step of indirection.

This might also be more appropriate to discuss on the Closure Compiler issue tracker or mailing list, but I figured I should start here in case I am either missing something obvious, or there is a simpler answer than I am imagining.

Example: Given a plain JS module _with closure's workaround to let closure flavored modules access es6-flavored modules, and a @JsType-annotated class designed to provide access to it, it seems as though this should work out of the box:

goog.declareModuleId('my.js.module.SampleJsModule');
export function helloWorld() {
    return "Hello from a module"
}

@JsType(namespace = "my.js.module", isNative = true)
public class SampleJsModule {
    public static native String helloWorld();
}

By itself this compiles fine, but emits a compiler error in closure:

public class MyTest {
    @Test
    public void testText() {
        Assert.assertEquals("Hello from a module", SampleJsModule.helloWorld());
    }
}

/redacted/path/to/example/MyTest.impl.java.js:6: ERROR - [JSC_FORWARD_DECLARE_FOR_ES6_SHOULD_BE_CONST] goog.forwardDeclare alias for ES6 module should be const.
  6| let SampleJsModule = goog.forwardDeclare('my.js.module.SampleJsModule');
     ^^^^^^^^^^^^^^^^^^^^^^^

At first blush it seems like you could just replace this with const as requested, but this doesnt make sense since the class initializer will trigger external modules to be loaded:

 static $loadModules() {
  SampleJsModule = goog.module.get('my.js.module.SampleJsModule');
  Assert = goog.module.get('org.junit.Assert$impl');
 }

It turns out that closure has different limitations when using a "real" closure module and a "es6 module that supports closure module naming" - const must be used according to the error, which suggests perhaps that the goog.forwardDeclare should be named something else and never used? Based on the comments in base.js for goog.forwardDeclare, its possible that this line may not be necessary anyway (though the docs at least seem a little out of date - the link https://github.com/google/closure-compiler/wiki/Bad-Type-Annotation doesnt appear to exist), and that this was deprecated in favor of goog.requireType?

Describe the solution you'd like
Ideally, j2cl would support this case without any extra changes, like the below workaround.

Describe alternatives you've considered
The workaround we're currently using is to emit a "es6 to closure shim". First, the goog.declareModuleId line is modified to have some extra "i'm not available to be generally used in j2cl" suffix added, and then the "real" goog module is declared in its own file:

goog.module('my.js.module.SampleJsModule');

// oddly, this "let" doesn't bother closure? 
// this must be a goog.require, since goog.module.get() isnt allowed to be used top-level, even in another goog.module.
let shim = goog.require('my.js.module.SampleJsModule.shim');

exports = shim;

Now the JsType and anything that relies on it works correctly.

Additional context
Note that for most es6 modules, they don't assume closure-library is available (see also: nearly everything in NPM), and a shim is required to make the es6 module work from closure modules at all. Something like this serves as a simple bit of boilerplate:

// shim from plain es6 module to closure module
import * as module from './SampleJsModule.js';
goog.declareModuleId('my.js.module.SampleJsModule.shim');


// default exports cause an extra wrinkle for closure modules, the 
// 'my.js.module.SampleJsModule' must now export shim.module instead.
export {module};

Then, the closure+es6 to closure shim would reference this, the original es6 module is untouched, and the

[gkdn]

IIUC, the essence of your problem is, JsCompiler complains in let x = goog.forwardDeclare(..) if the module is defined with goog.declareModuleId(..); is that correct?

[niloc132]

That could be one potential solution, but I'm not certain that this the only or best way to resolve this sort of case. I'm not certain, but it might also be possible to have two different variables that store the referenced type - the let x_fwdD = goog.forwardDeclare('x') at the top level, and the x_module = goog.module.get('x') within $loadModules. It won't be that simple since various references (jsdoc, etc) will have to reference the renamed variable, but perhaps there is some middle ground that can be reached?

Like I said, the rules of forwardDeclare (or possibly requireType) aren't entirely clear to me, both exactly why it is used in this way, or why jscomp doesn't permit the use of let instead of const there.

In my very simple experiments of plain closure modules (rather than modifying j2cl or editing j2cl output), goog.require sufficed to reference es6 modules with a goog.declareModuleId call.

[gkdn]

JsCompiler generally doesn't permit the use of let to avoid redefinitions of the types but current goog.module.get solution is the only way to make cyclic deps work at the moment and that requires aliases to be non-const.

If goog.declareModuleId doesn't work then you can ask for guidance from Closure team. Unless there is a reason for this to not work with the J2CL's current goog.module.get pattern, I think the error could be added to the J2clSuppressWarningsGuard and Es6RewriteModules can be slightly modified to continue with rewriting after error reporting.

[niloc132]

Posted at https://groups.google.com/g/closure-compiler-discuss/c/dBgj7B3GqhE