Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use interfaces instead of namespaces in externs #1223

Open
theseanl opened this issue Nov 23, 2020 · 4 comments
Open

Use interfaces instead of namespaces in externs #1223

theseanl opened this issue Nov 23, 2020 · 4 comments

Comments

@theseanl
Copy link
Contributor

theseanl commented Nov 23, 2020

Currently, tsickle creates one namespace per file in externs, declares exported members as properties of such a namespace, and pass it around to express import * as ns syntax. This seems to be causing closure compiler to be overly conservative on property mangling, according to google/closure-compiler#2132.

This have had positive effects, namely, it has hided many problems with generated externs -- sometimes the generated externs skips over some properties over a namespace, but apparently the above issue make closure compiler to safely back off on such non-declared properties. But now this is being worked on, IMO tsickle could switch to interface-per-file format as described in the above issue. It would have another benefit of expressing export * from '...' syntax cleanly as multiple @extends.

I am not sure whether the below is a good closure compiler annotation, but it seems to be compiling well.

/** @externs */

/**********************************************************************/

/** @interface */
var intf1 = function() {};

/**
 * @constructor
 * @struct
 */
intf1.prototype.X = function() {};

/** @type {string} */
intf1.prototype.X.prototype.foo;

/** @type{!intf1} */
var myNs1;

/**********************************************************************/

/** @interface */
var intf2 = function() {};

/**
 * @record
 * @struct
 */
intf2.prototype.Y = function() {};

/** @type {string} */
intf2.prototype.Y.prototype.bar;

/** @type {!intf2} */
var myNs2;

/**********************************************************************/

/**
 * @interface
 * @extends {intf1}
 * @extends {intf2}
 */
var intf = function() {};

/** @type {!intf} */
var myNs
/**
 * @fileoverview 
 * @suppress {uselessCode}
 */

myNs.X;

(new myNs.X()).foo;
(new myNs.X()).bar; // [JSC_INEXISTENT_PROPERTY] Property bar never defined on intf1.prototype.X

/**
 * @param {intf1.prototype.X} x
 * @param {intf2.prototype.Y} y
 */
function g(x, y) {
    x.foo = y.bar;
    x = y; // [JSC_TYPE_MISMATCH] assignment found : ... required : ...
}

myNs.Z // [JSC_INEXISTENT_PROPERTY] Property Z never defined on intf

(Moved from #1220 (comment))

@evmar
Copy link
Contributor

evmar commented Nov 23, 2020

Thanks for filing this separately, I can ask some people who know more than me about it.

@evmar
Copy link
Contributor

evmar commented Nov 23, 2020

Note: tsickle only generates a namespace per file for module d.ts, not for script (aka 'global') dts.

@blickly
Copy link
Contributor

blickly commented Nov 23, 2020

Can you include a code snippet to show what the current behavior is that you're proposing to move away from?

@theseanl theseanl changed the title Can tsickle generate externs using interfaces? Use interfaces instead of namespaces in externs Nov 23, 2020
@theseanl
Copy link
Contributor Author

Current behavior would be the following:

/** @externs */

/**********************************************************************/

/** @const */
var myNs1 = {};

/**
 * @constructor
 * @struct
 */
myNs1.X = function() {};

/** @type {string} */
msNs1.X.prototype.foo;

/**********************************************************************/

/** @const */
var myNs2 = {};

/**
 * @record
 * @struct
 */
myNs2.Y = function() {};

/** @type {string} */
myNs2.Y.prototype.bar = function() {};

/**********************************************************************/

// /**
//  * @const {(typeof myNs1|typeof myN2)}
//  */
// var myNs;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants