Closure-compiler: ES6 module support for "type-only import" like `goog.requireType` or TS `import type`

[jplaisted]

The type system currently has no way of referencing types in ES6 modules without importing them. We need some sort of weak reference / import type for ES6 modules.

Bike shedding ideas dump:

1. Allow paths in type names. Use : as a delimiter for path:type.

/** @param {./foo.js:Type.Nested} n */
function foo(n) {}

1. Some commented out import syntax that brings the names into the type scope.

// @import {Type} from './foo.js';

/** @param {Type.Nested} n */
function foo(n) {}

1. Since the option above might be difficult with the current way we handle JsDoc (needs to be attached to something) and because Type doesn't really exist (it isn't a variable), we could modify the above option to act more like how we handle typedefs, i.e. annotate a variable declaration.

/** @import {Type} from './foo.js'; */
let Type;

/** @param {Type.Nested} n */
function foo(n) {}

[evmar]

1 resembles the analogous TS feature, I think. I've never used it myself but I think it was meant to look like dynamic imports, under the reasoning that imports in type position don't have runtime implications.

var x: import('./y).Type; 

https://davidea.st/articles/typescript-2-9-import-types

[jplaisted]

Ah, thanks for the info. Yeah basing this off dynamic import syntax may make a bit more sense, if we can do it.

/** @param {import('./foo.js').Type.Nested} n */
function foo(n) {}

/** @typedef {import('other/resolution').Type} */
let TypeFromOtherModule;

+ @tjgq

[ChadKillingsworth]

This isn't restricted to ES6 modules. CommonJS need the same thing.

[jplaisted]

@ChadKillingsworth Do we have any documentation as to how import-ing a CommonJS module works in the closure compiler? Specifically with default CommonJS exports. I'm wondering what /** import('some-commonjs-module') */ would mean. Maybe it makes more sense to have both import() and require()? I personally disagree with trying to use ES6 import syntax for non-ES6 modules.

[ChadKillingsworth]

@jplaisted It's a bit complicated, but also well defined. I don't have a problem using import('module') in an annotation to get a hold of the types in a CommonJS module. I'd rather not complicate this project further and treat ES6 as the go-forward strategy.

Since typescript already does this, there is precedent.

[jplaisted]

but also well defined

Defined where? I am asking for link to documentation. Doesn't have to be for the compiler specifically if the compiler just followed something else.

[ChadKillingsworth]

Ahh now that's the million dollar question. The "official" documentation is buried in a github issue on Node's repo somewhere and not at all clear.

Here's an overly simplistic view: https://webpack.js.org/migrate/4/#import-and-commonjs

In a nutshell, CommonJS modules are considered to have a single export property which is the default property.

[jplaisted]

From what I understand Node has yet to decide whether to support the interop, and how. But I'm not an expert, that's just what I've heard.

If there's no documentation, and if we're going off Node which afaik doesn't have an officially approved propsal, then that seems to be not well defined. Plus, as far as I can tell by just messing around, TypeScript has different meaning for import() than Webpack for a CommonJS module.

That aside, I think I'll just fix this for ES6 modules and leave you to fix it for CommonJS. I don't think it will be too difficult. My plan right now is to have a new pass that runs before module rewriting that rewrites these import comments to the global symbols the modules will be rewritten to. This pass will have access to what type of module is being imported (CJS or ES6 or Closure, etc), so it will be up to you to just add an else if (isCommonJs) block.

Sound good to you?

Note: obviously in a ideal world the type checker would just understand these annotations. Problem is the type checker doesn't understand modules of any kind at all right now. Ideally this new pass goes away once the type checker runs before module rewriting.

[ChadKillingsworth]

Sound good to you?

Sure does.

One more thing though - we'll need to prevent these files from being dropped from the compilation early.

[erwinmombay]

Hey all, is there any progress on this? This is making it impossible for us to upgrade closure compiler without reverting https://github.com/google/closure-compiler/commit/acea045bcc566c8d1ba74bc8f63fbb1568a18c2b (we actually had to)

would it be an unreasonable request to revert https://github.com/google/closure-compiler/commit/acea045bcc566c8d1ba74bc8f63fbb1568a18c2b until #3041 is implemented.

We cannot simply do imports of the Type as the solution, since importing a module could have side effects. This would also break circular references for us (which I assume we do in our large codebase)

[jplaisted]

I haven't had the time to look at this in awhile. I can bring it up at our prioritization meetings and see if we can schedule it.

Personally, I do feel it may be unreasonable to revert that PR. What we had wasn't really robust / didn't really work for most people. But you have my apologies, I probably should've checked to see who (if anyone) was using it before deleting it.

[brad4d]

Internal issue created http://b/129956426