From 5bd6b70d985e2736f56d2bec6cce56bee0227b1f Mon Sep 17 00:00:00 2001
From: Vinoth Kannan <vinothkannan@vinkas.com>
Date: Tue, 20 Aug 2019 22:09:52 +0530
Subject: [PATCH]  DEV: debundle plugin css assets and don't load if disabled
 (#7646)

---
 app/assets/stylesheets/desktop.scss           |  2 --
 app/assets/stylesheets/mobile.scss            |  2 --
 .../common/_discourse_stylesheet.html.erb     |  4 +++
 lib/discourse.rb                              |  8 +++++-
 lib/discourse_plugin.rb                       |  4 +--
 lib/discourse_plugin_registry.rb              | 26 ++++++++++++-------
 lib/plugin/instance.rb                        | 16 +++++++-----
 lib/stylesheet/compiler.rb                    |  2 +-
 lib/stylesheet/importer.rb                    | 19 ++++++++------
 lib/stylesheet/manager.rb                     |  6 ++---
 .../discourse_plugin_registry_spec.rb         | 23 +++++++++-------
 spec/components/discourse_plugin_spec.rb      |  4 +--
 spec/components/plugin/instance_spec.rb       | 10 +++----
 spec/components/stylesheet/manager_spec.rb    |  2 +-
 14 files changed, 76 insertions(+), 52 deletions(-)

diff --git a/app/assets/stylesheets/desktop.scss b/app/assets/stylesheets/desktop.scss
index 76012fc9c0..70229afbbc 100644
--- a/app/assets/stylesheets/desktop.scss
+++ b/app/assets/stylesheets/desktop.scss
@@ -25,6 +25,4 @@
 
 /* These files doesn't actually exist, they are injected by Stylesheet::Compiler. */
 
-@import "plugins";
-@import "plugins_desktop";
 @import "category_backgrounds";
diff --git a/app/assets/stylesheets/mobile.scss b/app/assets/stylesheets/mobile.scss
index 244c90102a..a2158fe44a 100644
--- a/app/assets/stylesheets/mobile.scss
+++ b/app/assets/stylesheets/mobile.scss
@@ -38,6 +38,4 @@
 
 /* These files doesn't actually exist, they are injected by Stylesheet::Compiler. */
 
-@import "plugins";
-@import "plugins_mobile";
 @import "category_backgrounds";
diff --git a/app/views/common/_discourse_stylesheet.html.erb b/app/views/common/_discourse_stylesheet.html.erb
index 45f6476e90..d16cb7cb4e 100644
--- a/app/views/common/_discourse_stylesheet.html.erb
+++ b/app/views/common/_discourse_stylesheet.html.erb
@@ -11,3 +11,7 @@
 <%- if theme_ids.present? %>
   <%= discourse_stylesheet_link_tag(mobile_view? ? :mobile_theme : :desktop_theme) %>
 <%- end %>
+
+<%- Discourse.find_plugin_css_assets(include_official: allow_plugins?, include_unofficial: allow_third_party_plugins?).each do |file| %>
+  <%= discourse_stylesheet_link_tag(file) %>
+<%- end %>
diff --git a/lib/discourse.rb b/lib/discourse.rb
index b7cf9a91c0..fb19a776f1 100644
--- a/lib/discourse.rb
+++ b/lib/discourse.rb
@@ -214,10 +214,16 @@ module Discourse
     end
   end
 
+  def self.find_plugin_css_assets(args)
+    self.find_plugins(args).find_all do |plugin|
+      plugin.css_asset_exists?
+    end.map { |plugin| plugin.directory_name }
+  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.map { |plugin| "plugins/#{plugin.directory_name}" }
   end
 
   def self.assets_digest
diff --git a/lib/discourse_plugin.rb b/lib/discourse_plugin.rb
index 990df5b4e0..5d687e0788 100644
--- a/lib/discourse_plugin.rb
+++ b/lib/discourse_plugin.rb
@@ -36,8 +36,8 @@ class DiscoursePlugin
     @registry.register_js(file, opts)
   end
 
-  def register_css(file)
-    @registry.register_css(file)
+  def register_css(file, plugin_directory_name)
+    @registry.register_css(file, plugin_directory_name)
   end
 
   def register_archetype(name, options = {})
diff --git a/lib/discourse_plugin_registry.rb b/lib/discourse_plugin_registry.rb
index a8cefe0ae9..f8111ed71b 100644
--- a/lib/discourse_plugin_registry.rb
+++ b/lib/discourse_plugin_registry.rb
@@ -47,15 +47,15 @@ class DiscoursePluginRegistry
     end
 
     def stylesheets
-      @stylesheets ||= Set.new
+      @stylesheets ||= Hash.new
     end
 
     def mobile_stylesheets
-      @mobile_stylesheets ||= Set.new
+      @mobile_stylesheets ||= Hash.new
     end
 
     def desktop_stylesheets
-      @desktop_stylesheets ||= Set.new
+      @desktop_stylesheets ||= Hash.new
     end
 
     def sass_variables
@@ -116,8 +116,9 @@ class DiscoursePluginRegistry
     self.svg_icons << icon
   end
 
-  def register_css(filename)
-    self.class.stylesheets << filename
+  def register_css(filename, plugin_directory_name)
+    self.class.stylesheets[plugin_directory_name] ||= Set.new
+    self.class.stylesheets[plugin_directory_name] << filename
   end
 
   def self.register_locale(locale, options = {})
@@ -153,7 +154,7 @@ class DiscoursePluginRegistry
   JS_REGEX = /\.js$|\.js\.erb$|\.js\.es6|\.js\.no-module\.es6$/
   HANDLEBARS_REGEX = /\.hbs$|\.js\.handlebars$/
 
-  def self.register_asset(asset, opts = nil)
+  def self.register_asset(asset, opts = nil, plugin_directory_name = nil)
     if asset =~ JS_REGEX
       if opts == :admin
         self.admin_javascripts << asset
@@ -166,19 +167,26 @@ class DiscoursePluginRegistry
       end
     elsif asset =~ /\.css$|\.scss$/
       if opts == :mobile
-        self.mobile_stylesheets << asset
+        self.mobile_stylesheets[plugin_directory_name] ||= Set.new
+        self.mobile_stylesheets[plugin_directory_name] << asset
       elsif opts == :desktop
-        self.desktop_stylesheets << asset
+        self.desktop_stylesheets[plugin_directory_name] ||= Set.new
+        self.desktop_stylesheets[plugin_directory_name] << asset
       elsif opts == :variables
         self.sass_variables << asset
       else
-        self.stylesheets << asset
+        self.stylesheets[plugin_directory_name] ||= Set.new
+        self.stylesheets[plugin_directory_name] << asset
       end
     elsif asset =~ HANDLEBARS_REGEX
       self.handlebars << asset
     end
   end
 
+  def self.stylesheets_exists?(plugin_directory_name)
+    self.stylesheets[plugin_directory_name].present? || self.mobile_stylesheets[plugin_directory_name].present? || self.desktop_stylesheets[plugin_directory_name].present?
+  end
+
   def self.register_seed_data(key, value)
     self.seed_data[key] = value
   end
diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb
index 67695e15a0..b8158fa757 100644
--- a/lib/plugin/instance.rb
+++ b/lib/plugin/instance.rb
@@ -431,7 +431,7 @@ class Plugin::Instance
       full_path = File.dirname(path) << "/assets/" << file
     end
 
-    assets << [full_path, opts]
+    assets << [full_path, opts, directory_name]
   end
 
   def register_service_worker(file, opts = nil)
@@ -654,8 +654,12 @@ class Plugin::Instance
     end
   end
 
-  def asset_name
-    @asset_name ||= File.dirname(path).split("/").last
+  def directory_name
+    @directory_name ||= File.dirname(path).split("/").last
+  end
+
+  def css_asset_exists?
+    DiscoursePluginRegistry.stylesheets_exists?(directory_name)
   end
 
   def js_asset_exists?
@@ -669,12 +673,12 @@ class Plugin::Instance
   end
 
   def js_file_path
-    @file_path ||= "#{Plugin::Instance.js_path}/#{asset_name}.js.erb"
+    @file_path ||= "#{Plugin::Instance.js_path}/#{directory_name}.js.erb"
   end
 
   def register_assets!
-    assets.each do |asset, opts|
-      DiscoursePluginRegistry.register_asset(asset, opts)
+    assets.each do |asset, opts, plugin_directory_name|
+      DiscoursePluginRegistry.register_asset(asset, opts, plugin_directory_name)
     end
   end
 
diff --git a/lib/stylesheet/compiler.rb b/lib/stylesheet/compiler.rb
index 19925513a3..1dea378083 100644
--- a/lib/stylesheet/compiler.rb
+++ b/lib/stylesheet/compiler.rb
@@ -12,7 +12,7 @@ module Stylesheet
 
       if Importer.special_imports[asset.to_s]
         filename = "theme_#{options[:theme_id]}.scss"
-        file = "@import \"common/foundation/variables\"; @import \"theme_variables\"; @import \"#{asset}\";"
+        file = "@import \"common/foundation/variables\"; @import \"common/foundation/mixins\"; @import \"theme_variables\"; @import \"#{asset}\";"
       else
         filename = "#{asset}.scss"
         path = "#{ASSET_ROOT}/#{filename}"
diff --git a/lib/stylesheet/importer.rb b/lib/stylesheet/importer.rb
index 728731e3c5..7258a99978 100644
--- a/lib/stylesheet/importer.rb
+++ b/lib/stylesheet/importer.rb
@@ -19,16 +19,19 @@ module Stylesheet
       Import.new("#{theme_dir(@theme_id)}/theme_field.scss", source: @theme_field)
     end
 
-    register_import "plugins" do
-      import_files(DiscoursePluginRegistry.stylesheets)
-    end
+    Discourse.plugins.each do |plugin|
+      plugin_directory_name = plugin.directory_name
 
-    register_import "plugins_mobile" do
-      import_files(DiscoursePluginRegistry.mobile_stylesheets)
-    end
+      ["", "mobile", "desktop"].each do |type|
+        asset_name = type.present? ? "#{plugin_directory_name}_#{type}" : plugin_directory_name
+        stylesheets = type.present? ? DiscoursePluginRegistry.send("#{type}_stylesheets") : DiscoursePluginRegistry.stylesheets
 
-    register_import "plugins_desktop" do
-      import_files(DiscoursePluginRegistry.desktop_stylesheets)
+        if stylesheets[plugin_directory_name].present?
+          register_import asset_name do
+            import_files(stylesheets[plugin_directory_name])
+          end
+        end
+      end
     end
 
     register_import "plugins_variables" do
diff --git a/lib/stylesheet/manager.rb b/lib/stylesheet/manager.rb
index bd7ccc8b44..2d9a2754eb 100644
--- a/lib/stylesheet/manager.rb
+++ b/lib/stylesheet/manager.rb
@@ -294,9 +294,9 @@ class Stylesheet::Manager
   # so we could end up poisoning the cache with a bad file that can not be removed
   def plugins_digest
     assets = []
-    assets += DiscoursePluginRegistry.stylesheets.to_a
-    assets += DiscoursePluginRegistry.mobile_stylesheets.to_a
-    assets += DiscoursePluginRegistry.desktop_stylesheets.to_a
+    DiscoursePluginRegistry.stylesheets.each { |_, paths| assets += paths.to_a }
+    DiscoursePluginRegistry.mobile_stylesheets.each { |_, paths| assets += paths.to_a }
+    DiscoursePluginRegistry.desktop_stylesheets.each { |_, paths| assets += paths.to_a }
     Digest::SHA1.hexdigest(assets.sort.join)
   end
 
diff --git a/spec/components/discourse_plugin_registry_spec.rb b/spec/components/discourse_plugin_registry_spec.rb
index f3c784eb3c..2dd353e1a6 100644
--- a/spec/components/discourse_plugin_registry_spec.rb
+++ b/spec/components/discourse_plugin_registry_spec.rb
@@ -13,14 +13,14 @@ describe DiscoursePluginRegistry do
   context '#stylesheets' do
     it 'defaults to an empty Set' do
       registry.stylesheets = nil
-      expect(registry.stylesheets).to eq(Set.new)
+      expect(registry.stylesheets).to eq(Hash.new)
     end
   end
 
   context '#mobile_stylesheets' do
     it 'defaults to an empty Set' do
       registry.mobile_stylesheets = nil
-      expect(registry.mobile_stylesheets).to eq(Set.new)
+      expect(registry.mobile_stylesheets).to eq(Hash.new)
     end
   end
 
@@ -70,20 +70,22 @@ describe DiscoursePluginRegistry do
   end
 
   context '.register_css' do
+    let(:plugin_directory_name) { "hello" }
+
     before do
-      registry_instance.register_css('hello.css')
+      registry_instance.register_css('hello.css', plugin_directory_name)
     end
 
     it 'is not leaking' do
-      expect(DiscoursePluginRegistry.new.stylesheets).to be_blank
+      expect(DiscoursePluginRegistry.new.stylesheets[plugin_directory_name]).to be_nil
     end
 
     it 'is returned by DiscoursePluginRegistry.stylesheets' do
-      expect(registry_instance.stylesheets.include?('hello.css')).to eq(true)
+      expect(registry_instance.stylesheets[plugin_directory_name].include?('hello.css')).to eq(true)
     end
 
     it "won't add the same file twice" do
-      expect { registry_instance.register_css('hello.css') }.not_to change(registry.stylesheets, :size)
+      expect { registry_instance.register_css('hello.css', plugin_directory_name) }.not_to change(registry.stylesheets[plugin_directory_name], :size)
     end
   end
 
@@ -157,11 +159,12 @@ describe DiscoursePluginRegistry do
     end
 
     it "does register general css properly" do
-      registry.register_asset("test.css")
-      registry.register_asset("test2.css")
+      plugin_directory_name = "my_plugin"
+      registry.register_asset("test.css", nil, plugin_directory_name)
+      registry.register_asset("test2.css", nil, plugin_directory_name)
 
-      expect(registry.mobile_stylesheets.count).to eq(0)
-      expect(registry.stylesheets.count).to eq(2)
+      expect(registry.mobile_stylesheets[plugin_directory_name]).to be_nil
+      expect(registry.stylesheets[plugin_directory_name].count).to eq(2)
     end
 
     it "registers desktop css properly" do
diff --git a/spec/components/discourse_plugin_spec.rb b/spec/components/discourse_plugin_spec.rb
index b91ada89be..42c4a72cb6 100644
--- a/spec/components/discourse_plugin_spec.rb
+++ b/spec/components/discourse_plugin_spec.rb
@@ -28,8 +28,8 @@ describe DiscoursePlugin do
   end
 
   it "delegates adding css to the registry" do
-    registry.expects(:register_css).with('test.css')
-    plugin.register_css('test.css')
+    registry.expects(:register_css).with('test.css', 'test')
+    plugin.register_css('test.css', 'test')
   end
 
   it "delegates creating archetypes" do
diff --git a/spec/components/plugin/instance_spec.rb b/spec/components/plugin/instance_spec.rb
index 9369240103..916fef5e8b 100644
--- a/spec/components/plugin/instance_spec.rb
+++ b/spec/components/plugin/instance_spec.rb
@@ -94,8 +94,8 @@ describe Plugin::Instance do
 
       plugin.send :register_assets!
 
-      expect(DiscoursePluginRegistry.mobile_stylesheets.count).to eq(0)
-      expect(DiscoursePluginRegistry.stylesheets.count).to eq(2)
+      expect(DiscoursePluginRegistry.mobile_stylesheets[plugin.directory_name]).to be_nil
+      expect(DiscoursePluginRegistry.stylesheets[plugin.directory_name].count).to eq(2)
     end
 
     it "remaps vendored_core_pretty_text asset" do
@@ -220,10 +220,10 @@ describe Plugin::Instance do
 
       expect(DiscoursePluginRegistry.javascripts.count).to eq(2)
       expect(DiscoursePluginRegistry.admin_javascripts.count).to eq(2)
-      expect(DiscoursePluginRegistry.desktop_stylesheets.count).to eq(2)
+      expect(DiscoursePluginRegistry.desktop_stylesheets[plugin.directory_name].count).to eq(2)
       expect(DiscoursePluginRegistry.sass_variables.count).to eq(2)
-      expect(DiscoursePluginRegistry.stylesheets.count).to eq(2)
-      expect(DiscoursePluginRegistry.mobile_stylesheets.count).to eq(1)
+      expect(DiscoursePluginRegistry.stylesheets[plugin.directory_name].count).to eq(2)
+      expect(DiscoursePluginRegistry.mobile_stylesheets[plugin.directory_name].count).to eq(1)
     end
   end
 
diff --git a/spec/components/stylesheet/manager_spec.rb b/spec/components/stylesheet/manager_spec.rb
index e735071258..e7054f3b7a 100644
--- a/spec/components/stylesheet/manager_spec.rb
+++ b/spec/components/stylesheet/manager_spec.rb
@@ -77,7 +77,7 @@ describe Stylesheet::Manager do
       manager = Stylesheet::Manager.new(:desktop_theme, theme.id)
       digest1 = manager.digest
 
-      DiscoursePluginRegistry.stylesheets.add "fake_file"
+      DiscoursePluginRegistry.stylesheets["fake"] = Set.new(["fake_file"])
 
       manager = Stylesheet::Manager.new(:desktop_theme, theme.id)
       digest2 = manager.digest