Skip to content
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

Refactor module init #2015

Merged
merged 1 commit into from
Jan 9, 2025
Merged

Refactor module init #2015

merged 1 commit into from
Jan 9, 2025

Conversation

liamcmitchell
Copy link
Contributor

Simplify html.js by moving all init into modules.

  • run preview & hint first, they remove DOM, reducing work for other modules
  • register swup:page:view callbacks from within modules
  • move isEmbedded, isPreview and isHint into globals
  • run module init from within modules
  • adjust descriptionElementFromHash to also move comment nodes - preserving tabset comments
  • inline many things in preview
  • fix scroll caused by preview loading (on chrome/edge at least)
  • fix quick switch link after swup nav (init wasn't called after nav)
  • inline many things in sidebar version select
  • move swup init into own module, it doesn't need to know about other modules now
  • inline many things in hint-page

One thing I noticed when refactoring the init logic:

  if (isPreview && isEmbedded) {
    initPreview()
  } if (isHint && isEmbedded) {
    initHintsPage()
  } else {
  //  init swup, versions, keyboard shortcuts, qs, sidebar...

Notice that the second if is not an else if even though it's formatted like one. I assume that it was intended to be an else if and have refactored it that way.

By shuffling the module init and preventing a lot of unnecessary work, previews load almost 2x faster for me locally.

Before

image

After

image

-0.4kb JS

Copy link

github-actions bot commented Jan 9, 2025

@josevalim josevalim merged commit b644a01 into elixir-lang:main Jan 9, 2025
4 checks passed
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

@liamcmitchell liamcmitchell deleted the assets branch January 9, 2025 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants