Sometimes forgets types when importing via properties when using NODE resolution

[cpcallen]

When importing types using NODE resolution (i.e., via require(…)), Closure Compiler in some cases recognises the resulting types and in other cases does not. Consider:

types.js:

/** @constructor */
function Public() {}

/** @constructor */
Public.SubType = function() {};

/** @constructor */
function Private() {}

exports.Public = Public;
exports.testOnly = {Private};

test.js:

const {Public, testOnly} = require('./types.js');
const {Private} = testOnly;

let /** !Public */ pub;  // OK
let /** !Public.SubType */ sub;  // OK
let /** !Private */ private1;  // WARNING: Unknown type Private
let /** !testonly.Private */ private2;  // WARNING: Unknown type testOnly.Private

Output (v20210406):

$ google-closure-compiler -O=ADVANCED_OPTIMIZATIONS --module_resolution=NODE --process_common_js_modules types.js test.js --entry_point=test.js
test.js:6:9: WARNING - [JSC_UNRECOGNIZED_TYPE_ERROR] Bad type annotation. Unknown type Private
  6| let /** !Private */ private1;  // WARNING: Unknown type Private
              ^

test.js:7:9: WARNING - [JSC_UNRECOGNIZED_TYPE_ERROR] Bad type annotation. Unknown type testonly.Private
  7| let /** !testonly.Private */ private2;  // WARNING: Unknown type testOnly.Private
              ^

0 error(s), 2 warning(s), 100.0% typed

It's not really clear to me why it recognises Public.SubType as a type, but not Private. I'm not sure if this is a bug report or a feature request, but it is certainly one or the other as the present behaviour is unhelpful (and in a slightly insidious way: I've just spent several hours tracking down a bug caused by a typo that I was very surprised hadn't been caught by Closure Compiler—but it turned out it had no idea what type was being returned by calls to the Private constructor in my test harness because it had completely failed to infer a type here, and manually adding a type declaration provoked the unexpected warnings shown above).

[rishipal]

This might be requesting a new feature. I was able to use Private as a type inside test.js only if it's exported directly (like Public), but not when it's exported as a property inside an object (your example). ​

The guidance here does not restrict using the latter export pattern.

Closure compiler rewrites them differently, which somehow affects type resolution:

module$types.default.Public = function() {};

v/s

function Private$$module$types() {}
module$types.default.Private = {Private:Private$$module$types};

[ChadKillingsworth]

The rewriting of CommonJS modules is less than idea and there are several edge cases. The pass needs complete refactored and the bulk of the rewriting needs moved after type checking. This is a large effort I haven't had time to work on to date.

[mprobst]

If you are using Closure Compiler mostly to type check NodeJS code (i.e. you don't need optimization or other Closure Compiler specific features), it might be worth considering TypeScript instead. TypeScript generally has better built out support for CommonJS.

[cpcallen]

If you are using Closure Compiler mostly to type check NodeJS code it might be worth considering TypeScript instead.

This is absolutely excellent advice, but in our case TS was not clearly going to win when we started, and even if we were starting again now we probably wouldn't go that route because our project:

• is a sandboxed JavaScript runtime, and we want to be able to test it by having it execute itself recursively, and
• is intended for running interactive educational JavaScript programming experiences, and we want users to be able to drill down through the entire codebase (their code, our application, the web server, and the underlying JS runtime); having them all written in JavaScript makes this much more tractable.

[mprobst]

I'll leave it to @ChadKillingsworth to comment on the problem specifically and whether we'll get around to fixing it.

To manage expectations though, Google-internally we do not use CommonJS with Closure Compiler, so it's unlikely that we'll spend substantial time improving support for it.