Skip to content

Commit 4c58d70

Browse files
committed
Support multiple css paths
* Ensure only one css_path configuration is used * Raise error if length of css_paths does not match routes * Add comment in config file about css_paths
1 parent 1780066 commit 4c58d70

File tree

7 files changed

+219
-6
lines changed

7 files changed

+219
-6
lines changed

lib/config/critical_path_css.yml

+3
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ defaults: &defaults
33
manifest_name: application
44
# Else provide the relative path of your CSS file from the /public directory
55
# css_path: /path/to/css/from/public/main.css
6+
# Or provide a separate path for each route
7+
# css_paths:
8+
# - /path/to/css/from/public/main.css
69
routes:
710
- /
811

lib/critical_path_css/configuration.rb

+4
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ def css_path
1313
@config['css_path']
1414
end
1515

16+
def css_paths
17+
@config['css_paths']
18+
end
19+
1620
def manifest_name
1721
@config['manifest_name']
1822
end

lib/critical_path_css/css_fetcher.rb

+6-3
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ def initialize(config)
1010
end
1111

1212
def fetch
13-
@config.routes.map { |route| [route, css_for_route(route)] }.to_h
13+
@config.routes.map.with_index { |route, index|
14+
css_path = @config.css_paths[index].present? ? @config.css_paths[index] : @config.css_path
15+
[route, css_for_route(route, css_path)]
16+
}.to_h
1417
end
1518

1619
def fetch_route(route)
@@ -19,10 +22,10 @@ def fetch_route(route)
1922

2023
protected
2124

22-
def css_for_route(route)
25+
def css_for_route(route, css_path)
2326
options = {
2427
'url' => @config.base_url + route,
25-
'css' => @config.css_path,
28+
'css' => css_path,
2629
## optional params
2730
# viewport dimensions
2831
'width' => 1300,

lib/critical_path_css/rails/config_loader.rb

+17-2
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,19 @@ class ConfigLoader
55

66
def load
77
config = YAML.safe_load(ERB.new(File.read(configuration_file_path)).result, [], [], true)[::Rails.env]
8-
config['css_path'] = "#{::Rails.root}/public" + (
8+
validate_css_path config
9+
if config['css_path']
10+
config['css_path'] = "#{::Rails.root}/public" + (
911
config['css_path'] ||
1012
ActionController::Base.helpers.stylesheet_path(
1113
config['manifest_name'], host: ''
1214
)
13-
)
15+
)
16+
config['css_paths'] = []
17+
else
18+
config['css_path'] = ''
19+
config['css_paths'] = config['css_paths'].collect { |path| "#{::Rails.root}/public#{path}" }
20+
end
1421
config
1522
end
1623

@@ -19,6 +26,14 @@ def load
1926
def configuration_file_path
2027
@configuration_file_path ||= ::Rails.root.join('config', CONFIGURATION_FILENAME)
2128
end
29+
30+
def validate_css_path(config)
31+
if config['css_path'] && config['css_paths']
32+
raise LoadError, 'Cannot specify both css_path and css_paths'
33+
elsif config['css_paths'] && config['css_paths'].length != config['routes'].length
34+
raise LoadError, 'Must specify css_paths for each route'
35+
end
36+
end
2237
end
2338
end
2439
end
+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
module CriticalPathCSS
22
module Rails
3-
VERSION = '2.6.0'.freeze
3+
VERSION = '2.7.0'.freeze
44
end
55
end
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
require 'spec_helper'
2+
3+
RSpec.describe 'CssFetcher' do
4+
describe '#fetch' do
5+
let(:subject) { CriticalPathCss::CssFetcher.new(config) }
6+
let(:response) {
7+
['foo','', OpenStruct.new(exitstatus: 0)]
8+
}
9+
let(:routes) { ['/', '/new_route'] }
10+
let(:config) do
11+
OpenStruct.new(
12+
base_url: 'http://0.0.0.0:9292',
13+
css_path: css_path,
14+
css_paths: css_paths,
15+
penthouse_options: {},
16+
routes: routes
17+
)
18+
end
19+
20+
context 'when a single css_path is configured' do
21+
let(:css_path) { '/test.css' }
22+
let(:css_paths) { [] }
23+
24+
it 'generates css for each route from the same file' do
25+
expect(Open3).to receive(:capture3) do |arg1, arg2, arg3|
26+
options = JSON.parse(arg3)
27+
expect(options['css']).to eq '/test.css'
28+
end.twice.and_return(response)
29+
subject.fetch
30+
end
31+
end
32+
33+
context 'when multiple css_paths are configured' do
34+
let(:css_path) { '' }
35+
let(:css_paths) { ['/test.css', '/test2.css'] }
36+
37+
it 'generates css for each route from the respective file' do
38+
expect(Open3).to receive(:capture3) do |arg1, arg2, arg3|
39+
options = JSON.parse(arg3)
40+
expect(options['css']).to eq '/test.css' if options['url'] == 'http://0.0.0.0:9292/'
41+
expect(options['css']).to eq '/test2.css' if options['url'] == 'http://0.0.0.0:9292/new_route'
42+
end.twice.and_return(response)
43+
subject.fetch
44+
end
45+
end
46+
47+
context 'when same css file applies to multiple routes' do
48+
let(:css_path) { '' }
49+
let(:css_paths) { ['/test.css', '/test2.css', '/test.css'] }
50+
let(:routes) { ['/', '/new_route', '/newer_route'] }
51+
52+
it 'generates css for each route from the respective file' do
53+
expect(Open3).to receive(:capture3) do |arg1, arg2, arg3|
54+
options = JSON.parse(arg3)
55+
expect(options['css']).to eq '/test.css' if options['url'] == 'http://0.0.0.0:9292/'
56+
expect(options['css']).to eq '/test2.css' if options['url'] == 'http://0.0.0.0:9292/new_route'
57+
expect(options['css']).to eq '/test.css' if options['url'] == 'http://0.0.0.0:9292/newer_route'
58+
end.thrice.and_return(response)
59+
subject.fetch
60+
end
61+
end
62+
end
63+
end
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
require 'spec_helper'
2+
3+
RSpec.describe 'ConfigLoader' do
4+
let(:subject) { CriticalPathCss::Rails::ConfigLoader.new }
5+
describe '#load' do
6+
before do
7+
allow(File).to receive(:read).and_return(config_file)
8+
end
9+
10+
context 'when single css_path is specified' do
11+
let(:config_file) {
12+
<<~CONFIG
13+
defaults: &defaults
14+
base_url: http://0.0.0.0:9292
15+
css_path: /test.css
16+
routes:
17+
- /
18+
19+
development:
20+
<<: *defaults
21+
22+
test:
23+
<<: *defaults
24+
CONFIG
25+
}
26+
27+
it 'sets css_path with the path' do
28+
config = subject.load
29+
30+
expect(config['css_path']).to eq '/app/spec/internal/public/test.css'
31+
end
32+
33+
it 'leaves css_paths empty' do
34+
config = subject.load
35+
36+
expect(config['css_paths']).to eq []
37+
end
38+
end
39+
40+
context 'when multiple css_paths are specified' do
41+
let(:config_file) {
42+
<<~CONFIG
43+
defaults: &defaults
44+
base_url: http://0.0.0.0:9292
45+
css_paths:
46+
- /test.css
47+
- /test2.css
48+
routes:
49+
- /
50+
- /new_route
51+
52+
development:
53+
<<: *defaults
54+
55+
test:
56+
<<: *defaults
57+
CONFIG
58+
}
59+
60+
it 'sets css_path to empty string' do
61+
config = subject.load
62+
63+
expect(config['css_path']).to eq ''
64+
end
65+
66+
it 'leaves css_paths to an array of paths' do
67+
config = subject.load
68+
69+
expect(config['css_paths']).to eq ['/app/spec/internal/public/test.css','/app/spec/internal/public/test2.css']
70+
end
71+
end
72+
73+
context 'when single css_path and multiple css_paths are both specified' do
74+
let(:config_file) {
75+
<<~CONFIG
76+
defaults: &defaults
77+
base_url: http://0.0.0.0:9292
78+
css_path: /test.css
79+
css_paths:
80+
- /test.css
81+
- /test2.css
82+
routes:
83+
- /
84+
- /new_route
85+
86+
development:
87+
<<: *defaults
88+
89+
test:
90+
<<: *defaults
91+
CONFIG
92+
}
93+
94+
it 'raises an error' do
95+
expect { subject.load }.to raise_error LoadError, 'Cannot specify both css_path and css_paths'
96+
end
97+
end
98+
99+
context 'when css_paths and routes are not the same length' do
100+
let(:config_file) {
101+
<<~CONFIG
102+
defaults: &defaults
103+
base_url: http://0.0.0.0:9292
104+
css_paths:
105+
- /test.css
106+
- /test2.css
107+
routes:
108+
- /
109+
- /new_route
110+
- /newer_route
111+
112+
development:
113+
<<: *defaults
114+
115+
test:
116+
<<: *defaults
117+
CONFIG
118+
}
119+
120+
it 'raises an error' do
121+
expect { subject.load }.to raise_error LoadError, 'Must specify css_paths for each route'
122+
end
123+
end
124+
end
125+
end

0 commit comments

Comments
 (0)