Add Polymer.IronIconsetSvg = for closure compilier

[stevenjb]

This allows us to add iron-iconset-svg as a dependency for closure compilation.

[rictic]

This PR looks fine to me, no objections to merge it, but can you give some more info on why you need to expose it as an identifier to depend on it?

[samccone]

I am a little bit worried about this change, we do not do this anywhere else in the elements (overall) So if we decide that this is a good idea, then we should batch update all elements, otherwise adding this inconsistency seems dangerous.

[rictic]

Yep, agreed. No worries though, if this is necessary then we'll figure something out. It's likely that the compiler is already doing what's necessary here, or could be extended to do so. e.g. it generates a type name for every element, for this one it generates the type IronIconsetSvgElement

[stevenjb]

Hmm. I noticed that we do this for IronMeta, so I assumed that IronIconsetSvg was an outlyer, not the other way around.

@jklein24 - should we be using @type {IronIconsetSvgElement} instead?

[jklein24]

@stevenjb, yes, just using IronIconsetSvgElement is fine. For the record, the way this works is that if there's no LHS assigned the result of the Polymer call, the compiler generates the FooBarElement type. FWIW, you're not the first person to ask about this so I probably need to document it better somewhere. Just not sure what the right place for that documentation is :-/

[mgiuffrida]

Yep, that works for us!

FWIW:, two elements do use Polymer.FooBar = Polymer(...):

• PolymerElements/iron-a11y-announcer
• PolymerElements/iron-meta

One uses var FooBar = Polymer(...) (probably not accessible to closure?):

• PolymerElements/paper-menu-button

All the others would have closure-generated types like IronIconsetSvgElement. Aside from the outliers above, I agree with everyone else that consistency is important here. Any time we make a change like the one suggested here, we'd break usages of closure-generated types like IronIconsetSvgElement, so PolymerElements should probably avoid making this kind of change in general.

[jklein24]

Actually the compiler is totally fine with "var FooBar" too. It will just use "FooBar" as the type.

[rictic]

Good point on this effectively renaming the element's type. Fortunately we'd notice any breakage when syncing with google3 and could update all users pretty easily.

AIUI the reason we do this in a few places is to make it easier to use an element imperatively. IronMeta e.g. is particularly useful for this, as it's effectively a little db for global state.

[stevenjb]

I've confirmed that IronIconsetSvgElement does the trick and have reverted this change locally, so we can close the pull request (once we're done with any discussion).

It is a little unfortunate that it's: @type {IronIconsetSvgElement}

but

@type {Polymer.IronMeta}

Is there strong reason not to also create e.g. an IronMetaElement type so that once a dev learns about FooElement it can be used always?

On Tue, Sep 22, 2015 at 6:16 PM, Peter Burns notifications@github.com wrote:

Good point on this effectively renaming the element's type. Fortunately we'd notice any breakage when syncing with google3 and could update all users pretty easily.

AIUI the reason we do this in a few places is to make it easier to use an element imperatively. IronMeta e.g. is particularly useful for this, as it's effectively a little db for global state.

— Reply to this email directly or view it on GitHub https://github.com/PolymerElements/iron-iconset-svg/pull/20#issuecomment-142460360 .

[rictic]

Ok, closing the pull request. I believe that we can continue discussing after it's closed.

Does closure compiler support having multiple names for a type?

[jklein24]

cc @matrixfrog - It feels a bit weird to me to duplicate a type under multiple names, but I definitely see what you're saying and why it might reduce developer confusion. Tyler, what do you think?

[MatrixFrog]

In general if you want two names for the same type you usually do

/** @constructor */
var name2 = name

or

/** @constructor */
qualfied.name2 = name

if it's a class and you want to be able to construct it using either name. Or,

 /** @type {name1} */
 var name2;

if you just want to use the 2nd name in type annotations. Not sure if either of those approaches works here.

[jklein24]

Hmm... I suppose it would be ok just just add a generated

/** @constructor */
var IronMetaElement = Polymer.IronMeta;

in cases like this. That way users could always use the FooBarElement type in annotations, but can also use the LHS of the Polymer call as an alias.