-
-
Notifications
You must be signed in to change notification settings - Fork 79.2k
Register only one DOMContentLoaded event listener in onDOMContentLoaded
#34158
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
Conversation
|
Seems fine by me. (You may could add a test) |
6760d0a to
7278a2b
Compare
|
@GeoSot Changed the implementation so it wouldn't add an event listener when the function isn't called during |
|
@rohit2sharma95 give it a look, if you like, for a second opinion |
rohit2sharma95
left a comment
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 work @alpadev 🥇
7278a2b to
9c9bb7c
Compare
…aded function Instead of adding an event listener everytime the utility function is called, cache the callbacks and execute them all at once.
d4dd813 to
99e448e
Compare
|
@XhmikosR removed the IIFE as requested |
DOMContentLoaded event listener in onDOMContentLoaded
…oaded` (twbs#34158) * refactor: reuse one DOMContentLoaded event listener in onDOMContentLoaded function Instead of adding an event listener everytime the utility function is called, cache the callbacks and execute them all at once. * refactor: drop iife for onDOMContentLoaded Co-authored-by: XhmikosR <[email protected]>
Closes #34103
Since #34114 is more focused around changing the way we register our components with jQuery, this PR is changing the
onDOMContentLoadedutility function, to make use of an array to cache callbacks (while the document is loading) and execute them all at once. Therefore we register only oneDOMContentLoadedevent listener instead of one for every call to the function.