Skip to content

Remove @suppress in custom-element.js#23877

Merged
dreamofabear merged 6 commits intoampproject:masterfrom
dreamofabear:remove-suppress-types
Aug 13, 2019
Merged

Remove @suppress in custom-element.js#23877
dreamofabear merged 6 commits intoampproject:masterfrom
dreamofabear:remove-suppress-types

Conversation

@dreamofabear
Copy link
Copy Markdown

Enables stronger type checking in custom-element.js by removing @suppress and @this annotations. I think these were originally needed on an older version of Closure compiler.

Fixed a few type errors that this caught and verified that this would've caught #23858.

/to @lannka @powerivq

@powerivq
Copy link
Copy Markdown
Contributor

powerivq commented Aug 9, 2019

Thanks @choumx Scanning through the codebase, there are few other places that suppresses checkType presumably for the same reason. Working on some others.

@lannka
Copy link
Copy Markdown
Contributor

lannka commented Aug 10, 2019

cool! thanks for the quick fix

@dreamofabear
Copy link
Copy Markdown
Author

This is causing single-pass build to DCE elementName():

/**
* The name of the custom element.
* @return {string}
*/
elementName() {

Investigating.

@dreamofabear dreamofabear merged commit f2c8f47 into ampproject:master Aug 13, 2019
@dreamofabear dreamofabear deleted the remove-suppress-types branch August 13, 2019 21:06
@lannka
Copy link
Copy Markdown
Contributor

lannka commented Aug 15, 2019

* @private Visible for testing.
* @extends {AMP.BaseTemplate}
* @suppress {checkTypes}
*/
export class AmpMustache extends AMP.BaseTemplate {

@choumx there is usage of this @suppress {checkType} in amp-mustache. could you also take a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants