Fix a grab bag of closure compiler warnings #5329
Conversation
This fixes all warnings in about half of the remaining files with warnings.
In this file currently you can refer to the function as: caseMap.dashToCamelCase, CaseMap.dashToCamelCase, or dashToCamelCase. Let's just do a standard import style here.
rictic
requested review from
dfreedm,
kevinpschaaf,
sorvell and
TimvdLippe
as code owners
|
Still about ten files left. I'm waiting on a couple other internal-specific cleanup changes to land so I can tackle them, but I hope to have another PR in the next week or so taking care of the rest, then we should totally enforce no warnings in travis! I don't think we need an updated closure compiler version, but it never hurts. These changes were tested against this week's release |
| return null; | ||
| } | ||
| return PolymerDomModule.import(moduleId); | ||
| return /** @type {?DomModule} */(DomModule.import(moduleId)); |
There was a problem hiding this comment.
Any recollection why the original code used customElements.get to get the DomModule class? It was changed in this PR that doesn't have any detail: https://github.com/Polymer/polymer/pull/4748/files
There was a problem hiding this comment.
Yeah, Steve mentioned that ideally utils wouldn't depend on elements, and the customElements.get was there to avoid having that dependency after modularization, I guess in case someone wanted to use targeted imports to not load in dom-module.js. =
I tried to preserve that by declaring some interfaces, but I'm worried it would be fragile, both to changes in DomModule, and in the face of aggressive compiler optimizations. And in practice essentially everyone who loads style-gather is going to load dom-module anyways...
There was a problem hiding this comment.
Seems ok for now. We've recently discussed one way to make Polymer double-import safe would be to have <dom-module> only register if not already registered (first in wins), and then have all DomModule.import call sites get DomModule from the CE registry like the old code here did. But if we wanted to do that it would be a more sweeping change, and can do it separately. Steve agreed this change should be fine for now.
no-op at runtime, just helping jscompiler (and humans!) understand the code