DEV: Debundle plugin javascript assets and don’t load if disabled (#7566)
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 regis@hanol.fr
- DEV: using equality is slightly easier to read than inequality
Co-Authored-By: Régis Hanol regis@hanol.fr
- DEV: use
selectmethod instead offind_allfor readability
Co-Authored-By: Régis Hanol regis@hanol.fr
diff --git a/.gitignore b/.gitignore
index 2b3d1fa..bf341e7 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 f5cd239..0000000
--- 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 19e879b..0000000
--- 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 51d5746..0678a01 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? %>
<script src="<%= ExtraLocalesController.url('admin') %>"></script>
<%= preload_script "admin" %>
diff --git a/config/application.rb b/config/application.rb
index cd2a9f8..2d946b3 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 f1adb92..d413c1a 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 33da7de..9849d08 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 2cdb862..7f85be9 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 678b0b5..2d3b233 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
GitHub sha: 839916aa