Merge pull request zweilove#35 from Umofomia/bundle-processor

Convert splitter engine to be a Sprockets bundle processor

Author: jhilden

README.md

@@ -25,13 +25,17 @@ CssSplitter integrates with the Rails 3.1+ Asset Pipeline to generate additional
 
 The first step is identifying the stylesheets that have more than 4095 selectors and therefore need to be split for IE.
 
-Once you know which stylesheets need to be split, you need to create a second "container file" for those stylesheets with the file extension `.split2`, that will contain the styles beyond the 4095 selector limit.
+Once you know which stylesheets need to be split, you need to create a second "container file" for those stylesheets with the `_split2` suffix appended to the base filename that will contain the styles beyond the 4095 selector limit.  The extension of this file should be just `.css` without any additional preprocessor extensions.
 
-For example, if you want to split `too_big_stylesheet.css`, you need to create a new file `too_big_stylesheet_split2.css.split2` in the same directory.  The only content of that container, will be an include of the original file, e.g.:
+For example, if you want to split `too_big_stylesheet.css.scss`, you need to create a new file `too_big_stylesheet_split2.css` in the same directory.  The only content of that container, will contain a `require` directive to the name of the original asset, e.g.:
 
-    # app/assets/stylesheets/too_big_stylesheet_split2.css.split2
+    # app/assets/stylesheets/too_big_stylesheet_split2.css
 
-    //= include 'too_big_stylesheet.css'
+    /*
+     *= require 'too_big_stylesheet'
+     */
+
+If your stylesheet is big enough to need splitting into more than two more files, simply create additional `_split3`, `_split4`, etc. files, the contents of which should be identical to the `_split2` file.
 
 You also need to remember to add those new files to the asset pipeline, so they will be compiled. For example:
 
@@ -43,12 +47,10 @@ You also need to remember to add those new files to the asset pipeline, so they
 
 Here is a checklist of requirements for your split stylesheet:
 
-1. It needs to have different filename than orginal, e.g. `original_stylesheet_split2` or `application_split2`
-2. It needs to have `.split2` as a file extension, e.g. `.css.split2`, `.css.sass.split2`, or `.css.split2.erb`
-3. It needs to include the content of the orginal stylesheet, e.g. through `//= include 'application'` or `<%= environment.find_asset('application') %>`
-4. It needs to be added to list of precompiled assets
-
-
+1. It needs to have the `_splitN` suffix appended to the original asset name, e.g. `original_stylesheet_split2` or `application_split2`
+2. It needs to have `.css` as a file extension.
+3. It needs to require the orginal stylesheet.
+4. It needs to be added to list of precompiled assets.
 
 ### 2. Including your split stylesheets
 
@@ -60,51 +62,28 @@ You can just use our `split_stylesheet_link_tag` helper, which would look someth
     <%= split_stylesheet_link_tag "too_big_stylesheet", :media => "all" %>
 
     # output
-    <link href="/stylesheets/too_big_stylesheet.css" media="screen" rel="stylesheet" type="text/css" />
+    <link href="/stylesheets/too_big_stylesheet.css" media="all" rel="stylesheet" type="text/css" />
     <!--[if lte IE 9]>
-      <link href="/stylesheets/too_big_stylesheet_split2.css" media="screen" rel="stylesheet" type="text/css" />
+      <link href="/stylesheets/too_big_stylesheet_split2.css" media="all" rel="stylesheet" type="text/css" />
     <![endif]-->
 
+If your stylesheet is split into more than two files, add the `split_count` option to specify the total number of files.
+
+    <%= split_stylesheet_link_tag "too_big_stylesheet", :split_count => 3 %>
+
 Or you can just create similar HTML as in the above example yourself.  If you want to use the `split_stylesheet_link_tag` helper you need to make sure the gem is loaded in production, so you can't put it in the `:assets` group in your Gemfile.
 
 ## How it works
 
-Basically, CssSplitter is registering a new `Sprockets::Engine` for the `.split2` file extension, that will fill those files with all the selectors beyond the 4095th.  Unfortunately, those `.split2` files need to be created manually, because we haven't figured out a way for a `Sprockets::Engine` to output multiple files.  They need to present before the compile step.
+Basically, CssSplitter is registering a new Sprockets bundle processor that looks for CSS assets named with the `_splitN` suffix and will fill those files with all the selectors beyond the 4095th.  Unfortunately, those `_splitN` files need to be created manually, because we haven't figured out a way for a `Sprockets::Engine` to output multiple files.  They need to present before the compile step.
 
 If you have more questions about how it works, look at the code or contact us.
 
 ## Gotchas
 
-#### Having a JS asset with the same name as the the split stylesheet
-
-If you want to split a style (e.g. `assets/stylesheets/application.*`) and have a JS asset with the same name (`assets/javascripts/application.*`) in your asset load_path (as is the default in Rails), you need to include the stylesheet along with the file extension `// = include 'application.css'` because otherwise it will try to include the JS asset of the same name instead.  Sprocket's `= include` directive doesn't seem to differentiate between different types/folders and just takes the first asset it can find for any given name (see #10).
-
-#### Don't use Sprocket's `= require_tree .` or `= require_self` for stylesheets
-It's recommended that you **always use Sass's `@import`** for all your stylesheets in favor of Sprocket's `= require` directives, just as the official `sass-rails` gem says: https://github.com/rails/sass-rails#important-note
-
-If you have a `.split2` stylesheet in your tree that in turn includes the base stylesheet like shown below, you will end up with a nasty `Sprockets::CircularDependencyError`!
-
-    /* assets/stylesheets/application.css */
-    /* = require_tree .
-    
-    /* assets/stylesheets/application_split2.css.split2 */
-    /* = include 'application.css' */
-
-If you have `require_self` in the stylesheet that you're splitting, as shown below, the `.split2` will end up having **both** the original stylesheet and the split contents. You'll end up with an even bigger stylesheet.
-
-    /* assets/stylesheets/application.css */
-    /* = require_self
-    
-    /* assets/stylesheets/application_split2.css.split2 */
-    /* = include 'application.css' */
-
-
-## Limitations & Known Issues
-
-**More than 8190 selectors**
-
-Currently the gem only supports stylesheets that need to be split into 2 files.  It could theoretically create more splits (e.g. if you should have more than 8190 selectors), but in that case you should probably refactor your stylesheets anyway.  Contact us, if you have this requirement.
+#### Differences from previous versions
 
+Note that if you used the 0.x.x versions of this gem, the naming and contents of the split files have changed. Split files no longer need to have the `.split2` extension and now use the `require` directive rather than the `include` directive. The previous prohibition against using `require_tree .` and `require_self` directives also no longer applies.
 
 ## Credits & License
 

app/helpers/css_splitter/application_helper.rb

@@ -1,18 +1,20 @@
 module CssSplitter
   module ApplicationHelper
     def split_stylesheet_link_tag(*sources)
-      original_sources = sources.dup
+      options     = sources.extract_options!
+      split_count = options.delete(:split_count) || 2
 
-      options = sources.extract_options!
-      sources.collect!{ |source| "#{source}_split2" }
-      sources << options
+      sources.map do |source|
+        split_sources = (2..split_count).map { |index| "#{source}_split#{index}" }
+        split_sources << options
 
-      [
-        stylesheet_link_tag(*original_sources),
-        "<!--[if lte IE 9]>",
-        stylesheet_link_tag(*sources),
-        "<![endif]-->"
-      ].join("\n").html_safe
+        [
+          stylesheet_link_tag(source, options),
+          "<!--[if lte IE 9]>",
+          stylesheet_link_tag(*split_sources),
+          "<![endif]-->"
+        ]
+      end.flatten.join("\n").html_safe
     end
   end
 end

lib/css_splitter/engine.rb

@@ -3,7 +3,7 @@ class Engine < ::Rails::Engine
     isolate_namespace CssSplitter
 
     initializer 'css_splitter.sprockets_engine', after: 'sprockets.environment', group: :all do |app|
-      app.assets.register_engine '.split2', CssSplitter::SprocketsEngine
+      app.assets.register_bundle_processor 'text/css', CssSplitter::SprocketsEngine
     end
 
     initializer 'css_splitter.action_controller' do |app|

lib/css_splitter/sprockets_engine.rb

@@ -11,12 +11,12 @@ def prepare
     end
 
     def evaluate(scope, locals, &block)
-      # determine which is the current split (e.g. split2, split3)
-      split = if scope.pathname.extname =~ /(\d+)$/; $1
-              elsif scope.pathname.basename.to_s =~ /_split(\d+)\.css/; $1
-              else 2
-              end
-      CssSplitter::Splitter.split_string(data, split.to_i)
+      # Evaluate the split if the asset is named with a trailing _split2, _split3, etc.
+      if scope.logical_path =~ /_split(\d+)$/
+        CssSplitter::Splitter.split_string(data, $1.to_i)
+      else
+        data
+      end
     end
   end
 

test/css_splitter_test.rb

@@ -1,7 +1,41 @@
 require 'test_helper'
 
 class CssSplitterTest < ActiveSupport::TestCase
+
+  setup :clear_assets_cache
+
   test "truth" do
     assert_kind_of Module, CssSplitter
   end
+
+  test "asset pipeline stylesheet splitting" do
+    part1 = "#test{background-color:red}"   * CssSplitter::Splitter::MAX_SELECTORS_DEFAULT
+    part2 = "#test{background-color:green}" * CssSplitter::Splitter::MAX_SELECTORS_DEFAULT
+    part3 = "#test{background-color:blue}"
+
+    assert_equal "#{part1}#{part2}#{part3}\n",  assets["erb_stylesheet"].to_s
+    assert_equal "#{part2}\n",                  assets["erb_stylesheet_split2"].to_s
+    assert_equal "#{part3}\n",                  assets["erb_stylesheet_split3"].to_s
+  end
+
+  test "asset pipeline stylesheet splitting on stylesheet combined using requires" do
+    red   = "#test{background-color:red}"   * 100
+    green = "#test{background-color:green}" * CssSplitter::Splitter::MAX_SELECTORS_DEFAULT
+    blue  = "#test{background-color:blue}"
+
+    assert_equal "#{red}#{green}#{blue}\n",                           assets["combined"].to_s
+    assert_equal "#{"#test{background-color:green}" * 100}#{blue}\n", assets["combined_split2"].to_s
+  end
+
+  private
+
+  def clear_assets_cache
+    assets_cache = Rails.root.join("tmp/cache/assets")
+    assets_cache.rmtree if assets_cache.exist?
+  end
+
+  def assets
+    Rails.application.assets
+  end
+
 end

test/dummy/app/assets/stylesheets/combined.css.scss

@@ -0,0 +1,4 @@
+//= require 'red_100'
+//= require 'green_max'
+
+#test { background-color: blue; }

test/dummy/app/assets/stylesheets/combined_split2.css

@@ -0,0 +1,3 @@
+/*
+ *= require 'combined'
+ */

test/dummy/app/assets/stylesheets/erb_stylesheet.css.scss.erb

@@ -0,0 +1,7 @@
+<% CssSplitter::Splitter::MAX_SELECTORS_DEFAULT.times do %>
+#test { background-color: red; }
+<% end %>
+<% CssSplitter::Splitter::MAX_SELECTORS_DEFAULT.times do %>
+#test { background-color: green; }
+<% end %>
+#test { background-color: blue; }

test/dummy/app/assets/stylesheets/erb_stylesheet_split2.css

@@ -0,0 +1,3 @@
+/*
+ *= require 'erb_stylesheet.css'
+ */

test/dummy/app/assets/stylesheets/erb_stylesheet_split3.css

@@ -0,0 +1,3 @@
+/*
+ *= require 'erb_stylesheet.css'
+ */