Use readyState to trigger removal of old <link> [#198]

Previously we had a fixed 200ms delay before removing the old stylesheet on live CSS refresh. I'm not sure why it has been done that way. Problem is that Sprockets asset pipeline often takes more time to produce a CSS file, causing a flash of unstyled content for Rails developers. Waiting until readyState is 'complete' is clearly a superior way to detect when the new stylesheet has finished loading, and that's what this commit implements. Tested in Safari, Chrome, Firefox and Opera, and seems that it should work in IE too.
doubleotoo · May 20, 2012 · 8c98f67 · 8c98f67
1 parent 086b280
commit 8c98f67
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 4 deletions.
diff --git a/src/reloader.coffee b/src/reloader.coffee
@@ -63,7 +63,6 @@ exports.Reloader = class Reloader
 
   constructor: (@window, @console, @Timer) ->
     @document = @window.document
-    @stylesheetGracePeriod = 200
     @importCacheWaitPeriod = 200
     @plugins = []
 
@@ -78,6 +77,7 @@ exports.Reloader = class Reloader
 
   reload: (path, options) ->
     @options = options  # avoid passing it through all the funcs
+    @options.stylesheetReloadTimeout ?= 15000
     for plugin in @plugins
       if plugin.reload && plugin.reload(path, options)
         return
@@ -202,17 +202,28 @@ exports.Reloader = class Reloader
     clone = link.cloneNode(false)
     clone.href = @generateCacheBustUrl(link.href)
 
+    timeoutElapsed = no
+
+    removeOldLinkIfThatsAgreeable = ->
+      return if !link.parentNode
+      if timeoutElapsed or (clone.readyState is 'complete')
+        link.parentNode.removeChild(link)
+        clone.onreadystatechange = null
+
+    clone.onreadystatechange = removeOldLinkIfThatsAgreeable
+
     # insert the new LINK before the old one
     parent = link.parentNode
     if parent.lastChild is link
         parent.appendChild(clone)
     else
         parent.insertBefore clone, link.nextSibling
 
-    # give the browser some time to parse the new stylesheet, then remove the old one
+    # remove the old LINK even if we don't get the completion event
     timer = new @Timer ->
-      link.parentNode.removeChild(link) if link.parentNode
-    timer.start(@stylesheetGracePeriod)
+      timeoutElapsed = yes
+      removeOldLinkIfThatsAgreeable()
+    timer.start(@options.stylesheetReloadTimeout)
 
 
   reattachImportedRule: ({ rule, index, link }) ->

diff --git a/test/html/delayed/test.css b/test/html/delayed/test.css
@@ -0,0 +1,4 @@
+body {
+    background: green;
+    background: red;
+}
diff --git a/test/html/delayed/test.html b/test/html/delayed/test.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>LiveReload Test</title>
+    <link rel="stylesheet" href="http://localhost/~andreyvit/testcss.php">
+</head>
+<body>
+    <script src="../../../dist/livereload.js?host=localhost" async defer></script>
+</body>
+</html>
diff --git a/test/html/delayed/testcss.php b/test/html/delayed/testcss.php
@@ -0,0 +1,4 @@
+<?
+sleep(1);
+header("Content-Type: text/css");
+fpassthru(fopen('/Users/andreyvit/dev/products/LiveReload/js/test/html/delayed/test.css', 'rb'));