FEATURE: Load translation overrides without JS eval

app/assets/javascripts/discourse/initializers/localization.js.es6

@@ -1,5 +1,3 @@
-import PreloadStore from "preload-store";
-
 export default {
   name: "localization",
   after: "inject-objects",
@@ -21,20 +19,9 @@ export default {
     }
 
     // Merge any overrides into our object
-    const overrides = PreloadStore.get("translationOverrides") || {};
+    const overrides = I18n._overrides || {};
     Object.keys(overrides).forEach(k => {
       const v = overrides[k];
-
-      // Special case: Message format keys are functions
-      if (/_MF$/.test(k)) {
-        k = k.replace(/^[a-z_]*js\./, "");
-        I18n._compiledMFs[k] = new Function(
-          "transKey",
-          `return (${v})(transKey);`
-        );
-        return;
-      }
-
       k = k.replace("admin_js", "js");
 
       const segs = k.split(".");
@@ -52,6 +39,14 @@ export default {
       }
     });
 
+    const mfOverrides = I18n._mfOverrides || {};
+    Object.keys(mfOverrides).forEach(k => {
+      const v = mfOverrides[k];
+
+      k = k.replace(/^[a-z_]*js\./, "");
+      I18n._compiledMFs[k] = v;
+    });
+
     bootbox.addLocale(I18n.currentLocale(), {
       OK: I18n.t("composer.modal_ok"),
       CANCEL: I18n.t("composer.modal_cancel"),

app/controllers/application_controller.rb

@@ -520,7 +520,6 @@ class ApplicationController < ActionController::Base
     store_preloaded("customHTML", custom_html_json)
     store_preloaded("banner", banner_json)
     store_preloaded("customEmoji", custom_emoji)
-    store_preloaded("translationOverrides", I18n.client_overrides_json(I18n.locale))
   end
 
   def preload_current_user_data

app/controllers/extra_locales_controller.rb

@@ -8,57 +8,64 @@ class ExtraLocalesController < ApplicationController
     :redirect_to_login_if_required,
     :verify_authenticity_token
 
+  OVERRIDES_BUNDLE ||= 'overrides'
+
   def show
     bundle = params[:bundle]
 
-    raise Discourse::InvalidAccess.new if bundle !~ /^(admin|wizard)$/ || !current_user&.staff?
+    raise Discourse::InvalidAccess.new if !valid_bundle?(bundle)
 
     if params[:v]&.size == 32
       hash = ExtraLocalesController.bundle_js_hash(bundle)
-      immutable_for(24.hours) if hash == params[:v]
+      immutable_for(1.year) if hash == params[:v]
     end
 
     render plain: ExtraLocalesController.bundle_js(bundle), content_type: "application/javascript"
   end
 
   def self.bundle_js_hash(bundle)
-    @bundle_js_hash ||= {}
+    if bundle == OVERRIDES_BUNDLE
-    @bundle_js_hash["#{bundle}_#{I18n.locale}"] ||= Digest::MD5.hexdigest(bundle_js(bundle))
+      site = RailsMultisite::ConnectionManagement.current_db
+
+      @by_site ||= {}
+      @by_site[site] ||= {}
+      @by_site[site][I18n.locale] ||= begin
+        js = bundle_js(bundle)
+        js.present? ? Digest::MD5.hexdigest(js) : nil
+      end
+    else
+      @bundle_js_hash ||= {}
+      @bundle_js_hash["#{bundle}_#{I18n.locale}"] ||= Digest::MD5.hexdigest(bundle_js(bundle))
+    end
   end
 
   def self.url(bundle)
-    if Rails.env == "production"
+    "#{Discourse.base_uri}/extra-locales/#{bundle}?v=#{bundle_js_hash(bundle)}"
-      "#{Discourse.base_uri}/extra-locales/#{bundle}?v=#{bundle_js_hash(bundle)}"
+  end
-    else
+
-      "#{Discourse.base_uri}/extra-locales/#{bundle}"
+  def self.client_overrides_exist?
-    end
+    bundle_js_hash(OVERRIDES_BUNDLE).present?
   end
 
   def self.bundle_js(bundle)
     locale_str = I18n.locale.to_s
     bundle_str = "#{bundle}_js"
 
-    translations = JsLocaleHelper.translations_for(locale_str)
+    if bundle == OVERRIDES_BUNDLE
-
+      JsLocaleHelper.output_client_overrides(locale_str)
-    translations.keys.each do |l|
+    else
-      translations[l].keys.each do |k|
+      JsLocaleHelper.output_extra_locales(bundle_str, locale_str)
-        bundle_translations = translations[l].delete(k)
-        translations[l].deep_merge!(bundle_translations) if k == bundle_str
-      end
     end
+  end
 
-    js = ""
+  def self.clear_cache!
+    site = RailsMultisite::ConnectionManagement.current_db
+    @by_site&.delete(site)
+  end
 
-    if translations.present?
+  private
-      js = <<~JS.squish
-        (function() {
-          if (window.I18n) {
-            window.I18n.extras = #{translations.to_json};
-          }
-        })();
-      JS
-    end
 
-    js
+  def valid_bundle?(bundle)
+    bundle == OVERRIDES_BUNDLE || (bundle =~ /^(admin|wizard)$/ && current_user&.staff?)
   end
 end

app/helpers/application_helper.rb

@@ -93,9 +93,14 @@ module ApplicationHelper
 
   def preload_script(script)
     path = script_asset_path(script)
+    preload_script_url(path)
+  end
 
-"<link rel='preload' href='#{path}' as='script'/>
+  def preload_script_url(url)
-<script src='#{path}'></script>".html_safe
+    <<~HTML.html_safe
+      <link rel="preload" href="#{url}" as="script">
+      <script src="#{url}"></script>
+    HTML
   end
 
   def discourse_csrf_tags

app/models/translation_override.rb

@@ -39,6 +39,7 @@ class TranslationOverride < ActiveRecord::Base
 
   def self.i18n_changed(keys)
     I18n.reload!
+    ExtraLocalesController.clear_cache!
     MessageBus.publish('/i18n-flush', refresh: true)
 
     keys.flatten.each do |key|

app/views/layouts/application.html.erb

@@ -20,6 +20,9 @@
     <%= build_plugin_html 'server:before-script-load' %>
 
     <%= preload_script "locales/#{I18n.locale}" %>
+    <%- if ExtraLocalesController.client_overrides_exist? %>
+      <%= preload_script_url ExtraLocalesController.url('overrides') %>
+    <%- end %>
     <%= preload_script "ember_jquery" %>
     <%= preload_script "preload-store" %>
     <%= preload_script "vendor" %>
@@ -29,7 +32,7 @@
       <%= preload_script file %>
     <%- end %>
     <%- if staff? %>
-      <script src="<%= ExtraLocalesController.url('admin') %>"></script>
+      <%= preload_script_url ExtraLocalesController.url('admin') %>
       <%= preload_script "admin" %>
     <%- end %>
 

app/views/wizard/index.html.erb

@@ -3,9 +3,12 @@
     <%= discourse_stylesheet_link_tag :wizard, theme_ids: nil %>
     <%= preload_script 'ember_jquery' %>
     <%= preload_script "locales/#{I18n.locale}" %>
+    <%- if ExtraLocalesController.client_overrides_exist? %>
+      <%= preload_script_url ExtraLocalesController.url('overrides') %>
+    <%- end %>
     <%= preload_script 'wizard-vendor' %>
     <%= preload_script 'wizard-application' %>
-    <script src="<%= ExtraLocalesController.url("wizard") %>"></script>
+    <%= preload_script_url ExtraLocalesController.url('wizard') %>
     <%= csrf_meta_tags %>
 
     <%= render partial: "layouts/head" %>

config/initializers/100-i18n.rb

@@ -12,5 +12,8 @@ I18n.reload!
 I18n.init_accelerator!
 
 unless Rails.env.test?
-  MessageBus.subscribe("/i18n-flush") { I18n.reload! }
+  MessageBus.subscribe("/i18n-flush") do
+    I18n.reload!
+    ExtraLocalesController.clear_cache!
+  end
 end

lib/freedom_patches/translate_accelerator.rb

@@ -137,9 +137,10 @@ module I18n
 
     def overrides_by_locale(locale)
       return unless @overrides_enabled
-
       return {} if GlobalSetting.skip_db?
 
+      execute_reload if @requires_reload
+
       site = RailsMultisite::ConnectionManagement.current_db
 
       by_site = @overrides_by_site[site]
@@ -170,11 +171,6 @@ module I18n
       end
     end
 
-    def client_overrides_json(locale)
-      client_json = (overrides_by_locale(locale) || {}).select { |k, _| k[/^(admin_js|js)\./] }
-      MultiJson.dump(client_json)
-    end
-
     def translate(*args)
       execute_reload if @requires_reload
 

lib/js_locale_helper.rb

@@ -159,6 +159,39 @@ module JsLocaleHelper
     result
   end
 
+  def self.output_client_overrides(locale)
+    translations = (I18n.overrides_by_locale(locale) || {}).select { |k, _| k[/^(admin_js|js)\./] }
+    return "" if translations.blank?
+
+    message_formats = {}
+
+    translations.delete_if do |key, value|
+      if key.to_s.end_with?("_MF")
+        message_formats[key] = value
+      end
+    end
+
+    message_formats = message_formats.map { |k, v| "#{k.inspect}: #{v}" }.join(", ")
+
+    <<~JS
+      I18n._mfOverrides = {#{message_formats}};
+      I18n._overrides = #{translations.to_json};
+    JS
+  end
+
+  def self.output_extra_locales(bundle, locale)
+    translations = translations_for(locale)
+
+    translations.keys.each do |l|
+      translations[l].keys.each do |k|
+        bundle_translations = translations[l].delete(k)
+        translations[l].deep_merge!(bundle_translations) if k == bundle
+      end
+    end
+
+    translations.present? ? "I18n.extras = #{translations.to_json};" : ""
+  end
+
   MOMENT_LOCALE_MAPPING ||= {
     "hy" => "hy-am",
     "en" => "en-gb"

spec/components/freedom_patches/translate_accelerator_spec.rb

@@ -204,26 +204,5 @@ describe "translate accelerator" do
       override_translation('en', 'fish', 'fake fish')
       expect(Fish.model_name.human).to eq('Fish')
     end
-
-    describe "client json" do
-      it "is empty by default" do
-        expect(I18n.client_overrides_json('en')).to eq('{}')
-      end
-
-      it "doesn't return server overrides" do
-        override_translation('en', 'foo', 'bar')
-        expect(I18n.client_overrides_json('en')).to eq('{}')
-      end
-
-      it "returns client overrides" do
-        override_translation('en', 'js.foo', 'bar')
-        override_translation('en', 'admin_js.beep', 'boop')
-        json = ::JSON.parse(I18n.client_overrides_json('en'))
-
-        expect(json).to be_present
-        expect(json['js.foo']).to eq('bar')
-        expect(json['admin_js.beep']).to eq('boop')
-      end
-    end
   end
 end

spec/helpers/application_helper_spec.rb

@@ -6,13 +6,20 @@ require 'rails_helper'
 describe ApplicationHelper do
 
   describe "preload_script" do
+    def preload_link(url)
+      <<~HTML
+          <link rel="preload" href="#{url}" as="script">
+          <script src="#{url}"></script>
+      HTML
+    end
+
     it "provides brotli links to brotli cdn" do
       set_cdn_url "https://awesome.com"
 
       helper.request.env["HTTP_ACCEPT_ENCODING"] = 'br'
       link = helper.preload_script('application')
 
-      expect(link).to eq("<link rel='preload' href='https://awesome.com/brotli_asset/application.js' as='script'/>\n<script src='https://awesome.com/brotli_asset/application.js'></script>")
+      expect(link).to eq(preload_link("https://awesome.com/brotli_asset/application.js"))
     end
 
     context "with s3 CDN" do
@@ -45,26 +52,26 @@ describe ApplicationHelper do
         helper.request.env["HTTP_ACCEPT_ENCODING"] = 'br'
         link = helper.preload_script('application')
 
-        expect(link).to eq("<link rel='preload' href='https://s3cdn.com/assets/application.br.js' as='script'/>\n<script src='https://s3cdn.com/assets/application.br.js'></script>")
+        expect(link).to eq(preload_link("https://s3cdn.com/assets/application.br.js"))
       end
 
       it "gives s3 cdn if asset host is not set" do
         link = helper.preload_script('application')
 
-        expect(link).to eq("<link rel='preload' href='https://s3cdn.com/assets/application.js' as='script'/>\n<script src='https://s3cdn.com/assets/application.js'></script>")
+        expect(link).to eq(preload_link("https://s3cdn.com/assets/application.js"))
       end
 
       it "can fall back to gzip compression" do
         helper.request.env["HTTP_ACCEPT_ENCODING"] = 'gzip'
         link = helper.preload_script('application')
-        expect(link).to eq("<link rel='preload' href='https://s3cdn.com/assets/application.gz.js' as='script'/>\n<script src='https://s3cdn.com/assets/application.gz.js'></script>")
+        expect(link).to eq(preload_link("https://s3cdn.com/assets/application.gz.js"))
       end
 
       it "gives s3 cdn even if asset host is set" do
         set_cdn_url "https://awesome.com"
         link = helper.preload_script('application')
 
-        expect(link).to eq("<link rel='preload' href='https://s3cdn.com/assets/application.js' as='script'/>\n<script src='https://s3cdn.com/assets/application.js'></script>")
+        expect(link).to eq(preload_link("https://s3cdn.com/assets/application.js"))
       end
     end
   end

spec/requests/extra_locales_controller_spec.rb

@@ -28,16 +28,16 @@ describe ExtraLocalesController do
       let(:moderator) { Fabricate(:moderator) }
       before { sign_in(moderator) }
 
-      it "caches for 24 hours if version is provided and it matches current hash" do
+      it "caches for 1 year if version is provided and it matches current hash" do
         get "/extra-locales/admin", params: { v: ExtraLocalesController.bundle_js_hash('admin') }
         expect(response.status).to eq(200)
-        expect(response.headers["Cache-Control"]).to eq("max-age=86400, public, immutable")
+        expect(response.headers["Cache-Control"]).to eq("max-age=31556952, public, immutable")
       end
 
       it "does not cache at all if version is invalid" do
         get "/extra-locales/admin", params: { v: 'a' * 32 }
         expect(response.status).to eq(200)
-        expect(response.headers["Cache-Control"]).not_to eq("max-age=86400, public, immutable")
+        expect(response.headers["Cache-Control"]).not_to include("max-age", "public", "immutable")
       end
 
       context "with plugin" do
@@ -67,6 +67,47 @@ describe ExtraLocalesController do
         end
       end
     end
+
+    context "overridden translations" do
+      after { I18n.reload! }
+
+      it "works for anonymous users" do
+        TranslationOverride.upsert!(I18n.locale, 'js.some_key', 'client-side translation')
+
+        get "/extra-locales/overrides", params: { v: ExtraLocalesController.bundle_js_hash('overrides') }
+        expect(response.status).to eq(200)
+        expect(response.headers["Cache-Control"]).to eq("max-age=31556952, public, immutable")
+      end
+
+      it "returns nothing when there are not overridden translations" do
+        get "/extra-locales/overrides"
+        expect(response.status).to eq(200)
+        expect(response.body).to be_empty
+      end
+
+      context "with translations" do
+        it "returns the correct translations" do
+          TranslationOverride.upsert!(I18n.locale, 'js.some_key', 'client-side translation')
+          TranslationOverride.upsert!(I18n.locale, 'js.client_MF', '{NUM_RESULTS, plural, one {1 result} other {many} }')
+          TranslationOverride.upsert!(I18n.locale, 'admin_js.another_key', 'admin client js')
+          TranslationOverride.upsert!(I18n.locale, 'server.some_key', 'server-side translation')
+          TranslationOverride.upsert!(I18n.locale, 'server.some_MF', '{NUM_RESULTS, plural, one {1 result} other {many} }')
+
+          get "/extra-locales/overrides"
+          expect(response.status).to eq(200)
+          expect(response.body).to_not include("server.some_key", "server.some_MF")
+
+          ctx = MiniRacer::Context.new
+          ctx.eval("I18n = {};")
+          ctx.eval(response.body)
+
+          expect(ctx.eval('typeof I18n._mfOverrides["js.client_MF"]')).to eq("function")
+          expect(ctx.eval('I18n._overrides["js.some_key"]')).to eq("client-side translation")
+          expect(ctx.eval('I18n._overrides["js.client_MF"] === undefined')).to eq(true)
+          expect(ctx.eval('I18n._overrides["admin_js.another_key"]')).to eq("admin client js")
+        end
+      end
+    end
   end
 
   describe ".bundle_js_hash" do
@@ -95,4 +136,25 @@ describe ExtraLocalesController do
       expect(ExtraLocalesController.bundle_js_hash("wizard")).to eq(expected_hash_de)
     end
   end
+
+  describe ".client_overrides_exist?" do
+    after do
+      I18n.reload!
+      ExtraLocalesController.clear_cache!
+    end
+
+    it "returns false if there are no client-side translation overrides" do
+      expect(ExtraLocalesController.client_overrides_exist?).to eq(false)
+
+      TranslationOverride.upsert!(I18n.locale, 'server.some_key', 'server-side translation')
+      expect(ExtraLocalesController.client_overrides_exist?).to eq(false)
+    end
+
+    it "returns true if there are client-side translation overrides" do
+      expect(ExtraLocalesController.client_overrides_exist?).to eq(false)
+
+      TranslationOverride.upsert!(I18n.locale, 'js.some_key', 'client-side translation')
+      expect(ExtraLocalesController.client_overrides_exist?).to eq(true)
+    end
+  end
 end

test/javascripts/initializers/localization-test.js.es6

@@ -1,9 +1,9 @@
-import PreloadStore from "preload-store";
 import LocalizationInitializer from "discourse/initializers/localization";
 
 QUnit.module("initializer:localization", {
   _locale: I18n.locale,
   _translations: I18n.translations,
+  _overrides: I18n._overrides,
 
   beforeEach() {
     I18n.locale = "fr";
@@ -31,14 +31,15 @@ QUnit.module("initializer:localization", {
   afterEach() {
     I18n.locale = this._locale;
     I18n.translations = this._translations;
+    I18n._overrides = this._overrides;
   }
 });
 
 QUnit.test("translation overrides", function(assert) {
-  PreloadStore.store("translationOverrides", {
+  I18n._overrides = {
     "js.composer.reply": "WAT",
     "js.topic.reply.help": "foobar"
-  });
+  };
   LocalizationInitializer.initialize(this.registry);
 
   assert.equal(
@@ -56,10 +57,10 @@ QUnit.test("translation overrides", function(assert) {
 QUnit.test(
   "skip translation override if parent node is not an object",
   function(assert) {
-    PreloadStore.store("translationOverrides", {
+    I18n._overrides = {
       "js.composer.reply": "WAT",
       "js.composer.reply.help": "foobar"
-    });
+    };
     LocalizationInitializer.initialize(this.registry);
 
     assert.equal(I18n.t("composer.reply.help"), "[fr.composer.reply.help]");