angular / clutz

Closure to TypeScript `.d.ts` generator
MIT License
162 stars 60 forks source link

Incremental Clutz can't distinguish imports from goog.module() files and goog.provide() files. #596

Open LucasSloan opened 6 years ago

LucasSloan commented 6 years ago

A goog.provide file:

goog.provide('goog.example');
/** @constructor */
goog.example = function() {};

Produces this .d.ts:

declare namespace ಠ_ಠ.clutz.goog {
  class example extends example_Instance {
  }
  class example_Instance {
    private noStructuralTyping_: any;
  }
}
declare module 'goog:goog.example' {
  import alias = ಠ_ಠ.clutz.goog.example;
  export default alias;
}

A goog.module file:

goog.module("goog.example");
/** @constructor */
const A = function() {};
exports.A = A;

Produces this .d.ts:

declare namespace ಠ_ಠ.clutz.module$exports$goog$example {
  class A extends A_Instance {
  }
  class A_Instance {
    private noStructuralTyping_: any;
  }
}
declare module 'goog:goog.example' {
  import alias = ಠ_ಠ.clutz.module$exports$goog$example;
  export = alias;
}

The goog.provide file generates the namespace ಠ_ಠ.clutz.goog, while the goog.module generates the namespace ಠ_ಠ.clutz.module$exports$goog$example.

A goog.module file could require goog.example:

goog.module('importer');
const import = goog.require('goog.example');

In incremental mode, Clutz doesn't know if goog.example is from a goog.provide file or a goog.module file, so Clutz don't know to emit ಠ_ಠ.clutz.goog or ಠ_ಠ.clutz.module$exports$goog$example.

The two styles of namespace declaration need to be united, so incremental mode doesn't need to know about the style of its dependencies.

rkirov commented 6 years ago

I suggest we put this one on the back burner until we get more compelling use cases.

mprobst commented 6 years ago

My expectation would be that it is very common to have code that's goog.provide()d in a goog.module. All protos and most of the Closure standard library still use goog.provide. OTOH I have no idea how common it is to have those appear in exported signatures.

evmar commented 6 years ago

Is this fixed by #591?

mprobst commented 6 years ago

No, this bug is a known limitation with that PR.

rkirov commented 6 years ago

Just found one such case in the wild, so this is no longer purely hypothetical.

rkirov commented 6 years ago

reopening as this was rolledback