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

Using CDN for dependencies by default #1260

Closed
ohadschn opened this issue Sep 17, 2017 · 7 comments
Closed

Using CDN for dependencies by default #1260

ohadschn opened this issue Sep 17, 2017 · 7 comments

Comments

@ohadschn
Copy link
Contributor

The idea is to serve external dependencies (jQuery & plugins, Font Awesome, etc.) off a CDN. This has already been discussed at #1238 and #726, but I'm opening this issue to be the canonical place for this discussion (if that's OK with you @mmistakes).

I'll reiterate my arguments, and I'd love to hear more opinions - perhaps we could persuade Michael one day ;)

  • Reduced server load due to caching (concatenation is especially frowned upon since HTTP/2).
  • Improved performance due to caching and parallelized fetches.
  • Going by StackOverflow discussion, the vast majority of users are in favor of CDNs, so it makes sense for the default to cater to the majority (the new footer_script feature makes it easy to change for users that don't wish to use a CDN).
  • While GitHub Pages do have some CDN power behind them, they must be configured carefully to work with custom domains. That CDN integration also doesn't address parallelization and optimized caching (since all JS files are currently bundled into a single file main.min.js).

The only con is the external dependency, but most users will be using GitHub pages which is arguably a bigger dependency anyway. And there are CDNs which are extremely unlikely to be offline. Finally, the most popular way to set up HTTPS for GitHub Pages with custom domains is via CloudFlare (so if say the CF CDN is used no new dependency is introduced).

@annegentle
Copy link
Contributor

annegentle commented Oct 6, 2017

Okay, I'll bite. Background: I use the gem version of the theme (4.5.2 to be exact) on GitHub Pages, building locally then uploading, with a custom domain without HTTPS. While investigating HTTPS, my solution of choice would not be CloudFlare but Netlify.

All of these choices are complex and dependency-oriented. As a use case, today I'm working on implementing lunr.js for site search. I can't figure out where I could serve JQuery from, much less which version was compatible with a Lunr.js script I was using, nor which order main.min.js, jquery, lunr and a search.js file are being loaded in. So adding a CDN dependency doesn't get me out of this debugging hell I'm in.

I do like the footer_script feature and will implement that when I get to 4.6 of the theme.

Some of your argument stacks external dependencies against other external dependencies and that somehow seems like apples and oranges as far as features of a web site. I can get by a while without HTTPS, but I can't get by at all if our Javascript dependencies are broken.

@ohadschn
Copy link
Contributor Author

ohadschn commented Oct 7, 2017

@annegentle Using Netlify for HTTPS doesn't really make a difference, the JS could still come from any CDN (e.g. CloudFlare). I guess you could say it's redundant but really won't make much of a difference.

I think splitting the script includes (which you would do when you add the CDN) actually would make your life easier, because with one main.min.js you can only put your scripts before it or after it. But you might want to put your scripts ina more subtle location, e.g. before the main site code but after the jQuery include (example: #1238).

Note that so far you are only proving my initial argument - only advanced, specialized users would even care where their JS files are coming from. The rest (vast majority) would simply enjoy the increased performance and reduced server load.

BTW I don't understand why you think extracting JS dependencies to a CDN would break anything. Basically the entire internet is doing it, it works great.

@stale
Copy link

stale bot commented Nov 6, 2017

This issue has been automatically marked as stale because it has not had recent activity.

If this is a bug and you can still reproduce this error on the master branch, please reply with any additional information you have about it in order to keep the issue open.

If this is a feature request, please consider whether it can be accomplished in another way. If it cannot, please elaborate on why it is core to this project and why you feel more than 80% of users would find this beneficial.

This issue will automatically be closed in 7 days if no further activity occurs. Thank you for all your contributions.

@stale stale bot added the Status: Stale label Nov 6, 2017
@annegentle
Copy link
Contributor

Extracting dependencies to a CDN doesn’t break but it can make debugging more complex. In my case, the problem was that the JSON I had generated wasn’t formatted correctly, but main.min.js showed an error in the local JavaScript Console. So I was stumped for a couple of days and saw this thread, and thought I’d comment.

@stale stale bot removed the Status: Stale label Nov 6, 2017
@ohadschn
Copy link
Contributor Author

ohadschn commented Nov 6, 2017

@annegentle sounds to me like a CDN would have actually made debugging easier, because it would have narrowed down the script where the error originated, rather than "something happened in main.min.js".

You just need to set the crossOrigin attribute on the CDN script tag.

@stale
Copy link

stale bot commented Dec 6, 2017

This issue has been automatically marked as stale because it has not had recent activity.

If this is a bug and you can still reproduce this error on the master branch, please reply with any additional information you have about it in order to keep the issue open.

If this is a feature request, please consider whether it can be accomplished in another way. If it cannot, please elaborate on why it is core to this project and why you feel more than 80% of users would find this beneficial.

This issue will automatically be closed in 7 days if no further activity occurs. Thank you for all your contributions.

@coliff
Copy link
Contributor

coliff commented Feb 21, 2018

For anyone finding this issue - here's an updated CDN-equivalent solution:

footer_scripts           :
  - https://code.jquery.com/jquery-3.3.1.min.js
  - https://cdnjs.cloudflare.com/ajax/libs/jquery-smooth-scroll/2.2.0/jquery.smooth-scroll.min.js
  - https://cdnjs.cloudflare.com/ajax/libs/fitvids/1.1.0/jquery.fitvids.min.js
  - https://cdnjs.cloudflare.com/ajax/libs/magnific-popup.js/1.1.0/jquery.magnific-popup.min.js
  - /assets/js/plugins/jquery.greedy-navigation.js
  - /assets/js/_main.js
  - https://use.fontawesome.com/releases/v5.0.6/js/all.js

okitem pushed a commit to okmalls/okmalls.github.io that referenced this issue Mar 9, 2024
Fixes stylesheet name in post `2019-08-09-getting-started.md`

---------

Co-authored-by: Cotes Chung <11371340+cotes2020@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants