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.
This commit is contained in:
Robin Ward 2021-04-23 10:24:42 -04:00 committed by GitHub
parent 6b10ada752
commit e3b1d1a718
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 129 additions and 39 deletions

View File

@ -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();
}
}

View File

@ -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)

View File

@ -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

View File

@ -1,3 +1,4 @@
<%- hijack_if_ember_cli! -%>
<!DOCTYPE html>
<html lang="<%= html_lang %>" class="<%= html_classes %>">
<head>

View File

@ -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)
plugins = apply_asset_filters(self.find_plugins(args), :css, args[:request])
assets = []
@ -319,9 +337,7 @@ module Discourse
plugin.js_asset_exists?
end
plugins = plugins.select do |plugin|
plugin.asset_filters.all? { |b| b.call(:js, args[:request]) }
end
plugins = apply_asset_filters(plugins, :js, args[:request])
plugins.map { |plugin| "plugins/#{plugin.directory_name}" }
end

View File

@ -15,7 +15,7 @@ Discourse::Application.routes.append do
end
after_initialize do
register_asset_filter do |type, request|
request&.fullpath&.start_with?('/styleguide')
register_asset_filter do |type, request, opts|
(opts[:path] || '').start_with?('/styleguide')
end
end

View File

@ -65,6 +65,25 @@ describe Discourse do
end
end
context "asset_filter_options" do
it "obmits path if request is missing" do
opts = Discourse.asset_filter_options(:js, nil)
expect(opts[:path]).to be_blank
end
it "returns a hash with a path from the request" do
req = stub(fullpath: "/hello", headers: {})
opts = Discourse.asset_filter_options(:js, req)
expect(opts[:path]).to eq("/hello")
end
it "overwrites the path if the asset path is present" do
req = stub(fullpath: "/bootstrap.json", headers: { "HTTP_X_DISCOURSE_ASSET_PATH" => "/hello" })
opts = Discourse.asset_filter_options(:js, req)
expect(opts[:path]).to eq("/hello")
end
end
context 'plugins' do
let(:plugin_class) do
Class.new(Plugin::Instance) do
@ -107,7 +126,7 @@ describe Discourse do
expect(Discourse.find_plugin_css_assets({}).length).to eq(2)
expect(Discourse.find_plugin_js_assets({}).length).to eq(2)
plugin1.register_asset_filter do |type, request|
plugin1.register_asset_filter do |type, request, opts|
false
end
expect(Discourse.find_plugin_css_assets({}).length).to eq(1)