Merge pull request #32 from randallreedjr/multiple-css-paths

Support multiple css paths

Author: michael-misshore

README.md

@@ -54,6 +54,7 @@ First, you'll need to configue a few things in the YAML file: `config/critical_p
 
 * `manifest_name`: If you're using the asset pipeline, add the manifest name.
 * `css_path`: If you're not using the asset pipeline, you'll need to define the path to the application's main CSS. The gem assumes your CSS lives in `RAILS_ROOT/public`. If your main CSS file is in `RAILS_ROOT/public/assets/main.css`, you would set the variable to `/assets/main.css`.
+* `css_paths`: If you have the need to specify multiple CSS source files, you can do so with `css_paths`. Note that `css_path` and `css_paths` are **mutually exclusive**; if using `css_path`, configuration for `css_paths` should be omitted, and vice versa. When using this option, a separate CSS path must be specified for each route, and they will be matched based on the order specified (the first CSS path will be applied to the first route, the second CSS path to the second route, etc).
 * `routes`: List the routes that you would like to generate the critical CSS for. (i.e. /resources, /resources/show/1, etc.)
 * `base_url`: Add your application's URL for the necessary environments.
 
@@ -146,6 +147,12 @@ This gem is to be tested inside of docker/docker-compose. [Combustion](https://g
 
 Once shell'd in, run `bundle exec rspec spec` to run the test. The test rails app lives in `spec/internal`, and it can be viewed locally at `http://localhost:9292/`
 
+If you encounter Chromium errors trying to run the tests, installing [Puppeteer](https://github.com/GoogleChrome/puppeteer) might help.
+
+```Bash
+  npm install puppeteer
+```
+
 
 ## Versions
 

lib/config/critical_path_css.yml

@@ -3,6 +3,9 @@ defaults: &defaults
   manifest_name: application
   # Else provide the relative path of your CSS file from the /public directory
   # css_path: /path/to/css/from/public/main.css
+  # Or provide a separate path for each route
+  # css_paths:
+  #   - /path/to/css/from/public/main.css
   routes:
     - /
 

lib/critical_path_css/configuration.rb

@@ -13,6 +13,10 @@ def css_path
       @config['css_path']
     end
 
+    def css_paths
+      @config['css_paths']
+    end
+
     def manifest_name
       @config['manifest_name']
     end

lib/critical_path_css/css_fetcher.rb

@@ -10,7 +10,10 @@ def initialize(config)
     end
 
     def fetch
-      @config.routes.map { |route| [route, css_for_route(route)] }.to_h
+      @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
     end
 
     def fetch_route(route)
@@ -19,10 +22,10 @@ def fetch_route(route)
 
     protected
 
-    def css_for_route(route)
+    def css_for_route(route, css_path)
       options = {
         'url' => @config.base_url + route,
-        'css' => @config.css_path,
+        'css' => css_path,
         ## optional params
         # viewport dimensions
         'width' => 1300,

lib/critical_path_css/rails/config_loader.rb

@@ -5,12 +5,19 @@ class ConfigLoader
 
       def load
         config = YAML.safe_load(ERB.new(File.read(configuration_file_path)).result, [], [], true)[::Rails.env]
-        config['css_path'] = "#{::Rails.root}/public" + (
+        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
       end
 
@@ -19,6 +26,14 @@ def load
       def configuration_file_path
         @configuration_file_path ||= ::Rails.root.join('config', CONFIGURATION_FILENAME)
       end
+
+      def validate_css_path(config)
+        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
+          raise LoadError, 'Must specify css_paths for each route'
+        end
+      end
     end
   end
 end

lib/critical_path_css/rails/version.rb

@@ -1,5 +1,5 @@
 module CriticalPathCSS
   module Rails
-    VERSION = '2.10.1'.freeze
+    VERSION = '3.0.0'.freeze
   end
 end

spec/internal/app/views/root/index.html.erb

@@ -1 +1,2 @@
+<h1>Critical Path CSS Rails Test App</h1>
 <p><%= CriticalPathCss.fetch(request.path) %></p>

spec/lib/critical_path_css/css_fetcher_spec.rb

@@ -0,0 +1,63 @@
+require 'spec_helper'
+
+RSpec.describe 'CssFetcher' do
+  describe '#fetch' do
+    let(:subject) { CriticalPathCss::CssFetcher.new(config) }
+    let(:response) {
+      ['foo','', OpenStruct.new(exitstatus: 0)]
+    }
+    let(:routes) { ['/', '/new_route'] }
+    let(:config) do
+      OpenStruct.new(
+        base_url: 'http://0.0.0.0:9292',
+        css_path: css_path,
+        css_paths: css_paths,
+        penthouse_options: {},
+        routes: routes
+      )
+    end
+
+    context 'when a single css_path is configured' do
+      let(:css_path) { '/test.css' }
+      let(:css_paths) { [] }
+
+      it 'generates css for each route from the same file' do
+        expect(Open3).to receive(:capture3) do |arg1, arg2, arg3|
+          options = JSON.parse(arg3)
+          expect(options['css']).to eq '/test.css'
+        end.twice.and_return(response)
+        subject.fetch
+      end
+    end
+
+    context 'when multiple css_paths are configured' do
+      let(:css_path) { '' }
+      let(:css_paths) { ['/test.css', '/test2.css'] }
+
+      it 'generates css for each route from the respective file' do
+        expect(Open3).to receive(:capture3) do |arg1, arg2, arg3|
+          options = JSON.parse(arg3)
+          expect(options['css']).to eq '/test.css' if options['url'] == 'http://0.0.0.0:9292/'
+          expect(options['css']).to eq '/test2.css' if options['url'] == 'http://0.0.0.0:9292/new_route'
+        end.twice.and_return(response)
+        subject.fetch
+      end
+    end
+
+    context 'when same css file applies to multiple routes' do
+      let(:css_path) { '' }
+      let(:css_paths) { ['/test.css', '/test2.css', '/test.css'] }
+      let(:routes) { ['/', '/new_route', '/newer_route'] }
+
+      it 'generates css for each route from the respective file' do
+        expect(Open3).to receive(:capture3) do |arg1, arg2, arg3|
+          options = JSON.parse(arg3)
+          expect(options['css']).to eq '/test.css' if options['url'] == 'http://0.0.0.0:9292/'
+          expect(options['css']).to eq '/test2.css' if options['url'] == 'http://0.0.0.0:9292/new_route'
+          expect(options['css']).to eq '/test.css' if options['url'] == 'http://0.0.0.0:9292/newer_route'
+        end.thrice.and_return(response)
+        subject.fetch
+      end
+    end
+  end
+end

spec/lib/critical_path_css/rails/config_loader_spec.rb

@@ -0,0 +1,125 @@
+require 'spec_helper'
+
+RSpec.describe 'ConfigLoader' do
+  let(:subject) { CriticalPathCss::Rails::ConfigLoader.new }
+  describe '#load' do
+    before do
+      allow(File).to receive(:read).and_return(config_file)
+    end
+
+    context 'when single css_path is specified' do
+      let(:config_file) {
+        <<~CONFIG
+          defaults: &defaults
+            base_url: http://0.0.0.0:9292
+            css_path: /test.css
+            routes:
+              - /
+
+          development:
+            <<: *defaults
+
+          test:
+            <<: *defaults
+        CONFIG
+      }
+
+      it 'sets css_path with the path' do
+        config = subject.load
+
+        expect(config['css_path']).to eq '/app/spec/internal/public/test.css'
+      end
+
+      it 'leaves css_paths empty' do
+        config = subject.load
+
+        expect(config['css_paths']).to eq []
+      end
+    end
+
+    context 'when multiple css_paths are specified' do
+      let(:config_file) {
+        <<~CONFIG
+          defaults: &defaults
+            base_url: http://0.0.0.0:9292
+            css_paths:
+              - /test.css
+              - /test2.css
+            routes:
+              - /
+              - /new_route
+
+          development:
+            <<: *defaults
+
+          test:
+            <<: *defaults
+        CONFIG
+      }
+
+      it 'sets css_path to empty string' do
+        config = subject.load
+
+        expect(config['css_path']).to eq ''
+      end
+
+      it 'leaves css_paths to an array of paths' do
+        config = subject.load
+
+        expect(config['css_paths']).to eq ['/app/spec/internal/public/test.css','/app/spec/internal/public/test2.css']
+      end
+    end
+
+    context 'when single css_path and multiple css_paths are both specified' do
+      let(:config_file) {
+        <<~CONFIG
+          defaults: &defaults
+            base_url: http://0.0.0.0:9292
+            css_path: /test.css
+            css_paths:
+              - /test.css
+              - /test2.css
+            routes:
+              - /
+              - /new_route
+
+          development:
+            <<: *defaults
+
+          test:
+            <<: *defaults
+        CONFIG
+      }
+
+      it 'raises an error' do
+        expect { subject.load }.to raise_error LoadError, 'Cannot specify both css_path and css_paths'
+      end
+    end
+
+    context 'when css_paths and routes are not the same length' do
+      let(:config_file) {
+        <<~CONFIG
+          defaults: &defaults
+            base_url: http://0.0.0.0:9292
+            css_paths:
+              - /test.css
+              - /test2.css
+            routes:
+              - /
+              - /new_route
+              - /newer_route
+
+          development:
+            <<: *defaults
+
+          test:
+            <<: *defaults
+        CONFIG
+      }
+
+      it 'raises an error' do
+        expect { subject.load }.to raise_error LoadError, 'Must specify css_paths for each route'
+      end
+    end
+  end
+end