json_only option should allow multiple locales in one file (#531)

* - `json_only` option should allow multiple locales.
- Refactor segment writer code to use formatter classes. This can be extended in the future if needed to support other formats easily.

* RSpecs: should --> expect

CHANGELOG.md

@@ -11,7 +11,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Changed
 
-- Nothing
+- [Ruby] `json_only` option should allow multiple locales.
+- [Ruby] Simplified and cleaned code related to JS/JSON formatting.
 
 ### Fixed
 

lib/i18n/js/formatters/base.rb

@@ -0,0 +1,23 @@
+module I18n
+  module JS
+    module Formatters
+      class Base
+        def initialize(js_extend: false, namespace: nil, pretty_print: false)
+          @js_extend    = js_extend
+          @namespace    = namespace
+          @pretty_print = pretty_print
+        end
+
+        protected
+
+        def format_json(translations)
+          if @pretty_print
+            ::JSON.pretty_generate(translations)
+          else
+            translations.to_json
+          end
+        end
+      end
+    end
+  end
+end

lib/i18n/js/formatters/js.rb

@@ -0,0 +1,31 @@
+require "i18n/js/formatters/base"
+
+module I18n
+  module JS
+    module Formatters
+      class JS < Base
+        def format(translations)
+          contents = header
+          translations.each do |locale, translations_for_locale|
+            contents << line(locale, format_json(translations_for_locale))
+          end
+          contents
+        end
+
+        protected
+
+        def header
+          %(#{@namespace}.translations || (#{@namespace}.translations = {});\n)
+        end
+
+        def line(locale, translations)
+          if @js_extend
+            %(#{@namespace}.translations["#{locale}"] = I18n.extend((#{@namespace}.translations["#{locale}"] || {}), #{translations});\n)
+          else
+            %(#{@namespace}.translations["#{locale}"] = #{translations};\n)
+          end
+        end
+      end
+    end
+  end
+end

lib/i18n/js/formatters/json.rb

@@ -0,0 +1,13 @@
+require "i18n/js/formatters/base"
+
+module I18n
+  module JS
+    module Formatters
+      class JSON < Base
+        def format(translations)
+          format_json(translations)
+        end
+      end
+    end
+  end
+end

lib/i18n/js/segment.rb

@@ -1,5 +1,6 @@
 require "i18n/js/private/hash_with_symbol_keys"
-require "i18n/js/errors"
+require "i18n/js/formatters/js"
+require "i18n/js/formatters/json"
 
 module I18n
   module JS
@@ -34,21 +35,20 @@ def save!
             write_file(file_for_locale(locale), @translations.slice(locale))
           end
         else
-          if @json_only
-            raise I18n::JS::JsonOnlyLocaleRequiredError
-          end
           write_file
         end
       end
 
       protected
 
+      def file_for_locale(locale)
+        @file.gsub(LOCALE_INTERPOLATOR, locale.to_s)
+      end
+
       def write_file(_file = @file, _translations = @translations)
         FileUtils.mkdir_p File.dirname(_file)
-        contents = js_header
-        _translations.each do |locale, translations_for_locale|
-          contents << js_translations(locale, translations_for_locale)
-        end
+        _translations = Utils.deep_key_sort(_translations) if @sort_translation_keys
+        contents = formatter.format(_translations)
 
         return if File.exist?(_file) && File.read(_file) == contents
 
@@ -57,42 +57,18 @@ def write_file(_file = @file, _translations = @translations)
         end
       end
 
-      def js_header
+      def formatter
         if @json_only
-          ''
+          Formatters::JSON.new(**formatter_options)
         else
-          %(#{@namespace}.translations || (#{@namespace}.translations = {});\n)
+          Formatters::JS.new(**formatter_options)
         end
       end
 
-      def js_translations(locale, translations)
-        translations = Utils.deep_key_sort(translations) if @sort_translation_keys
-        translations = print_json(translations)
-        js_translations_line(locale, translations)
-      end
-
-      def js_translations_line(locale, translations)
-        if @json_only
-          %({"#{locale}":#{translations}})
-        elsif @js_extend
-          %(#{@namespace}.translations["#{locale}"] = I18n.extend((#{@namespace}.translations["#{locale}"] || {}), #{translations});\n)
-        else
-          %(#{@namespace}.translations["#{locale}"] = #{translations};\n)
-        end
-      end
-
-      # Outputs pretty or ugly JSON depending on :pretty_print option
-      def print_json(translations)
-        if @pretty_print
-          JSON.pretty_generate(translations)
-        else
-          translations.to_json
-        end
-      end
-
-      # interpolates filename
-      def file_for_locale(locale)
-        @file.gsub(LOCALE_INTERPOLATOR, locale.to_s)
+      def formatter_options
+        { js_extend: @js_extend,
+          namespace: @namespace,
+          pretty_print: @pretty_print }
       end
     end
   end

spec/fixtures/json_only.yml

@@ -6,3 +6,13 @@ translations:
     only:
       - "*.date.formats.*"
       - "*.number.currency.*"
+
+  - file: "tmp/i18n-js/json_only_multi.js"
+    only:
+      - "*.date.formats.*"
+      - "*.number.currency.*"
+
+  - file: "tmp/i18n-js/json_only_multi_pretty.js"
+    only:
+      - "*.date.formats.*"
+      - "*.number.currency.*"

spec/ruby/i18n/js/segment_spec.rb

@@ -64,19 +64,20 @@
   describe "#saving!" do
     before { allow(I18n::JS).to receive(:export_i18n_js_dir_path).and_return(temp_path) }
 
-    context "when json_only is true" do
+    context "when json_only is true with locale" do
       let(:file){ "tmp/i18n-js/%{locale}.js" }
       let(:json_only){ true }
-      it 'should output the keys as sorted' do
+
+      it 'should output JSON files per locale' do
         subject.save!
         file_should_exist "en.js"
         file_should_exist "fr.js"
 
-        expect(File.open(File.join(temp_path, "en.js")){|f| f.read}).to eql(
+        expect(File.read(File.join(temp_path, "en.js"))).to eql(
           %Q({"en":{"test":"Test"}})
         )
 
-        expect(File.open(File.join(temp_path, "fr.js")){|f| f.read}).to eql(
+        expect(File.read(File.join(temp_path, "fr.js"))).to eql(
           %Q({"fr":{"test":"Test2"}})
         )
       end
@@ -85,8 +86,37 @@
     context "when json_only is true without locale" do
       let(:file){ "tmp/i18n-js/segment.js" }
       let(:json_only){ true }
-      it 'should output the keys as sorted' do
-        expect { subject.save! }.to raise_error(I18n::JS::JsonOnlyLocaleRequiredError)
+
+      it 'should output one JSON file for all locales' do
+        subject.save!
+        file_should_exist "segment.js"
+
+        expect(File.read(File.join(temp_path, "segment.js"))).to eql(
+          %Q({"en":{"test":"Test"},"fr":{"test":"Test2"}})
+        )
+      end
+    end
+
+    context "when json_only and pretty print are true" do
+      let(:file){ "tmp/i18n-js/segment.js" }
+      let(:json_only){ true }
+      let(:pretty_print){ true }
+
+      it 'should output one JSON file for all locales' do
+        subject.save!
+        file_should_exist "segment.js"
+
+        expect(File.read(File.join(temp_path, "segment.js"))).to eql <<-EOS
+{
+  "en": {
+    "test": "Test"
+  },
+  "fr": {
+    "test": "Test2"
+  }
+}
+EOS
+.chomp
       end
     end
   end