DEV: Improve Ember CLI's bootstrap logic (#12792)

DEV: Improve Ember CLI’s bootstrap logic (#12792)

  • DEV: Give a nicer error when --proxy argument is missing

  • DEV: Improve Ember CLI’s bootstrap logic

Instead of having Ember CLI know which URLs to proxy or not, have it try the URL with a special header HTTP_X_DISCOURSE_EMBER_CLI. If present, and Discourse thinks we should bootstrap the application, it will instead stop rendering and return a HTTP HEAD with a response header telling Ember CLI to bootstrap.

In other words, any time Rails would otherwise serve up the HTML for the Ember app, it stops and says “no, you do it.”

  • DEV: Support asset filters by path using a new options object

Without this, Ember CLI’s bootstrap would not get the assets it wants because the path it was requesting was different than the browser path. This adds an optional request header to fix it.

So far this is only used by the styleguide.

diff --git a/app/assets/javascripts/discourse/lib/bootstrap-json/index.js b/app/assets/javascripts/discourse/lib/bootstrap-json/index.js
index e67e996..c66e186 100644
--- a/app/assets/javascripts/discourse/lib/bootstrap-json/index.js
+++ b/app/assets/javascripts/discourse/lib/bootstrap-json/index.js
@@ -141,25 +141,61 @@ function applyBootstrap(bootstrap, template) {
   return template;
 }
 
-function decorateIndex(assetPath, baseUrl, headers) {
+function buildFromBootstrap(assetPath, proxy, req) {
   // eslint-disable-next-line
   return new Promise((resolve, reject) => {
     fs.readFile(
       path.join(process.cwd(), "dist", assetPath),
       "utf8",
       (err, template) => {
-        getJSON(`${baseUrl}/bootstrap.json`, null, headers)
+        getJSON(`${proxy}/bootstrap.json`, null, req.headers)
           .then((json) => {
             resolve(applyBootstrap(json.bootstrap, template));
           })
           .catch(() => {
-            reject(`Could not get ${baseUrl}/bootstrap.json`);
+            reject(`Could not get ${proxy}/bootstrap.json`);
           });
       }
     );
   });
 }
 
+async function handleRequest(assetPath, proxy, req, res) {
+  if (assetPath.endsWith("index.html")) {
+    try {
+      // Avoid Ember CLI's proxy if doing a GET, since Discourse depends on some non-XHR
+      // GET requests to work.
+      if (req.method === "GET") {
+        let url = `${proxy}${req.path}`;
+
+        let queryLoc = req.url.indexOf("?");
+        if (queryLoc !== -1) {
+          url += req.url.substr(queryLoc);
+        }
+
+        req.headers["X-Discourse-Ember-CLI"] = "true";
+        let get = bent("GET", [200, 404, 403, 500]);
+        let response = await get(url, null, req.headers);
+        if (response.headers["x-discourse-bootstrap-required"] === "true") {
+          req.headers["X-Discourse-Asset-Path"] = req.path;
+          let json = await buildFromBootstrap(assetPath, proxy, req);
+          return res.send(json);
+        }
+        res.status(response.status);
+        res.set(response.headers);
+        res.send(await response.text());
+      }
+    } catch (e) {
+      res.send(`
+                <html>
+                  <h1>Discourse Build Error</h1>
+                  <p>${e.toString()}</p>
+                </html>
+              `);
+    }
+  }
+}
+
 module.exports = {
   name: require("./package").name,
 
@@ -172,6 +208,16 @@ module.exports = {
     let app = config.app;
     let options = config.options;
 
+    if (!proxy) {
+      // eslint-disable-next-line
+      console.error(`
+Discourse can't be run without a \`--proxy\` setting, because it needs a Rails application
+to serve API requests. For example:
+
+  yarn run ember serve --proxy "http://localhost:3000"\n`);
+      throw "--proxy argument is required";
+    }
+
     let watcher = options.watcher;
 
     let baseURL =
@@ -180,7 +226,6 @@ module.exports = {
         : cleanBaseURL(options.rootURL || options.baseURL);
 
     app.use(async (req, res, next) => {
-      let sentTemplate = false;
       try {
         const results = await watcher;
         if (this.shouldHandleRequest(req, options)) {
@@ -191,32 +236,15 @@ module.exports = {
             isFile = fs
               .statSync(path.join(results.directory, assetPath))
               .isFile();
-          } catch (err) {
-            /* ignore */
-          }
+          } catch (err) {}
 
           if (!isFile) {
             assetPath = "index.html";
           }
-
-          if (assetPath.endsWith("index.html")) {
-            let template;
-            try {
-              template = await decorateIndex(assetPath, proxy, req.headers);
-            } catch (e) {
-              template = `
-                <html>
-                  <h1>Discourse Build Error</h1>
-                  <p>${e.toString()}</p>
-                </html>
-              `;
-            }
-            sentTemplate = true;
-            return res.send(template);
-          }
+          await handleRequest(assetPath, proxy, req, res);
         }
       } finally {
-        if (!sentTemplate) {
+        if (!res.headersSent) {
           return next();
         }
       }
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 885e53e..6903b9d 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -107,9 +107,23 @@ class ApplicationController < ActionController::Base
 
   class RenderEmpty < StandardError; end
   class PluginDisabled < StandardError; end
+  class EmberCLIHijacked < StandardError; end
+
+  def catch_ember_cli_hijack
+    yield
+  rescue ActionView::Template::Error => ex
+    raise ex unless ex.cause.is_a?(EmberCLIHijacked)
+    send_ember_cli_bootstrap
+  end
 
   rescue_from RenderEmpty do
-    with_resolved_locale { render 'default/empty' }
+    catch_ember_cli_hijack do
+      with_resolved_locale { render 'default/empty' }
+    end
+  end
+
+  rescue_from EmberCLIHijacked do
+    send_ember_cli_bootstrap
   end
 
   rescue_from ArgumentError do |e|
@@ -286,13 +300,19 @@ class ApplicationController < ActionController::Base
       rescue Discourse::InvalidAccess
         return render plain: message, status: status_code
       end
-      with_resolved_locale do
-        error_page_opts[:layout] = opts[:include_ember] ? 'application' : 'no_ember'
-        render html: build_not_found_page(error_page_opts)
+      catch_ember_cli_hijack do
+        with_resolved_locale do
+          error_page_opts[:layout] = opts[:include_ember] ? 'application' : 'no_ember'
+          render html: build_not_found_page(error_page_opts)
+        end
       end
     end
   end
 
+  def send_ember_cli_bootstrap
+    head 200, content_type: "text/html", "X-Discourse-Bootstrap-Required": true
+  end
+
   # If a controller requires a plugin, it will raise an exception if that plugin is
   # disabled. This allows plugins to be disabled programatically.
   def self.requires_plugin(plugin_name)
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 2df54fc..624d332 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -593,4 +593,10 @@ module ApplicationHelper
       current_user ? nil : value
     end
   end
+
+  def hijack_if_ember_cli!
+    if request.headers["HTTP_X_DISCOURSE_EMBER_CLI"] == "true"
+      raise ApplicationController::EmberCLIHijacked.new
+    end
+  end
 end
diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb
index 796a7be..205637c 100644
--- a/app/views/layouts/application.html.erb
+++ b/app/views/layouts/application.html.erb
@@ -1,3 +1,4 @@
+<%- hijack_if_ember_cli! -%>
 <!DOCTYPE html>
 <html lang="<%= html_lang %>" class="<%= html_classes %>">
   <head>
diff --git a/lib/discourse.rb b/lib/discourse.rb
index 34aca6d..a04949f 100644
--- a/lib/discourse.rb
+++ b/lib/discourse.rb
@@ -290,12 +290,30 @@ module Discourse
     end
   end
 
-  def self.find_plugin_css_assets(args)
-    plugins = self.find_plugins(args)
-
-    plugins = plugins.select do |plugin|
-      plugin.asset_filters.all? { |b| b.call(:css, args[:request]) }
+  def self.apply_asset_filters(plugins, type, request)
+    filter_opts = asset_filter_options(type, request)
+    plugins.select do |plugin|
+      plugin.asset_filters.all? { |b| b.call(type, request, filter_opts) }
     end
+  end
+
+  def self.asset_filter_options(type, request)
+    result = {}
+    return result if request.blank?
+
+    path = request.fullpath
+    result[:path] = path if path.present?
+
+    # When we bootstrap using the JSON method, we want to be able to filter assets on
+    # the path we're bootstrapping for.
+    asset_path = request.headers["HTTP_X_DISCOURSE_ASSET_PATH"]
+    result[:path] = asset_path if asset_path.present?
+
+    result
+  end
+
+  def self.find_plugin_css_assets(args)

[... diff too long, it was truncated ...]

GitHub sha: e3b1d1a7

This commit appears in #12792 which was approved by davidtaylorhq. It was merged by eviltrout.