Lazy load / async load Disqus libraries for better performance

Would definitely speed up script performance to have it load asynchronously. I believe 7.x does some stuff to speed it up, but would be good to make sure it's in both 6 and 7.

I just ran into this. It is a necessary feature in my opinion.

This would be great - its quite a large script.

But maybe it would be better aimed at a higher level - like lazyload blocks, or lazyload panel panes? That way a generic solution could be used for other stuff??

Thanks, Joe

Attached is a patch to enable lazy loading of the Disqus comments script. I've added configuration options to enable/disable lazy loading, choose whether you want to lazy load the script based on when the comments scroll into view or whether the user clicks on an HTML element, and the class for elements to attach the click handler to.

We've had an internal review of this code, but another pair of eyes looking over this would be appreciated!

Re-rolled the patch with changes based on internal feedback.

Thanks! Looks OK but needs to go into D8 branch first. Here is quick port. Tested it really briefly and it seems to work. Some more testing would be more than welcome.

Tested the patch for both the branches; lazy loading works great!
but I see that the disqus comment count did not load. Please check if this bug could be reproduced.

just some minor nitpicks

+++ b/disqus.js
@@ -14,12 +14,32 @@ var disqus_def_email = '';
 var disqus_config;
 
 (function ($) {
+Drupal.disqus = {
+  loadCommentScript: function (shortname) {
+    // Make the AJAX call to get the Disqus comments.
+    this.loadScript(shortname, 'embed.js')
+  },

missing semicolon

@@ -166,4 +177,25 @@ class DisqusCommentManager implements DisqusCommentManagerInterface {
     return $data;
   }
 
+  /**
+   * Assembles data about lazy loading.
+   *
+   * @return array
+   */
+  function lazy_load_settings() {
+    $data = array();
+
+    switch (\Drupal::config('disqus.settings')->get('advanced.lazy_loading.method')) {
+      case DisqusCommentManager::DISQUS_LAZY_LOAD_ON_SCROLL:
+       $data['lazyLoadMethod'] = 'scroll';
+        break;

needs an extra space after $data['lazyLoadMethod'] = 'scroll'; (in d8 patch)

Fixed nitpiks.

Count bug is unrelated to this patch. It doesn't work even in HEAD. I suspect this problem is related to the way D8 loads links (via AJAX request). Currently we load count only once. We probably need to load it after AJAX request.

#2334865: Comment count script not working

It seems that this is, in part, for disqus-7.x-1.x-dev. Is there an order in which these patches should be applied? I'm just not clear on how to apply these files.

1. diff --git a/config/install/disqus.settings.yml b/config/install/disqus.settings.yml
   index 25563e1..e5a465a 100644
   
   index 25563e1..e5a465a 100644
   --- a/config/install/disqus.settings.yml
   
   --- a/config/install/disqus.settings.yml
   +++ b/config/install/disqus.settings.yml
   
   +++ b/config/install/disqus.settings.yml
   @@ -14,3 +14,7 @@ advanced:
   
   @@ -14,3 +14,7 @@ advanced:
        disqus_sso: false
        disqus_use_site_logo: false
        disqus_logo: ''
   +  lazy_loading:
   +    status: 0
   +    method: 'scroll'
   

   Needs config schema

2. +++ b/disqus.js
   @@ -18,6 +18,28 @@ var disqus_config;
   +  },
   +  loadScript: function (shortname, scriptName) {
   +    $.ajax({
   

   I'm wondering whether we should validate shortname to ensure that we don't request an arbitrary external script?

3. +++ b/disqus.js
   @@ -18,6 +18,28 @@ var disqus_config;
   +      cache: false
   

   Is there a reason for that?

4. +++ b/disqus.module
   @@ -226,11 +226,17 @@ function disqus_theme() {
   +    'disqus_lazy_load' => array(
   +      'variables' => array('children' => NULL),
   +    ),
   
   @@ -240,6 +246,24 @@ function theme_disqus_noscript() {
   +function theme_disqus_lazy_load($variables) {
   +  if (\Drupal::config('disqus.settings')->get('advanced.lazy_loading.status') && \Drupal::config('disqus.settings')->get('advanced.lazy_loading.method') == 'click') {
   +    return '<p class="disqus-lazy-load">' . t('View the discussion thread.') . '</p>' . $variables['children'];
   +  }
   +  else {
   +    return $variables['children'];
   +  }
   +}
   +
   

   This would use a template now by default, so I guess we should convert that.

I've tested the patch from #5 on the latest 7.12 and it works great.

@gregnostic Count no longer works after applying the patch in #5 for 7.x for me.

Thank you so much slashrsm for your initial work on this. I have updated your patch to work for Drupal 8.2.x

I didn't test this throughly but it applies successfully via composer now and it works on the front-end via button click. I had to replace the lazy load theme method with a twig template in the templates dir. You would need to modify or override this template in order to change the button look/text.

My apologies if this was not done "properly", I usually work on proprietary modules and so I haven't contributed a ton to Drupal.org

Here is a new D8 patch that simply fixes the whitespace errors in the #15 patch

1508786_D8_lazy_load.patch:149: trailing whitespace.
  
1508786_D8_lazy_load.patch:189: trailing whitespace.
    
1508786_D8_lazy_load.patch:220: trailing whitespace.
    
1508786_D8_lazy_load.patch:227: trailing whitespace.
    
1508786_D8_lazy_load.patch:243: trailing whitespace.

Thanks @slashrsm and @chris-matthews

I piggy-backed off your patch to add Comment Count functionality to the "Show Comments" button.

Patch below.

Patch added.

Forgot to add untracked template file. Rerolled patch. Also fixed issue with undefined variable.

Patch added.

Can someone document what this patch is solving in the issue summary? Why we are loading the comment count when we just want to lazy load JS files? Maybe I'm missing something though.

@dawehner

My apologies - my work is built on top of the lazy load JS file functionality and wanted to provide the code here just in case.

If needed once the lazy load JS file functionality is merged I can reroll the files into a separate patch/issue.

Thanks!

Thumbs up for this issue. Enabling the Disqus comments pulls down my site's page speed score from 90s all the way down to 50s.

I see that there's a patch for this problem, but that it doesn't use an IntersectionObserver. Most modern browsers support it and it will be worth it to take a look. I'll try the patch soon.

Here's how I'm solving the issue for my site:

1. Create a checkbox in Disqus settings saying whether to lazy load the Disqus thread
2. If lazy loading is enabled and if the browser does not support IntersectionObserver, there will be no lazy loading. It might be tempting to create our own intersection observer, but all modern browsers already support them or will eventually support them. So, it is better not to write additional code which will be deprecated soon.
3. If lazy loading is enabled and the browser supports IntersectionObserver, then attach an intersection observer which will load the Disqus thread when the user scrolls down to the comments area.

While we're working on the disqus.js file building this feature, we can also do some cleanup (optional). Like using $ sign instead of jQuery and breaking things into 2 behaviors, one for comment counts and one for the thread.

Just tested the patch in #22 and it works very well. Thanks! It would've been better to use an Intersection observer rather than a scroll observer. Also, there are many disqus_* variables in the global namespace (not created by the patch), it will be better to put them inside the Drupal.disqus namespace to keep the global namespace cleaner. This can be done as a separate task though.

The new lazyloading settings are not present in the config schema.

As for the comment counts, can we not simply detect if there is a comment-count related HTML element on the page? If some element needs a comment count, we load them, otherwise, we don't – one less setting to manage. For example, the thread is only required when the page contains a div#disqus_thread element. In the same way, the comment count should only be loaded when there are relevant elements present on the page.

The patch in comment #22 has worked for me and it's deployed to my prod site. I'll let the maintainers take it from here. If you're a maintainer please see my previous comment number 27. At least the config schema needs to be updated – the rest is optional I guess. Kindly make sure you credit the right people.

I installed the patch #22. The configuration page, button and comment counts works (plus twig template) but when I click in "Show comments" button, nothing happens. In configuration page, I setted up the default class on button that I saw in twig template.

Console of Chrome debug don't show any errors at all. What can I do?

EDIT: Enabling comments when scroll down works. Thanks for that!!

Updated patch for version 7.x.

I tried to apply the patch from #33 but it says this hunk is rejected. The rejections occur whether or not I apply against the dev or the lastest version of 7.

diff a/disqus.module b/disqus.module    (rejected hunks)
@@ -149,6 +159,10 @@ function disqus_element_post_render($children, &$element) {
     $element['#attached']['js'][] = drupal_get_path('module', 'disqus') . '/js/disqus_ga.js';
   }
 
+  if (variable_get('disqus_lazy_load', FALSE)) {
+    $disqus += disqus_lazy_load_settings();
+  }
+
   /**
    * Pass callbacks on if needed. Callbacks array is two dimensional array
    * with callback type as key on first level and array of JS callbacks on the

Since Drupal Core has started an initiative to remove jQuery usage, I have started the initiative of removing jQuery dependency from visitor-facing scripts of the Disqus module.

I'd request you to help test those changes because after they're merged, I will help fix this issue without jQuery asap so that your pages load faster without jQuery and lazy-loaded Disqus comments :D

I've updated a patch to work with version 2.0.1-alpha1 because I've to use it with Drupal 9.

I've updated a patch to work with version 2.0.1-alpha4.

The patch doesn't apply against the 2.0.x branch.

Patch #38 broke the module on 2.0.1-alpha5. The comments stopped loading.

Updated patch to work with 2.0.1-alpha5.

After applying patch #42, visiting configuration page throws a white screen with this error:

Class 'Drupal\disqus\Form\DisqusCommentManager' not found in Drupal\disqus\Form\DisqusSettings Form->buildForm() (line 293 of C:\wamp64\www\site\modules\contrib\disqus\src\Form\DisqusSettingsForm.php)

Lazy load via IntersectionObserver version. Changes and improvements

• Simplified options and placed under Behavior.
• Since we're lazy loading, using `defer` instead of `async` for all scripts.
• Removed click and scroll, replaced with IntersectionObserver.
• On browser without IntersectionObserver, it will load directly.

Backported IntersectionObserver version to 1.x. No tests, since there is none setup for 1.x.

This is marked as for the 2.x branch, but there is no patch for that branch right now... should this go back to "needs work" then? I'm happy to test out a patch for 2.x, but I'm a little lost on what the patch is doing exactly.

Oh this is my bad. I did not read close enough that #47 was a backport for 1.x, but #46 already existed for 2.x. Once I took some time and read slowly, I pulled in that patch on my site, and tested it out, and my PageSpeed scores were significantly improved. Marking as RTBC.

Patch #46 worked so far for me, but I see some possible improvements we could maybe also integrate.

• Split disqus.js and disqus.settings.js in two different libraries definitions, because disqus.settings.js seems to be only required for Drupal's Disqus settings form.
• Switch from var to let/const syntax in all JS files.
• Try to remove jQuery once and if possible also jQuery dependency from disqus.js to avoid extra loading of jQuery.
• Simplify all if (settings.disqus.language || false) { to if (settings.disqus.language) {.
• For better readability add extra lines between each libraries definition in disqus.libraries.yml file.
• It seems var disqus_lazy_load = 0; is never used.

@ckng: what do you think about possible improvements? Should we maybe set this issue back to needs work?

I've also linked related issue where already in the past some work was done to remove jQuery from disqus.js.

@ IT-Cru IMHO if we have some improvement which is ready to be merged, we should include that as it is for now. I say so because this issue has been open for a long time now. Every time there is a working patch, some other thing changes in the module which makes the patch become stale. So, if we have it working this time, we should merge what we have so that we at least remove of the jQuery dependency. Later, we can create another PR to separate the admin JS from the user-facing JS.

What do you think?

@jigarius: Complete fine for me. This is why I'm not switched it back to needs work and only mentioned possible additional improvements I see for near future fixing issues. It also make it easier when it is merged and rebase existing MRs like added linked issue which already remove usage of jQuery.

Better to split other improvements into small tasks which could be easier tested :)