Skip to content

Add exception to prevent invalid configuration #48

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,11 @@ The generator adds the following files:

First, you'll need to configue a few things in the YAML file: `config/critical_path_css.yml`

**Note** that `manifest_name`, `css_path`, `css_paths` are all **mutually exclusive**; if using `css_path`, configuration for `manifest_name` AND `css_paths` should be omitted.

* `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).
* `css_paths`: If you have the need to specify multiple CSS source files, you can do so with `css_paths`. 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.

Expand Down
4 changes: 3 additions & 1 deletion lib/critical_path_css/rails/config_loader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ def format_path(path)
end

def validate_css_paths
if config['css_path'] && config['css_paths']
if config['manifest_name'] && (config['css_path'] || config['css_paths'])
raise LoadError, 'Cannot specify both manifest_name and css_path(s)'
elsif 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'
Expand Down
2 changes: 1 addition & 1 deletion lib/critical_path_css/rails/version.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module CriticalPathCSS
module Rails
VERSION = '4.0.0'.freeze
VERSION = '4.0.1'.freeze
end
end
10 changes: 10 additions & 0 deletions spec/fixtures/files/config/manifest-and-path-both-specified.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
defaults: &defaults
base_url: http://0.0.0.0:9292
manifest_name: application
css_path: /test.css

development:
<<: *defaults

test:
<<: *defaults
15 changes: 15 additions & 0 deletions spec/fixtures/files/config/manifest-and-paths-both-specified.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
defaults: &defaults
base_url: http://0.0.0.0:9292
manifest_name: application
css_paths:
- /test.css
- /test2.css
routes:
- /
- /new_route

development:
<<: *defaults

test:
<<: *defaults
16 changes: 16 additions & 0 deletions spec/lib/critical_path_css/rails/config_loader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,22 @@
end
end

context 'when manifest name and css path are both specified' do
let(:config_file) { file_fixture('config/manifest-and-path-both-specified.yml').read }

it 'raises an error' do
expect { subject }.to raise_error LoadError, 'Cannot specify both manifest_name and css_path(s)'
end
end

context 'when manifest name and css paths are both specified' do
let(:config_file) { file_fixture('config/manifest-and-paths-both-specified.yml').read }

it 'raises an error' do
expect { subject }.to raise_error LoadError, 'Cannot specify both manifest_name and css_path(s)'
end
end

context 'when single css_path and multiple css_paths are both specified' do
let(:config_file) { file_fixture('config/paths-both-specified.yml').read }

Expand Down