Skip to content

Commit

Permalink
fixing configuration checks (#967)
Browse files Browse the repository at this point in the history
fixed and improved configuration checks broken by #899
  • Loading branch information
Anatolii Didukh authored and justin808 committed Oct 28, 2017
1 parent cc9db6d commit 882cc5d
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 47 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Changes since last non-beta release.
- Fixed `react_component_hash` functionality in cases of prerendering errors: [PR 960](https://github.com/shakacode/react_on_rails/pull/960) by [Judahmeek](https://github.com/Judahmeek).
- Fix to add missing dependency to run generator spec individually: [PR 962](https://github.com/shakacode/react_on_rails/pull/962) by [tricknotes](https://github.com/tricknotes).
- Fixes check for i18n_dir in LocalesToJs returning false when i18n_dir was set. [PR 899](https://github.com/shakacode/react_on_rails/pull/899) by [hakongit](https://github.com/hakongit).
- Fixed and improved I18n directories checks: [PR 967](https://github.com/shakacode/react_on_rails/pull/967) by [railsme](https://github.com/railsme)

### [10.0.0] - 2017-10-08
#### Created
Expand Down
6 changes: 2 additions & 4 deletions lib/react_on_rails/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def self.setup_config_values
end

def self.check_i18n_directory_exists
return if @configuration.i18n_dir.blank?
return if @configuration.i18n_dir.nil?
return if Dir.exist?(@configuration.i18n_dir)

raise "Error configuring /config/react_on_rails.rb: invalid value for `config.i18n_dir`. "\
Expand All @@ -30,7 +30,7 @@ def self.check_i18n_directory_exists
end

def self.check_i18n_yml_directory_exists
return if @configuration.i18n_yml_dir.blank?
return if @configuration.i18n_yml_dir.nil?
return if Dir.exist?(@configuration.i18n_yml_dir)

raise "Error configuring /config/react_on_rails.rb: invalid value for `config.i18n_yml_dir`. "\
Expand Down Expand Up @@ -105,8 +105,6 @@ def self.configuration
server_render_method: "ExecJS",
symlink_non_digested_assets_regex: nil,
build_test_command: "",
i18n_dir: "",
i18n_yml_dir: "",
build_production_command: ""
)
end
Expand Down
5 changes: 1 addition & 4 deletions lib/react_on_rails/locales_to_js.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@
module ReactOnRails
class LocalesToJs
def initialize
if !i18n_dir.nil? && !File.directory?(i18n_dir)
raise "config.i18n_dir is set and it is not a directory. Did you set config.i18n_dir in the react_on_rails initializer?"
end
return if i18n_yml_dir.nil?
return if i18n_dir.nil?
return unless obsolete?
@translations, @defaults = generate_translations
convert
Expand Down
93 changes: 61 additions & 32 deletions spec/react_on_rails/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,42 +4,71 @@

module ReactOnRails
RSpec.describe Configuration do
it "raises if the i18n directory does not exist" do
junk_name = "/XXXX/junkXXXX"
expect do
ReactOnRails.configure do |config|
config.i18n_dir = junk_name
end
end.to raise_error(/#{junk_name}/)
end
let(:existing_path) { Pathname.new(Dir.mktmpdir) }
let(:not_existing_path) { "/path/to/#{SecureRandom.hex(4)}" }

it "does not raises if the i18n directory does exist" do
dir = __dir__
expect do
ReactOnRails.configure do |config|
config.i18n_dir = dir
end
end.to_not raise_error
expect(ReactOnRails.configuration.i18n_dir).to eq(dir)
end
describe ".i18n_dir" do
let(:i18n_dir) { existing_path }

it "raises if the i18n yaml directory does not exist" do
junk_name = "/YYYY/junkYYYY"
expect do
ReactOnRails.configure do |config|
config.i18n_yml_dir = junk_name
end
end.to raise_error(/#{junk_name}/)
after do
ReactOnRails.configure { |config| config.i18n_dir = nil }
end

it "passes if directory exists" do
expect do
ReactOnRails.configure do |config|
config.i18n_dir = i18n_dir
end
end.not_to raise_error
end

it "fails with empty string value" do
expect do
ReactOnRails.configure do |config|
config.i18n_dir = ""
end
end.to raise_error(RuntimeError, /invalid value for `config\.i18n_dir`/)
end

it "fails with not existing directory" do
expect do
ReactOnRails.configure do |config|
config.i18n_dir = not_existing_path
end
end.to raise_error(RuntimeError, /invalid value for `config\.i18n_dir`/)
end
end

it "does not raises if the i18n yaml directory does exist" do
dir = __dir__
expect do
ReactOnRails.configure do |config|
config.i18n_yml_dir = dir
end
end.to_not raise_error
expect(ReactOnRails.configuration.i18n_yml_dir).to eq(dir)
describe ".i18n_yml_dir" do
let(:i18n_yml_dir) { existing_path }

after do
ReactOnRails.configure { |config| config.i18n_yml_dir = nil }
end

it "passes if directory exists" do
expect do
ReactOnRails.configure do |config|
config.i18n_yml_dir = i18n_yml_dir
end
end.not_to raise_error
end

it "fails with empty string value" do
expect do
ReactOnRails.configure do |config|
config.i18n_yml_dir = ""
end
end.to raise_error(RuntimeError, /invalid value for `config\.i18n_yml_dir`/)
end

it "fails with not existing directory" do
expect do
ReactOnRails.configure do |config|
config.i18n_yml_dir = not_existing_path
end
end.to raise_error(RuntimeError, /invalid value for `config\.i18n_yml_dir`/)
end
end

it "be able to config default configuration of the gem" do
Expand Down
7 changes: 0 additions & 7 deletions spec/react_on_rails/locales_to_js_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,6 @@ module ReactOnRails
let(:translations_path) { "#{i18n_dir}/translations.js" }
let(:default_path) { "#{i18n_dir}/default.js" }

it "with i18n_dir set to ''" do
ReactOnRails.configure do |config|
config.i18n_dir = ''
end
expect { ReactOnRails::LocalesToJs.new }.to raise_error(/config.i18n_dir is set and it is not a directory. Did you set config.i18n_dir in the react_on_rails initializer?/)
end

shared_examples "locale to js" do
context "with obsolete js files" do
before do
Expand Down

0 comments on commit 882cc5d

Please sign in to comment.