Revert "DEV: Remove sprockets from plugin 'extra js' pipeline (#25502)"

This reverts commit 1757a688c4. It seems to be causing issues with plugin-sourced images.

lib/discourse_js_processor.rb

@@ -17,6 +17,10 @@ class DiscourseJsProcessor
     "proposal-export-namespace-from",
   ]
 
+  def self.plugin_transpile_paths
+    @@plugin_transpile_paths ||= Set.new
+  end
+
   def self.ember_cli?(filename)
     filename.include?("/app/assets/javascripts/discourse/dist/")
   end
@@ -29,6 +33,19 @@ class DiscourseJsProcessor
 
     data = transpile(data, root_path, logical_path) if should_transpile?(input[:filename])
 
+    # add sourceURL until we can do proper source maps
+    if !Rails.env.production? && !ember_cli?(input[:filename])
+      plugin_name = root_path[%r{/plugins/([\w-]+)/assets}, 1]
+      source_url =
+        if plugin_name
+          "plugins/#{plugin_name}/assets/javascripts/#{logical_path}"
+        else
+          logical_path
+        end
+
+      data = "eval(#{data.inspect} + \"\\n//# sourceURL=#{source_url}\");\n"
+    end
+
     { data: data }
   end
 
@@ -57,6 +74,22 @@ class DiscourseJsProcessor
     return false if relative_path.start_with?("#{js_root}/locales/")
     return false if relative_path.start_with?("#{js_root}/plugins/")
 
+    if %w[
+         start-discourse
+         onpopstate-handler
+         google-tag-manager
+         google-universal-analytics-v3
+         google-universal-analytics-v4
+         activate-account
+         auto-redirect
+         embed-application
+         app-boot
+       ].any? { |f| relative_path == "#{js_root}/#{f}.js" }
+      return true
+    end
+
+    return true if plugin_transpile_paths.any? { |prefix| relative_path.start_with?(prefix) }
+
     !!(relative_path =~ %r{^#{js_root}/[^/]+/} || relative_path =~ %r{^#{test_root}/[^/]+/})
   end
 

lib/discourse_plugin_registry.rb

@@ -53,15 +53,18 @@ class DiscoursePluginRegistry
   define_register :javascripts, Set
   define_register :auth_providers, Set
   define_register :service_workers, Set
+  define_register :admin_javascripts, Set
   define_register :stylesheets, Hash
   define_register :mobile_stylesheets, Hash
   define_register :desktop_stylesheets, Hash
   define_register :color_definition_stylesheets, Hash
+  define_register :handlebars, Set
   define_register :serialized_current_user_fields, Set
   define_register :seed_data, HashWithIndifferentAccess
   define_register :locales, HashWithIndifferentAccess
   define_register :svg_icons, Set
   define_register :custom_html, Hash
+  define_register :asset_globs, Set
   define_register :html_builders, Hash
   define_register :seed_path_builders, Set
   define_register :vendored_pretty_text, Set
@@ -155,11 +158,34 @@ class DiscoursePluginRegistry
     Archetype.register(name, options)
   end
 
+  def self.register_glob(root, extension, options = nil)
+    self.asset_globs << [root, extension, options || {}]
+  end
+
+  def self.each_globbed_asset(each_options = nil)
+    each_options ||= {}
+
+    self.asset_globs.each do |g|
+      root, ext, options = *g
+
+      if options[:admin]
+        next unless each_options[:admin]
+      else
+        next if each_options[:admin]
+      end
+
+      Dir.glob("#{root}/**/*.#{ext}") { |f| yield f }
+    end
+  end
+
   JS_REGEX = /\.js$|\.js\.erb$|\.js\.es6\z/
+  HANDLEBARS_REGEX = /\.(hb[rs]|js\.handlebars)\z/
 
   def self.register_asset(asset, opts = nil, plugin_directory_name = nil)
     if asset =~ JS_REGEX
-      if opts == :vendored_pretty_text
+      if opts == :admin
+        self.admin_javascripts << asset
+      elsif opts == :vendored_pretty_text
         self.vendored_pretty_text << asset
       elsif opts == :vendored_core_pretty_text
         self.vendored_core_pretty_text << asset
@@ -179,6 +205,8 @@ class DiscoursePluginRegistry
         self.stylesheets[plugin_directory_name] ||= Set.new
         self.stylesheets[plugin_directory_name] << asset
       end
+    elsif asset =~ HANDLEBARS_REGEX
+      self.handlebars << asset
     end
   end
 

lib/plugin/instance.rb

@@ -663,11 +663,6 @@ class Plugin::Instance
         Any hbs files under `assets/javascripts` will be automatically compiled and included."
       ERROR
 
-    raise <<~ERROR if file.start_with?("javascripts/") && file.end_with?(".js", ".js.es6")
-        [#{name}] Javascript files under `assets/javascripts` are automatically included in JS bundles.
-        Manual register_asset calls should be removed. (attempted to add #{file})
-      ERROR
-
     if opts && opts == :vendored_core_pretty_text
       full_path = DiscoursePluginRegistry.core_asset_for_name(file)
     else
@@ -724,6 +719,38 @@ class Plugin::Instance
   # this allows us to present information about a plugin in the UI
   # prior to activations
   def activate!
+    if @path
+      root_dir_name = File.dirname(@path)
+
+      # Automatically include all ES6 JS and hbs files
+      root_path = "#{root_dir_name}/assets/javascripts"
+      DiscoursePluginRegistry.register_glob(root_path, "js")
+      DiscoursePluginRegistry.register_glob(root_path, "js.es6")
+      DiscoursePluginRegistry.register_glob(root_path, "hbs")
+      DiscoursePluginRegistry.register_glob(root_path, "hbr")
+
+      admin_path = "#{root_dir_name}/admin/assets/javascripts"
+      DiscoursePluginRegistry.register_glob(admin_path, "js", admin: true)
+      DiscoursePluginRegistry.register_glob(admin_path, "js.es6", admin: true)
+      DiscoursePluginRegistry.register_glob(admin_path, "hbs", admin: true)
+      DiscoursePluginRegistry.register_glob(admin_path, "hbr", admin: true)
+
+      DiscourseJsProcessor.plugin_transpile_paths << root_path.sub(Rails.root.to_s, "").sub(
+        %r{\A/*},
+        "",
+      )
+      DiscourseJsProcessor.plugin_transpile_paths << admin_path.sub(Rails.root.to_s, "").sub(
+        %r{\A/*},
+        "",
+      )
+
+      test_path = "#{root_dir_name}/test/javascripts"
+      DiscourseJsProcessor.plugin_transpile_paths << test_path.sub(Rails.root.to_s, "").sub(
+        %r{\A/*},
+        "",
+      )
+    end
+
     self.instance_eval File.read(path), path
     if auto_assets = generate_automatic_assets!
       assets.concat(auto_assets)
@@ -735,6 +762,14 @@ class Plugin::Instance
 
     seed_data.each { |key, value| DiscoursePluginRegistry.register_seed_data(key, value) }
 
+    # TODO: possibly amend this to a rails engine
+
+    # Automatically include assets
+    Rails.configuration.assets.paths << auto_generated_path
+    Rails.configuration.assets.paths << File.dirname(path) + "/assets"
+    Rails.configuration.assets.paths << File.dirname(path) + "/admin/assets"
+    Rails.configuration.assets.paths << File.dirname(path) + "/test/javascripts"
+
     # Automatically include rake tasks
     Rake.add_rakelib(File.dirname(path) + "/lib/tasks")
 
@@ -756,7 +791,29 @@ class Plugin::Instance
       Discourse::Utils.atomic_ln_s(public_data, target)
     end
 
-    write_extra_js!
+    ensure_directory(js_file_path)
+
+    contents = []
+    handlebars_includes.each { |hb| contents << "require_asset('#{hb}')" }
+    javascript_includes.each { |js| contents << "require_asset('#{js}')" }
+
+    if !contents.present?
+      [js_file_path, extra_js_file_path].each do |f|
+        File.delete(f)
+      rescue Errno::ENOENT
+      end
+      return
+    end
+
+    contents.insert(0, "<%")
+    contents << "%>"
+
+    Discourse::Utils.atomic_write_file(extra_js_file_path, contents.join("\n"))
+
+    begin
+      File.delete(js_file_path)
+    rescue Errno::ENOENT
+    end
   end
 
   def auth_provider(opts)
@@ -812,16 +869,49 @@ class Plugin::Instance
     end
   end
 
+  def handlebars_includes
+    assets
+      .map do |asset, opts|
+        next if opts == :admin
+        next unless asset =~ DiscoursePluginRegistry::HANDLEBARS_REGEX
+        asset
+      end
+      .compact
+  end
+
   def javascript_includes
     assets
       .map do |asset, opts|
         next if opts == :vendored_core_pretty_text
+        next if opts == :admin
         next unless asset =~ DiscoursePluginRegistry::JS_REGEX
         asset
       end
       .compact
   end
 
+  def each_globbed_asset
+    if @path
+      # Automatically include all ES6 JS and hbs files
+      root_path = "#{File.dirname(@path)}/assets/javascripts"
+      admin_path = "#{File.dirname(@path)}/admin/assets/javascripts"
+
+      Dir
+        .glob(["#{root_path}/**/*", "#{admin_path}/**/*"])
+        .sort
+        .each do |f|
+          f_str = f.to_s
+          if File.directory?(f)
+            yield [f, true]
+          elsif f_str.end_with?(".js.es6") || f_str.end_with?(".hbs") || f_str.end_with?(".hbr")
+            yield [f, false]
+          elsif f_str.end_with?(".js")
+            yield [f, false]
+          end
+        end
+    end
+  end
+
   def register_reviewable_type(reviewable_type_class)
     extend_list_method Reviewable, :types, [reviewable_type_class.name]
   end
@@ -1206,36 +1296,12 @@ class Plugin::Instance
     File.expand_path "#{Rails.root}/app/assets/javascripts/plugins"
   end
 
-  def legacy_asset_paths
+  def js_file_path
-    [
+    "#{Plugin::Instance.js_path}/#{directory_name}.js.erb"
-      "#{Plugin::Instance.js_path}/#{directory_name}.js.erb",
-      "#{Plugin::Instance.js_path}/#{directory_name}_extra.js.erb",
-    ]
   end
 
   def extra_js_file_path
-    @extra_js_file_path ||= "#{Plugin::Instance.js_path}/#{directory_name}_extra.js"
+    @extra_js_file_path ||= "#{Plugin::Instance.js_path}/#{directory_name}_extra.js.erb"
-  end
-
-  def write_extra_js!
-    # No longer used, but we want to make sure the files are no longer present
-    # so they don't accidently get compiled by Sprockets.
-    legacy_asset_paths.each do |path|
-      File.delete(path)
-    rescue Errno::ENOENT
-    end
-
-    contents = javascript_includes.map { |js| File.read(js) }
-
-    if contents.present?
-      ensure_directory(extra_js_file_path)
-      Discourse::Utils.atomic_write_file(extra_js_file_path, contents.join("\n;\n"))
-    else
-      begin
-        File.delete(extra_js_file_path)
-      rescue Errno::ENOENT
-      end
-    end
   end
 
   def register_assets!

lib/pretty_text.rb

@@ -103,18 +103,18 @@ module PrettyText
     ctx.load("#{Rails.root}/lib/pretty_text/shims.js")
     ctx.eval("__setUnicode(#{Emoji.unicode_replacements_json})")
 
-    Discourse.plugins.each do |plugin|
+    to_load = []
-      Dir
+    DiscoursePluginRegistry.each_globbed_asset do |a|
-        .glob("#{plugin.directory}/assets/javascripts/**/discourse-markdown/**/*.js")
+      to_load << a if File.file?(a) && a =~ /discourse-markdown/
-        .filter { |a| File.file?(a) && a.end_with?(".js", ".js.es6") }
-        .each do |f|
-          module_name =
-            f.sub(%r{\A.+assets/javascripts/}, "discourse/plugins/#{plugin.name}/").sub(
-              /\.js(\.es6)?\z/,
-              "",
-            )
-          apply_es6_file(ctx: ctx, path: f, module_name: module_name)
     end
+    to_load.uniq.each do |f|
+      plugin_name = f[%r{/plugins/([^/]+)/}, 1]
+      module_name = f[%r{/assets/javascripts/(.+)\.}, 1]
+      apply_es6_file(
+        ctx: ctx,
+        path: f,
+        module_name: "discourse/plugins/#{plugin_name}/#{module_name}",
+      )
     end
 
     DiscoursePluginRegistry.vendored_core_pretty_text.each { |vpt| ctx.eval(File.read(vpt)) }

spec/lib/discourse_plugin_registry_spec.rb

@@ -83,6 +83,13 @@ RSpec.describe DiscoursePluginRegistry do
     end
   end
 
+  describe "#admin_javascripts" do
+    it "defaults to an empty Set" do
+      registry.reset!
+      expect(registry.admin_javascripts).to eq(Set.new)
+    end
+  end
+
   describe "#seed_data" do
     it "defaults to an empty Set" do
       registry.reset!
@@ -223,6 +230,13 @@ RSpec.describe DiscoursePluginRegistry do
       expect(registry.stylesheets[plugin_directory_name]).to eq(nil)
     end
 
+    it "registers admin javascript properly" do
+      registry.register_asset("my_admin.js", :admin)
+
+      expect(registry.admin_javascripts.count).to eq(1)
+      expect(registry.javascripts.count).to eq(0)
+    end
+
     it "registers vendored_core_pretty_text properly" do
       registry.register_asset("my_lib.js", :vendored_core_pretty_text)
 

spec/lib/plugin/instance_spec.rb

@@ -408,9 +408,15 @@ TEXT
       plugin.register_asset("desktop.css", :desktop)
       plugin.register_asset("desktop2.css", :desktop)
 
+      plugin.register_asset("code.js")
+
+      plugin.register_asset("my_admin.js", :admin)
+      plugin.register_asset("my_admin2.js", :admin)
+
       plugin.activate!
 
-      expect(DiscoursePluginRegistry.javascripts.count).to eq(1)
+      expect(DiscoursePluginRegistry.javascripts.count).to eq(2)
+      expect(DiscoursePluginRegistry.admin_javascripts.count).to eq(2)
       expect(DiscoursePluginRegistry.desktop_stylesheets[plugin.directory_name].count).to eq(2)
       expect(DiscoursePluginRegistry.stylesheets[plugin.directory_name].count).to eq(2)
       expect(DiscoursePluginRegistry.mobile_stylesheets[plugin.directory_name].count).to eq(1)