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

Use CDNs for JS libraries #292

Merged
merged 2 commits into from
Sep 3, 2019
Merged

Use CDNs for JS libraries #292

merged 2 commits into from
Sep 3, 2019

Conversation

SamLau95
Copy link
Collaborator

@SamLau95 SamLau95 commented Sep 2, 2019

This commit uses CDNs to serve five JS libraries that are currently
checked into this repo:

anchor.min.js
lunr/lunr.min.js
print.min.js
tocbot.min.js
turbolinks.js

The corresponding copies of these libraries in this repo are removed.

It's considered good practice to use CDNs to serve minified JS files
since CDNs are designed to serve static files fast.

This doesn't change page load time for me, but might have benefits for
people located outside California (where there are fewer GitHub
servers and more Cloudflare servers).

This commit uses CDNs to serve five JS libraries that are currently
checked into this repo:

    anchor.min.js
    lunr/lunr.min.js
    print.min.js
    tocbot.min.js
    turbolinks.js

The corresponding copies of these libraries in this repo are removed.

It's considered good practice to use CDNs to serve minified JS files
since CDNs are designed to serve static files fast.

This doesn't change page load time for me, but might have benefits for
people located outside California (where there are fewer GitHub
servers and more Cloudflare servers).
@choldgraf
Copy link
Collaborator

Interesting! I had made the assumption that it's be better / faster to load these locally instead of from a CDN. Thanks for clarifying here!

Since these scripts are loaded asynchronously now, we need to ensure
that the libraries are loaded before trying to initialize them.
@choldgraf
Copy link
Collaborator

Is this ready to go?

@SamLau95
Copy link
Collaborator Author

SamLau95 commented Sep 3, 2019

Yup, this is good to go.

@choldgraf choldgraf merged commit 59c1da5 into master Sep 3, 2019
@choldgraf
Copy link
Collaborator

choldgraf commented Sep 3, 2019

Then thanks!

@choldgraf choldgraf added the bug Something isn't working label Sep 17, 2019
@choldgraf choldgraf deleted the async-everything branch August 7, 2020 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants