Skip to content
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

Improved configuration checks #967

Merged
merged 1 commit into from
Oct 28, 2017
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
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