Merge pull request #34 from mudbugmedia/refactors

Refactors

Author: michael-misshore

README.md

@@ -27,7 +27,7 @@ However, more packages may need to be installed depending on your OS distributio
 After reviewing the dependency requirements, add `critical-path-css-rails` to your Gemfile:
 
 ```
-gem 'critical-path-css-rails', '~> 2.10.0'
+gem 'critical-path-css-rails', '~> 3.0.0'
 ```
 
 Download and install by running:

docker/ruby/Dockerfile

@@ -7,10 +7,11 @@ RUN apt-get update && apt-get install -y build-essential libpq-dev nodejs npm
 # Install Penthouse JS Dependencies
 RUN apt-get install -y libpangocairo-1.0-0 libx11-xcb1 libxcomposite1 libxcursor1 libxdamage1 libxi6 libxtst6 libnss3 libcups2 libxss1 libxrandr2 libgconf2-4 libasound2 libatk1.0-0 libgtk-3-0
 
+# Configure Node/NPM
 RUN npm cache clean -f
 RUN npm install -g n
-RUN n 8.9.3
-RUN ln -sf /usr/local/n/versions/node/8.9.3/bin/node /usr/bin/nodejs
+RUN n 10.15.1
+RUN ln -sf /usr/local/n/versions/node/10.15.1/bin/node /usr/bin/nodejs
 
 ENV BUNDLE_PATH /gems
 

lib/critical-path-css-rails.rb

@@ -6,16 +6,11 @@ module CriticalPathCss
   CACHE_NAMESPACE = 'critical-path-css'.freeze
 
   def self.generate(route)
-    ::Rails.cache.write(
-      route,
-      CssFetcher.new(config).fetch_route(route),
-      namespace: CACHE_NAMESPACE,
-      expires_in: nil
-    )
+    ::Rails.cache.write(route, fetcher.fetch_route(route), namespace: CACHE_NAMESPACE, expires_in: nil)
   end
 
   def self.generate_all
-    CssFetcher.new(config).fetch.each do |route, css|
+    fetcher.fetch.each do |route, css|
       ::Rails.cache.write(route, css, namespace: CACHE_NAMESPACE, expires_in: nil)
     end
   end
@@ -32,7 +27,11 @@ def self.fetch(route)
     ::Rails.cache.read(route, namespace: CACHE_NAMESPACE) || ''
   end
 
-  def self.config
-    @config ||= Configuration.new(CriticalPathCss::Rails::ConfigLoader.new.load)
+  def self.fetcher
+    @fetcher ||= CssFetcher.new(Configuration.new(config_loader.config))
+  end
+
+  def self.config_loader
+    @config_loader ||= CriticalPathCss::Rails::ConfigLoader.new
   end
 end

lib/critical_path_css/css_fetcher.rb

@@ -10,33 +10,21 @@ def initialize(config)
     end
 
     def fetch
-      @config.routes.map.with_index { |route, index|
-        css_path = @config.css_paths[index].present? ? @config.css_paths[index] : @config.css_path
-        [route, css_for_route(route, css_path)]
-      }.to_h
+      @config.routes.map { |route| [route, fetch_route(route)] }.to_h
     end
 
     def fetch_route(route)
-      css_for_route route
-    end
-
-    protected
-
-    def css_for_route(route, css_path)
       options = {
         'url' => @config.base_url + route,
-        'css' => css_path,
-        ## optional params
-        # viewport dimensions
+        'css' => fetch_css_path_for_route(route),
         'width' => 1300,
         'height' => 900,
+        'timeout' => 30_000,
         # CSS selectors to always include, e.g.:
         'forceInclude' => [
           #  '.keepMeEvenIfNotSeenInDom',
           #  '^\.regexWorksToo'
         ],
-        # ms; abort critical CSS generation after this timeout
-        'timeout' => 30_000,
         # set to true to throw on CSS errors (will run faster if no errors)
         'strict' => false,
         # characters; strip out inline base64 encoded resources larger than this
@@ -63,5 +51,17 @@ def css_for_route(route, css_path)
       end
       out
     end
+
+    private
+
+    def fetch_css_path_for_route(route)
+      index_for_route = @config.routes.index(route)
+
+      if index_for_route && @config.css_paths[index_for_route]
+        @config.css_paths[index_for_route]
+      else
+        @config.css_path
+      end
+    end
   end
 end

lib/critical_path_css/rails/config_loader.rb

@@ -3,22 +3,13 @@ module Rails
     class ConfigLoader
       CONFIGURATION_FILENAME = 'critical_path_css.yml'.freeze
 
-      def load
-        config = YAML.safe_load(ERB.new(File.read(configuration_file_path)).result, [], [], true)[::Rails.env]
-        validate_css_path config
-        if config['css_path']
-          config['css_path'] = "#{::Rails.root}/public" + (
-            config['css_path'] ||
-              ActionController::Base.helpers.stylesheet_path(
-                config['manifest_name'], host: ''
-              )
-          )
-          config['css_paths'] = []
-        else
-          config['css_path'] = ''
-          config['css_paths'] = config['css_paths'].collect { |path| "#{::Rails.root}/public#{path}" }
-        end
-        config
+      def initialize
+        validate_css_paths
+        format_css_paths
+      end
+
+      def config
+        @config ||= YAML.safe_load(ERB.new(File.read(configuration_file_path)).result, [], [], true)[::Rails.env]
       end
 
       private
@@ -27,7 +18,21 @@ def configuration_file_path
         @configuration_file_path ||= ::Rails.root.join('config', CONFIGURATION_FILENAME)
       end
 
-      def validate_css_path(config)
+      def format_css_paths
+        if config['css_path']
+          config['css_path']  = format_path(config['css_path'])
+          config['css_paths'] = []
+        else
+          config['css_path']  = ''
+          config['css_paths'] = config['css_paths'].collect { |path| format_path(path) }
+        end
+      end
+
+      def format_path(path)
+        "#{::Rails.root}/public#{path}"
+      end
+
+      def validate_css_paths
         if config['css_path'] && config['css_paths']
           raise LoadError, 'Cannot specify both css_path and css_paths'
         elsif config['css_paths'] && config['css_paths'].length != config['routes'].length