From 33959595be419b4778c14119cc5a0de57c8d052c Mon Sep 17 00:00:00 2001 From: David Taylor Date: Fri, 2 Feb 2024 18:39:31 +0000 Subject: [PATCH] Revert "DEV: Remove sprockets from plugin 'extra js' pipeline (#25502)" This reverts commit 1757a688c4749968db8e27623b975fc57519b97a. It seems to be causing issues with plugin-sourced images. --- lib/discourse_js_processor.rb | 33 ++++++ lib/discourse_plugin_registry.rb | 30 ++++- lib/plugin/instance.rb | 132 +++++++++++++++------ lib/pretty_text.rb | 24 ++-- spec/lib/discourse_plugin_registry_spec.rb | 14 +++ spec/lib/plugin/instance_spec.rb | 8 +- 6 files changed, 194 insertions(+), 47 deletions(-) diff --git a/lib/discourse_js_processor.rb b/lib/discourse_js_processor.rb index 26d142fa4d8..0526b0845e3 100644 --- a/lib/discourse_js_processor.rb +++ b/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 diff --git a/lib/discourse_plugin_registry.rb b/lib/discourse_plugin_registry.rb index e4bfcf0cd9b..f018cf5faf1 100644 --- a/lib/discourse_plugin_registry.rb +++ b/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 diff --git a/lib/plugin/instance.rb b/lib/plugin/instance.rb index 3513f64a5cb..eca1bd16cf7 100644 --- a/lib/plugin/instance.rb +++ b/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 - [ - "#{Plugin::Instance.js_path}/#{directory_name}.js.erb", - "#{Plugin::Instance.js_path}/#{directory_name}_extra.js.erb", - ] + def js_file_path + "#{Plugin::Instance.js_path}/#{directory_name}.js.erb" end def extra_js_file_path - @extra_js_file_path ||= "#{Plugin::Instance.js_path}/#{directory_name}_extra.js" - 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 + @extra_js_file_path ||= "#{Plugin::Instance.js_path}/#{directory_name}_extra.js.erb" end def register_assets! diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb index e0a7083404f..e22824a1179 100644 --- a/lib/pretty_text.rb +++ b/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| - Dir - .glob("#{plugin.directory}/assets/javascripts/**/discourse-markdown/**/*.js") - .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 = [] + DiscoursePluginRegistry.each_globbed_asset do |a| + to_load << a if File.file?(a) && a =~ /discourse-markdown/ + 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)) } diff --git a/spec/lib/discourse_plugin_registry_spec.rb b/spec/lib/discourse_plugin_registry_spec.rb index f25b05cb195..6bd872f78f0 100644 --- a/spec/lib/discourse_plugin_registry_spec.rb +++ b/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) diff --git a/spec/lib/plugin/instance_spec.rb b/spec/lib/plugin/instance_spec.rb index e2c0236b2b9..773e2f769c4 100644 --- a/spec/lib/plugin/instance_spec.rb +++ b/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)