From 839916aa4901ab62fb84bcaf7d91c4354091f506 Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Mon, 15 Jul 2019 20:22:54 +0530 Subject: [PATCH] DEV: Debundle plugin javascript assets and don't load if disabled (#7566) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit And don't load javascript assets if plugin is disabled. * precompile auto generated plugin js assets * SPEC: remove spec test functions * remove plugin js from test_helper Co-Authored-By: Régis Hanol * DEV: using equality is slightly easier to read than inequality Co-Authored-By: Régis Hanol * DEV: use `select` method instead of `find_all` for readability Co-Authored-By: Régis Hanol --- .gitignore | 3 ++ .../javascripts/plugin-third-party.js.erb | 15 -------- app/assets/javascripts/plugin.js.erb | 16 --------- app/views/layouts/application.html.erb | 8 ++--- config/application.rb | 6 ++-- lib/discourse.rb | 16 +++++++++ lib/plugin/instance.rb | 34 +++++++++++++++++++ spec/requests/list_controller_spec.rb | 16 --------- test/javascripts/test_helper.js | 1 - 9 files changed, 59 insertions(+), 56 deletions(-) delete mode 100644 app/assets/javascripts/plugin-third-party.js.erb delete mode 100644 app/assets/javascripts/plugin.js.erb diff --git a/.gitignore b/.gitignore index 2b3d1fa7230..bf341e74262 100644 --- a/.gitignore +++ b/.gitignore @@ -127,3 +127,6 @@ vendor/bundle/* # Vagrant .vagrant + +# ignore auto-generated plugin js assets +/app/assets/javascripts/plugins/* diff --git a/app/assets/javascripts/plugin-third-party.js.erb b/app/assets/javascripts/plugin-third-party.js.erb deleted file mode 100644 index f5cd239d59a..00000000000 --- a/app/assets/javascripts/plugin-third-party.js.erb +++ /dev/null @@ -1,15 +0,0 @@ -<% -# Include plugin javascripts/handlebars templates -Discourse.unofficial_plugins.each do |plugin| - plugin.javascript_includes.each { |js| require_asset(js) } - plugin.handlebars_includes.each { |hb| require_asset(hb) } - - plugin.each_globbed_asset do |f, is_dir| - if is_dir - depend_on(f) - else - require_asset(f) - end - end -end -%> diff --git a/app/assets/javascripts/plugin.js.erb b/app/assets/javascripts/plugin.js.erb deleted file mode 100644 index 19e879b2edd..00000000000 --- a/app/assets/javascripts/plugin.js.erb +++ /dev/null @@ -1,16 +0,0 @@ -<% -# Include plugin javascripts/handlebars templates -Discourse.official_plugins.each do |plugin| - plugin.javascript_includes.each { |js| require_asset(js) } - plugin.handlebars_includes.each { |hb| require_asset(hb) } - - plugin.each_globbed_asset do |f, is_dir| - if is_dir - depend_on(f) - else - require_asset(f) - end - end -end - -%> diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 51d5746f97b..0678a010cb1 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -25,13 +25,9 @@ <%= preload_script "vendor" %> <%= preload_script "pretty-text-bundle" %> <%= preload_script "application" %> - <%- if allow_plugins? %> - <%= preload_script "plugin" %> + <%- Discourse.find_plugin_js_assets(include_official: allow_plugins?, include_unofficial: allow_third_party_plugins?).each do |file| %> + <%= preload_script file %> <%- end %> - <%- if allow_third_party_plugins? %> - <%= preload_script "plugin-third-party" %> - <%- end %> - <%- if staff? %> <%= preload_script "admin" %> diff --git a/config/application.rb b/config/application.rb index cd2a9f85aaa..2d946b30ec7 100644 --- a/config/application.rb +++ b/config/application.rb @@ -132,8 +132,6 @@ module Discourse pretty-text-bundle.js wizard-application.js wizard-vendor.js - plugin.js - plugin-third-party.js markdown-it-bundle.js service-worker.js google-tag-manager.js @@ -261,6 +259,10 @@ module Discourse Discourse.activate_plugins! end + Discourse.find_plugin_js_assets(include_disabled: true).each do |file| + config.assets.precompile << "#{file}.js" + end + require_dependency 'stylesheet/manager' require_dependency 'svg_sprite/svg_sprite' diff --git a/lib/discourse.rb b/lib/discourse.rb index f1adb928c27..d413c1a6af5 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -204,6 +204,22 @@ module Discourse plugins.find_all { |p| !p.metadata.official? } end + def self.find_plugins(args) + plugins.select do |plugin| + next if args[:include_official] == false && plugin.metadata.official? + next if args[:include_unofficial] == false && !plugin.metadata.official? + next if args[:include_disabled] == false && !plugin.enabled? + + true + end + end + + def self.find_plugin_js_assets(args) + self.find_plugins(args).find_all do |plugin| + plugin.js_asset_exists? + end.map { |plugin| "plugins/#{plugin.asset_name}" } + end + def self.assets_digest @assets_digest ||= begin digest = Digest::MD5.hexdigest(ActionView::Base.assets_manifest.assets.values.sort.join) diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb index 33da7defb7d..9849d0847c4 100644 --- a/lib/plugin/instance.rb +++ b/lib/plugin/instance.rb @@ -529,6 +529,24 @@ class Plugin::Instance Discourse::Utils.execute_command('rm', '-f', target) Discourse::Utils.execute_command('ln', '-s', public_data, target) end + + ensure_directory(Plugin::Instance.js_path) + + contents = [] + handlebars_includes.each { |hb| contents << "require_asset('#{hb}')" } + javascript_includes.each { |js| contents << "require_asset('#{js}')" } + + each_globbed_asset do |f, is_dir| + contents << (is_dir ? "depend_on('#{f}')" : "require_asset('#{f}')") + end + + File.delete(js_file_path) if js_asset_exists? + + if contents.present? + contents.insert(0, "<%") + contents << "%>" + write_asset(js_file_path, contents.join("\n")) + end end def auth_provider(opts) @@ -629,8 +647,24 @@ class Plugin::Instance end end + def asset_name + @asset_name ||= File.dirname(path).split("/").last + end + + def js_asset_exists? + File.exists?(js_file_path) + end + protected + def self.js_path + File.expand_path "#{Rails.root}/app/assets/javascripts/plugins" + end + + def js_file_path + @file_path ||= "#{Plugin::Instance.js_path}/#{asset_name}.js.erb" + end + def register_assets! assets.each do |asset, opts| DiscoursePluginRegistry.register_asset(asset, opts) diff --git a/spec/requests/list_controller_spec.rb b/spec/requests/list_controller_spec.rb index 2cdb8628063..7f85be92835 100644 --- a/spec/requests/list_controller_spec.rb +++ b/spec/requests/list_controller_spec.rb @@ -646,20 +646,4 @@ RSpec.describe ListController do expect(topic_titles).to include(topic_in_sub_category.title) end end - - describe "safe mode" do - it "handles safe mode" do - get "/latest" - expect(response.body).to match(/plugin\.js/) - expect(response.body).to match(/plugin-third-party\.js/) - - get "/latest", params: { safe_mode: "no_plugins" } - expect(response.body).not_to match(/plugin\.js/) - expect(response.body).not_to match(/plugin-third-party\.js/) - - get "/latest", params: { safe_mode: "only_official" } - expect(response.body).to match(/plugin\.js/) - expect(response.body).not_to match(/plugin-third-party\.js/) - end - end end diff --git a/test/javascripts/test_helper.js b/test/javascripts/test_helper.js index 678b0b5399c..2d3b2334789 100644 --- a/test/javascripts/test_helper.js +++ b/test/javascripts/test_helper.js @@ -24,7 +24,6 @@ //= require pretty-text-bundle //= require markdown-it-bundle //= require application -//= require plugin //= require admin //= require sinon/pkg/sinon