HubSpot / offline

Automatically display online/offline indication to your users
http://github.hubspot.com/offline/docs/welcome
MIT License
8.67k stars 849 forks source link

Offline.js not working with async #266

Open Gekk3 opened 6 years ago

Gekk3 commented 6 years ago

Yeah, the project isn't actively maintained. but may be I will help to someone newbile like me to solve the problem. if you load your js asynchronously, this code will not work, as in most cases the init() function will never fire.

just change the code

, "complete" === document.readyState ? init() : null != document.addEventListener ? document.addEventListener("DOMContentLoaded", init, !1) :(_onreadystatechange = document.onreadystatechange, document.onreadystatechange = function() { return "complete" === document.readyState && init(), "function" == typeof _onreadystatechange ? _onreadystatechange.apply(null, arguments) :void 0; });

with:

, "complete" === document.readyState ? init() : interval = setInterval(function() { if(document.readyState === 'complete') { clearInterval(interval); init() } }, 100);

Yes its 2018, and years we still looking to figure out if the document has completed loading...

FranDias commented 6 years ago

Yeah, the project isn't actively maintained.

True, but we are open to anyone who might want to show the project some love.

pldilley commented 6 years ago

Saw this issue whilst checking out another issue. Can I propose this instead...

Here is the original code:

if (document.readyState === 'complete') {
    init();
  } else if (document.addEventListener != null) {
    document.addEventListener('DOMContentLoaded', init, false);
  } else {
    _onreadystatechange = document.onreadystatechange;
    document.onreadystatechange = function() {
      if (document.readyState === 'complete') {
        init();
      }
      return typeof _onreadystatechange === "function" ? _onreadystatechange.apply(null, arguments) : void 0;
    };
}

Here's what I'd do, since onreadystatechange seems to be very well supported: Bring the above third else branch up, to make it the default "not loaded yet" choice, then forget document.addEventListener and then make the polling the new else choice):

if (document.readyState === 'complete') {
    init();
  } else if ('readyState' in document) {
    _onreadystatechange = document.onreadystatechange;
    document.onreadystatechange = function() {
      if (document.readyState === 'complete') {
        init();
      }
      return typeof _onreadystatechange === "function" ? _onreadystatechange.apply(null, arguments) : void 0;
  } else {
    var interval = setInterval(function() {
        if (document.readyState === 'complete') {
            clearInterval(interval);
            init();
         } 
     }, 100);
  }

P.S. Pretty sure I missed a bracket somewhere

pldilley commented 6 years ago

P.S.S. If it sounds good to you guys, I can submit a pull req. if you like.

Gekk3 commented 6 years ago

I tested the code, seems that it works pretty well. I actually I did not manage to reach the interval (or create a situation that it executed) Possible that in the original code the "document.addEventListener" made the problems, if the page already loaded. Which makes me a little doubt about the need for 'interval' in such case.