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
Refactoring: Turn private custom element methods into functions. #3882
Refactoring: Turn private custom element methods into functions. #3882
Conversation
/** | ||
* Whether the element has been upgraded yet. | ||
* @return {boolean} | ||
* @final @this {!Element} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just for my own knowledge, is it right that this function inherits the same signatures as the method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No :)
LGTM, 1 question and 1 nit. |
} catch (e) { | ||
reportError(e, this); | ||
} finally { | ||
if (!isUpgraded(this)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wouldn't be called when there's an error in the previous code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Fixed.
And inline a few methods. The calls on the element are slow and cannot be optimized because the compiler doesn't touch the extern objects.
0027e4f
to
1ce6f02
Compare
Addressed review comments |
* master: (236 commits) trim all the columns (ampproject#3894) Refactoring: Turn private custom element methods into functions. (ampproject#3882) Lower the load priority of ad shaped iframes. (ampproject#3863) JsDoc fix (ampproject#3892) Add screenshots for Opera to AMP Validator extension. (ampproject#3866) Fix renaming of generated JSCompiler_prototypeAlias variable. (ampproject#3887) fix typo in amp-sidebar.md (ampproject#3833) Validator Roll-up (ampproject#3885) [CryptoService] Leverage browser native Crypto API to hash strings. (ampproject#3850) Size update (ampproject#3883) copy amp-ad docs to builtins (ampproject#3879) move doc to extension (ampproject#3878) [amp-experiment] Exposes isDismissed() method in AmpUserNotification (ampproject#3832) fix action-impl warning on dist (ampproject#3867) Add params for microad (ampproject#3827) Fixed some A4A tests. (ampproject#3859) Updates to colanalytics vendor config for amp-analytics. (ampproject#3849) Changes to implement A4A (AMP ads for AMP pages) (ampproject#3534) Addresses comment left over from PR#3841 (ampproject#3853) Expose submit event with on=submit:el.action syntax. (ampproject#3739) ...
And inline a few methods.
The calls on the element are slow and cannot be optimized because the compiler doesn't touch the extern objects.