From 1371f5e27e665d8a074365c7d78d3a15b1af8835 Mon Sep 17 00:00:00 2001 From: Michael Misshore Date: Mon, 19 Aug 2019 15:27:18 -0500 Subject: [PATCH] Add exception to prevent invalid configuration manifest and css path(s) cannot/should not be both defined --- README.md | 4 +++- lib/critical_path_css/rails/config_loader.rb | 4 +++- lib/critical_path_css/rails/version.rb | 2 +- .../config/manifest-and-path-both-specified.yml | 10 ++++++++++ .../config/manifest-and-paths-both-specified.yml | 15 +++++++++++++++ .../rails/config_loader_spec.rb | 16 ++++++++++++++++ 6 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 spec/fixtures/files/config/manifest-and-path-both-specified.yml create mode 100644 spec/fixtures/files/config/manifest-and-paths-both-specified.yml diff --git a/README.md b/README.md index 9722182..7d692f9 100644 --- a/README.md +++ b/README.md @@ -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. diff --git a/lib/critical_path_css/rails/config_loader.rb b/lib/critical_path_css/rails/config_loader.rb index d8dae0f..1158a1b 100644 --- a/lib/critical_path_css/rails/config_loader.rb +++ b/lib/critical_path_css/rails/config_loader.rb @@ -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' diff --git a/lib/critical_path_css/rails/version.rb b/lib/critical_path_css/rails/version.rb index 2ee8821..83f3347 100644 --- a/lib/critical_path_css/rails/version.rb +++ b/lib/critical_path_css/rails/version.rb @@ -1,5 +1,5 @@ module CriticalPathCSS module Rails - VERSION = '4.0.0'.freeze + VERSION = '4.0.1'.freeze end end diff --git a/spec/fixtures/files/config/manifest-and-path-both-specified.yml b/spec/fixtures/files/config/manifest-and-path-both-specified.yml new file mode 100644 index 0000000..8575dce --- /dev/null +++ b/spec/fixtures/files/config/manifest-and-path-both-specified.yml @@ -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 \ No newline at end of file diff --git a/spec/fixtures/files/config/manifest-and-paths-both-specified.yml b/spec/fixtures/files/config/manifest-and-paths-both-specified.yml new file mode 100644 index 0000000..990a290 --- /dev/null +++ b/spec/fixtures/files/config/manifest-and-paths-both-specified.yml @@ -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 \ No newline at end of file diff --git a/spec/lib/critical_path_css/rails/config_loader_spec.rb b/spec/lib/critical_path_css/rails/config_loader_spec.rb index e20c289..dc7bfac 100644 --- a/spec/lib/critical_path_css/rails/config_loader_spec.rb +++ b/spec/lib/critical_path_css/rails/config_loader_spec.rb @@ -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 }